From 0af76ca83781ea30939dfbf56351664fa3baf7cb Mon Sep 17 00:00:00 2001 From: Austin DeNoble Date: Thu, 28 May 2026 13:02:19 -0400 Subject: [PATCH 01/17] Refactor `pc config` to use key-based get/set/list/describe/unset commands Replaces the per-setting subcommands (get-api-key, set-api-key, set-color, set-environment) with a generic, registry-driven interface modeled after `gh config` and `aws configure`. A new keyDescriptor in registry.go holds each setting's getter, setter, validator, side-effect hook, short/long descriptions, and sensitivity flag. Adding a new key is supported through a registry entry rather than a command file. Adds `pc config get|set|unset|list|describe `, with --json and --reveal flag support for sensitive values like API keys. The four legacy commands are maintained as hidden, deprecated aliases to maintain functionality for existing users. --- internal/pkg/cli/command/config/cmd.go | 12 +- internal/pkg/cli/command/config/describe.go | 89 +++++++++ internal/pkg/cli/command/config/get.go | 65 +++++++ .../pkg/cli/command/config/get_api_key.go | 6 +- internal/pkg/cli/command/config/list.go | 72 +++++++ internal/pkg/cli/command/config/registry.go | 183 ++++++++++++++++++ internal/pkg/cli/command/config/set.go | 62 ++++++ .../pkg/cli/command/config/set_api_key.go | 8 +- internal/pkg/cli/command/config/set_color.go | 6 +- .../pkg/cli/command/config/set_environment.go | 11 +- internal/pkg/cli/command/config/unset.go | 36 ++++ internal/pkg/cli/command/root/root.go | 5 + 12 files changed, 541 insertions(+), 14 deletions(-) create mode 100644 internal/pkg/cli/command/config/describe.go create mode 100644 internal/pkg/cli/command/config/get.go create mode 100644 internal/pkg/cli/command/config/list.go create mode 100644 internal/pkg/cli/command/config/registry.go create mode 100644 internal/pkg/cli/command/config/set.go create mode 100644 internal/pkg/cli/command/config/unset.go diff --git a/internal/pkg/cli/command/config/cmd.go b/internal/pkg/cli/command/config/cmd.go index 467bf3ee..35556654 100644 --- a/internal/pkg/cli/command/config/cmd.go +++ b/internal/pkg/cli/command/config/cmd.go @@ -21,9 +21,17 @@ func NewConfigCmd() *cobra.Command { Long: configHelp, } - cmd.AddCommand(NewSetColorCmd()) - cmd.AddCommand(NewSetApiKeyCmd()) + // Primary commands + cmd.AddCommand(NewGetCmd()) + cmd.AddCommand(NewSetCmd()) + cmd.AddCommand(NewUnsetCmd()) + cmd.AddCommand(NewListCmd()) + cmd.AddCommand(NewDescribeCmd()) + + // Deprecated aliases kept for backwards compatibility cmd.AddCommand(NewGetApiKeyCmd()) + cmd.AddCommand(NewSetApiKeyCmd()) + cmd.AddCommand(NewSetColorCmd()) cmd.AddCommand(NewSetEnvCmd()) return cmd diff --git a/internal/pkg/cli/command/config/describe.go b/internal/pkg/cli/command/config/describe.go new file mode 100644 index 00000000..b1cf16b7 --- /dev/null +++ b/internal/pkg/cli/command/config/describe.go @@ -0,0 +1,89 @@ +package config + +import ( + "fmt" + "os" + "strings" + + "github.com/pinecone-io/cli/internal/pkg/utils/exit" + "github.com/pinecone-io/cli/internal/pkg/utils/help" + "github.com/pinecone-io/cli/internal/pkg/utils/msg" + "github.com/pinecone-io/cli/internal/pkg/utils/presenters" + "github.com/pinecone-io/cli/internal/pkg/utils/text" + "github.com/spf13/cobra" +) + +type DescribeCmdOptions struct { + reveal bool + json bool +} + +func NewDescribeCmd() *cobra.Command { + options := DescribeCmdOptions{} + + cmd := &cobra.Command{ + Use: "describe ", + Short: "Show detailed information about a configuration setting", + Example: help.Examples(` + pc config describe api-key + pc config describe environment + pc config describe color --json + `), + Args: cobra.ExactArgs(1), + ValidArgs: configKeyOrder, + Run: func(cmd *cobra.Command, args []string) { + keyName := args[0] + keyDesc, err := lookupKey(keyName) + if err != nil { + msg.FailMsg("%s", err) + exit.ErrorMsg(err.Error()) + return + } + + value := keyDesc.getStr() + if keyDesc.Sensitive && !options.reveal { + value = presenters.MaskHeadTail(value, 4, 4) + } + + if options.json { + type payload struct { + Key string `json:"key"` + Value string `json:"value"` + Description string `json:"description"` + LongDescription string `json:"long_description,omitempty"` + Sensitive bool `json:"sensitive"` + ValidValues []string `json:"valid_values,omitempty"` + } + fmt.Fprintln(os.Stdout, text.IndentJSON(payload{ + Key: keyName, + Value: value, + Description: keyDesc.Description, + LongDescription: keyDesc.LongDescription, + Sensitive: keyDesc.Sensitive, + ValidValues: keyDesc.ValidValues, + })) + return + } + + w := presenters.NewTabWriter() + fmt.Fprintf(w, "KEY\t%s\n", keyName) + fmt.Fprintf(w, "VALUE\t%s\n", displayValue(value)) + fmt.Fprintf(w, "SENSITIVE\t%s\n", text.BoolToString(keyDesc.Sensitive)) + if len(keyDesc.ValidValues) > 0 { + fmt.Fprintf(w, "VALID VALUES\t%s\n", strings.Join(keyDesc.ValidValues, ", ")) + } + fmt.Fprintf(w, "DESCRIPTION\t%s\n", keyDesc.Description) + w.Flush() + + if keyDesc.LongDescription != "" { + fmt.Fprintln(os.Stdout) + fmt.Fprintln(os.Stdout, keyDesc.LongDescription) + } + }, + } + + cmd.Flags().BoolVar(&options.reveal, "reveal", false, "Reveal the full value for sensitive settings like api-key") + cmd.Flags().BoolVarP(&options.json, "json", "j", false, "Output as JSON") + + return cmd +} diff --git a/internal/pkg/cli/command/config/get.go b/internal/pkg/cli/command/config/get.go new file mode 100644 index 00000000..697cd66e --- /dev/null +++ b/internal/pkg/cli/command/config/get.go @@ -0,0 +1,65 @@ +package config + +import ( + "fmt" + "os" + + "github.com/pinecone-io/cli/internal/pkg/utils/exit" + "github.com/pinecone-io/cli/internal/pkg/utils/help" + "github.com/pinecone-io/cli/internal/pkg/utils/msg" + "github.com/pinecone-io/cli/internal/pkg/utils/presenters" + "github.com/pinecone-io/cli/internal/pkg/utils/style" + "github.com/pinecone-io/cli/internal/pkg/utils/text" + "github.com/spf13/cobra" +) + +type GetCmdOptions struct { + reveal bool + json bool +} + +func NewGetCmd() *cobra.Command { + options := GetCmdOptions{} + + cmd := &cobra.Command{ + Use: "get ", + Short: "Get the current value of a configuration setting", + Example: help.Examples(` + pc config get api-key + pc config get api-key --reveal + pc config get environment + pc config get color + `), + Args: cobra.ExactArgs(1), + ValidArgs: configKeyOrder, + Run: func(cmd *cobra.Command, args []string) { + keyName := args[0] + keyDesc, err := lookupKey(keyName) + if err != nil { + msg.FailMsg("%s", err) + exit.ErrorMsg(err.Error()) + return + } + + value := keyDesc.getStr() + if keyDesc.Sensitive && !options.reveal { + value = presenters.MaskHeadTail(value, 4, 4) + } + + if options.json { + fmt.Fprintln(os.Stdout, text.IndentJSON(struct { + Key string `json:"key"` + Value string `json:"value"` + }{Key: keyName, Value: value})) + return + } + + msg.InfoMsg("%s: %s", style.Emphasis(keyName), displayValue(value)) + }, + } + + cmd.Flags().BoolVar(&options.reveal, "reveal", false, "Reveal the full value for sensitive settings like api-key") + cmd.Flags().BoolVarP(&options.json, "json", "j", false, "Output as JSON") + + return cmd +} diff --git a/internal/pkg/cli/command/config/get_api_key.go b/internal/pkg/cli/command/config/get_api_key.go index e2883227..7eba217a 100644 --- a/internal/pkg/cli/command/config/get_api_key.go +++ b/internal/pkg/cli/command/config/get_api_key.go @@ -21,8 +21,10 @@ func NewGetApiKeyCmd() *cobra.Command { options := GetAPIKeyCmdOptions{} cmd := &cobra.Command{ - Use: "get-api-key", - Short: "Get the current default API key configured for the Pinecone CLI", + Use: "get-api-key", + Short: "Get the current default API key configured for the Pinecone CLI", + Deprecated: "use 'pc config get api-key' instead", + Hidden: true, Example: help.Examples(` pc config get-api-key `), diff --git a/internal/pkg/cli/command/config/list.go b/internal/pkg/cli/command/config/list.go new file mode 100644 index 00000000..ef645a80 --- /dev/null +++ b/internal/pkg/cli/command/config/list.go @@ -0,0 +1,72 @@ +package config + +import ( + "fmt" + "os" + + "github.com/pinecone-io/cli/internal/pkg/utils/help" + "github.com/pinecone-io/cli/internal/pkg/utils/presenters" + "github.com/pinecone-io/cli/internal/pkg/utils/text" + "github.com/spf13/cobra" +) + +type ListCmdOptions struct { + reveal bool + json bool +} + +func NewListCmd() *cobra.Command { + options := ListCmdOptions{} + + cmd := &cobra.Command{ + Use: "list", + Short: "List all configuration settings and their current values", + Example: help.Examples(` + pc config list + pc config list --reveal + pc config list --json + `), + Args: cobra.NoArgs, + Run: func(cmd *cobra.Command, args []string) { + if options.json { + type entry struct { + Key string `json:"key"` + Value string `json:"value"` + Description string `json:"description"` + } + entries := make([]entry, 0, len(configKeyOrder)) + for _, keyName := range configKeyOrder { + keyDesc := configRegistry[keyName] + value := keyDesc.getStr() + if keyDesc.Sensitive && !options.reveal { + value = presenters.MaskHeadTail(value, 4, 4) + } + entries = append(entries, entry{ + Key: keyName, + Value: value, + Description: keyDesc.Description, + }) + } + fmt.Fprintln(os.Stdout, text.IndentJSON(entries)) + return + } + + w := presenters.NewTabWriter() + fmt.Fprintln(w, "KEY\tVALUE\tDESCRIPTION") + for _, keyName := range configKeyOrder { + keyDesc := configRegistry[keyName] + value := keyDesc.getStr() + if keyDesc.Sensitive && !options.reveal { + value = presenters.MaskHeadTail(value, 4, 4) + } + fmt.Fprintf(w, "%s\t%s\t%s\n", keyName, displayValue(value), keyDesc.Description) + } + w.Flush() + }, + } + + cmd.Flags().BoolVar(&options.reveal, "reveal", false, "Reveal the full value for sensitive settings like api-key") + cmd.Flags().BoolVarP(&options.json, "json", "j", false, "Output as JSON") + + return cmd +} diff --git a/internal/pkg/cli/command/config/registry.go b/internal/pkg/cli/command/config/registry.go new file mode 100644 index 00000000..381f5a0c --- /dev/null +++ b/internal/pkg/cli/command/config/registry.go @@ -0,0 +1,183 @@ +package config + +import ( + "context" + "errors" + "fmt" + "strings" + + conf "github.com/pinecone-io/cli/internal/pkg/utils/configuration/config" + "github.com/pinecone-io/cli/internal/pkg/utils/configuration/secrets" + "github.com/pinecone-io/cli/internal/pkg/utils/configuration/state" + "github.com/pinecone-io/cli/internal/pkg/utils/exit" + "github.com/pinecone-io/cli/internal/pkg/utils/help" + "github.com/pinecone-io/cli/internal/pkg/utils/msg" + "github.com/pinecone-io/cli/internal/pkg/utils/oauth" + "github.com/pinecone-io/cli/internal/pkg/utils/style" + "github.com/pinecone-io/cli/internal/pkg/utils/text" +) + +// ErrNoChange is returned by a keyDescriptor's setStr when the incoming value +// is equivalent to the current stored value and no write is needed. +var ErrNoChange = errors.New("no change") + +// keyDescriptor describes a single user-configurable setting. +type keyDescriptor struct { + Description string + LongDescription string // optional multi-paragraph detail shown by `pc config describe` + Sensitive bool + ValidValues []string // non-nil: values shown in help; nil: any non-empty string accepted + getStr func() string + setStr func(value string) error // returns ErrNoChange or a validation error + clearStr func() + // onChange is invoked after a successful setStr. It may call exit.Error for + // fatal side-effect failures. Returns human-readable info lines for the user. + onChange func(ctx context.Context, oldVal, newVal string) []string +} + +// configKeyOrder is the stable iteration order used by pc config list. +var configKeyOrder = []string{ + "api-key", + "environment", + "color", +} + +// configRegistry is a map of all config keys and their descriptors. +var configRegistry = map[string]keyDescriptor{ + "api-key": { + Description: "Default API key for authenticating with Pinecone", + LongDescription: help.Long(` + Configure the CLI to authenticate with Pinecone using an API key. + + When set, the API key takes priority over any target context established by + user login or service account credentials, and is used for all API calls. + + To clear the explicit API key, run 'pc config unset api-key'. + `), + Sensitive: true, + getStr: func() string { + return secrets.DefaultAPIKey.Get() + }, + setStr: func(value string) error { + if value == "" { + return fmt.Errorf("api-key value cannot be empty") + } + secrets.DefaultAPIKey.Set(value) + return nil + }, + clearStr: func() { + secrets.DefaultAPIKey.Clear() + }, + }, + + "environment": { + Description: "Pinecone environment to target (production or staging)", + LongDescription: help.Long(` + Select which Pinecone environment the CLI talks to. Most users should + leave this set to 'production'; 'staging' is intended for Pinecone + internal development. + + Changing the environment clears your existing authentication state: any + OAuth session is logged out, the default API key is cleared, and the + target organization and project are reset. You will need to re-authenticate + and re-target after switching. + `), + Sensitive: false, + ValidValues: []string{"production", "staging"}, + getStr: func() string { + return conf.Environment.Get() + }, + setStr: func(value string) error { + switch value { + case "production", "prod": + value = "production" + case "staging": + // already the canonical value + default: + return fmt.Errorf("invalid environment %q; must be one of: production, staging", value) + } + if conf.Environment.Get() == value { + return ErrNoChange + } + conf.Environment.Set(value) + return nil + }, + clearStr: func() { + conf.Environment.Clear() + }, + onChange: func(ctx context.Context, _, _ string) []string { + var lines []string + + token, err := oauth.Token(ctx) + if err != nil { + msg.FailMsg("Error retrieving oauth token: %s", err) + exit.Error(err, "error retrieving oauth token") + return nil // unreachable + } + if token != nil && (token.AccessToken != "" || token.RefreshToken != "") { + oauth.Logout() + lines = append(lines, fmt.Sprintf("You have been logged out; to login again, run %s", style.Code("pc login"))) + } else { + lines = append(lines, fmt.Sprintf("To login, run %s", style.Code("pc login"))) + } + + if secrets.DefaultAPIKey.Get() != "" { + secrets.DefaultAPIKey.Clear() + lines = append(lines, fmt.Sprintf("API key cleared; to set a new API key, run %s", style.Code("pc config set api-key "))) + } else { + lines = append(lines, fmt.Sprintf("To set a new API key, run %s", style.Code("pc config set api-key "))) + } + + if state.TargetOrg.Get().Name != "" || state.TargetProj.Get().Name != "" { + state.TargetOrg.Clear() + state.TargetProj.Clear() + lines = append(lines, fmt.Sprintf("Target organization and project cleared; to set a new target, run %s", style.Code("pc target -o myorg -p myproj"))) + } + + return lines + }, + }, + + "color": { + Description: "Enable or disable colored terminal output", + Sensitive: false, + ValidValues: []string{"true", "false"}, + getStr: func() string { + return text.BoolToString(conf.Color.Get()) + }, + setStr: func(value string) error { + var colorSetting bool + switch strings.ToLower(value) { + case "true", "on", "1": + colorSetting = true + case "false", "off", "0": + colorSetting = false + default: + return fmt.Errorf("invalid value %q for color; must be one of: true, false", value) + } + conf.Color.Set(colorSetting) + return nil + }, + clearStr: func() { + conf.Color.Clear() + }, + }, +} + +// lookupKey returns the descriptor for name, or a descriptive error listing valid keys. +func lookupKey(name string) (keyDescriptor, error) { + desc, ok := configRegistry[name] + if !ok { + return keyDescriptor{}, fmt.Errorf("unknown config key %q; valid keys are: %s", name, strings.Join(configKeyOrder, ", ")) + } + return desc, nil +} + +// displayValue formats a config value for human-readable output, substituting +// a placeholder when the value is empty. JSON output should use the raw value. +func displayValue(value string) string { + if value == "" { + return "" + } + return value +} diff --git a/internal/pkg/cli/command/config/set.go b/internal/pkg/cli/command/config/set.go new file mode 100644 index 00000000..36af22a2 --- /dev/null +++ b/internal/pkg/cli/command/config/set.go @@ -0,0 +1,62 @@ +package config + +import ( + "errors" + + "github.com/pinecone-io/cli/internal/pkg/utils/exit" + "github.com/pinecone-io/cli/internal/pkg/utils/help" + "github.com/pinecone-io/cli/internal/pkg/utils/msg" + "github.com/pinecone-io/cli/internal/pkg/utils/style" + "github.com/spf13/cobra" +) + +func NewSetCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "set ", + Short: "Set a configuration value", + Example: help.Examples(` + pc config set api-key pcsk_... + pc config set environment staging + pc config set color false + `), + Args: cobra.ExactArgs(2), + ValidArgsFunction: func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { + if len(args) == 0 { + return configKeyOrder, cobra.ShellCompDirectiveNoFileComp + } + return nil, cobra.ShellCompDirectiveNoFileComp + }, + Run: func(cmd *cobra.Command, args []string) { + keyName, value := args[0], args[1] + + keyDesc, err := lookupKey(keyName) + if err != nil { + msg.FailMsg("%s", err) + exit.ErrorMsg(err.Error()) + return + } + + oldVal := keyDesc.getStr() + + if err := keyDesc.setStr(value); err != nil { + if errors.Is(err, ErrNoChange) { + msg.InfoMsg("%s is already set to %s", style.Emphasis(keyName), style.Emphasis(oldVal)) + return + } + msg.FailMsg("%s", err) + exit.ErrorMsg(err.Error()) + return + } + + msg.SuccessMsg("%s updated", style.Emphasis(keyName)) + + if keyDesc.onChange != nil { + for _, line := range keyDesc.onChange(cmd.Context(), oldVal, value) { + msg.InfoMsg("%s", line) + } + } + }, + } + + return cmd +} diff --git a/internal/pkg/cli/command/config/set_api_key.go b/internal/pkg/cli/command/config/set_api_key.go index 95737a6a..52e61fb5 100644 --- a/internal/pkg/cli/command/config/set_api_key.go +++ b/internal/pkg/cli/command/config/set_api_key.go @@ -20,9 +20,11 @@ var ( func NewSetApiKeyCmd() *cobra.Command { cmd := &cobra.Command{ - Use: "set-api-key", - Short: "Configure the CLI to authenticate with Pinecone using a default API key", - Long: setAPIKeyHelp, + Use: "set-api-key", + Short: "Configure the CLI to authenticate with Pinecone using a default API key", + Long: setAPIKeyHelp, + Deprecated: "use 'pc config set api-key ' instead", + Hidden: true, Example: help.Examples(` pc config set-api-key "api-key-value" `), diff --git a/internal/pkg/cli/command/config/set_color.go b/internal/pkg/cli/command/config/set_color.go index c63bb5c0..28069d4c 100644 --- a/internal/pkg/cli/command/config/set_color.go +++ b/internal/pkg/cli/command/config/set_color.go @@ -12,8 +12,10 @@ import ( func NewSetColorCmd() *cobra.Command { cmd := &cobra.Command{ - Use: "set-color", - Short: "Configure whether the CLI prints output with color", + Use: "set-color", + Short: "Configure whether the CLI prints output with color", + Deprecated: "use 'pc config set color ' instead", + Hidden: true, Example: help.Examples(` pc config set-color true pc config set-color false diff --git a/internal/pkg/cli/command/config/set_environment.go b/internal/pkg/cli/command/config/set_environment.go index 6ccc80c0..b992a75c 100644 --- a/internal/pkg/cli/command/config/set_environment.go +++ b/internal/pkg/cli/command/config/set_environment.go @@ -15,13 +15,14 @@ import ( func NewSetEnvCmd() *cobra.Command { cmd := &cobra.Command{ - Use: "set-environment ", - Short: "Configure the environment (production or staging)", + Use: "set-environment ", + Short: "Configure the environment (production or staging)", + Deprecated: "use 'pc config set environment ' instead", + Hidden: true, Example: help.Examples(` pc config set-environment "production" pc config set-environment "staging" `), - Hidden: false, Run: func(cmd *cobra.Command, args []string) { if len(args) == 0 { msg.FailMsg("Please provide a value for environment. Accepted values are %s, %s", style.Emphasis("production"), style.Emphasis("staging")) @@ -64,9 +65,9 @@ func NewSetEnvCmd() *cobra.Command { if secrets.DefaultAPIKey.Get() != "" { secrets.DefaultAPIKey.Clear() - msg.InfoMsg("API key cleared; to set a new API key, run %s", style.Code("pc config set-api-key")) + msg.InfoMsg("API key cleared; to set a new API key, run %s", style.Code("pc config set api-key ")) } else { - msg.InfoMsg("To set a new API key, run %s", style.Code("pc config set-api-key")) + msg.InfoMsg("To set a new API key, run %s", style.Code("pc config set api-key ")) } if state.TargetOrg.Get().Name != "" || state.TargetProj.Get().Name != "" { diff --git a/internal/pkg/cli/command/config/unset.go b/internal/pkg/cli/command/config/unset.go new file mode 100644 index 00000000..592e34c3 --- /dev/null +++ b/internal/pkg/cli/command/config/unset.go @@ -0,0 +1,36 @@ +package config + +import ( + "github.com/pinecone-io/cli/internal/pkg/utils/exit" + "github.com/pinecone-io/cli/internal/pkg/utils/help" + "github.com/pinecone-io/cli/internal/pkg/utils/msg" + "github.com/pinecone-io/cli/internal/pkg/utils/style" + "github.com/spf13/cobra" +) + +func NewUnsetCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "unset ", + Short: "Reset a configuration value to its default", + Example: help.Examples(` + pc config unset api-key + pc config unset color + `), + Args: cobra.ExactArgs(1), + ValidArgs: configKeyOrder, + Run: func(cmd *cobra.Command, args []string) { + keyName := args[0] + keyDesc, err := lookupKey(keyName) + if err != nil { + msg.FailMsg("%s", err) + exit.ErrorMsg(err.Error()) + return + } + + keyDesc.clearStr() + msg.SuccessMsg("%s cleared", style.Emphasis(keyName)) + }, + } + + return cmd +} diff --git a/internal/pkg/cli/command/root/root.go b/internal/pkg/cli/command/root/root.go index a05007ce..481bb647 100644 --- a/internal/pkg/cli/command/root/root.go +++ b/internal/pkg/cli/command/root/root.go @@ -50,6 +50,11 @@ var skipAuthCommands = map[string]struct{}{ "pc target": {}, // handles its own auth after --show/--clear early returns "pc version": {}, "pc config": {}, + "pc config get": {}, + "pc config set": {}, + "pc config unset": {}, + "pc config list": {}, + "pc config describe": {}, "pc config get-api-key": {}, "pc config set-api-key": {}, "pc config set-color": {}, From 00eb59a4c1f040013de1845d7ba182adc43a5af6 Mon Sep 17 00:00:00 2001 From: Austin DeNoble Date: Thu, 28 May 2026 14:20:54 -0400 Subject: [PATCH 02/17] Fix onChange contract in `pc config set` and `unset` `pc config unset` now invokes the onChange hook when clearing a key changes its value, fixing a bug where `unset environment` on staging silently resets to production without clearing the staging-tied OAuth session, API key, or target org/project. `pc config set` now passes the canonical stored value to onChange via a post-set getStr() read, rather than the raw user input. Hooks like environment's now see "production" instead of "prod" when the input is normalized during setStr. --- internal/pkg/cli/command/config/set.go | 3 ++- internal/pkg/cli/command/config/unset.go | 8 ++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/internal/pkg/cli/command/config/set.go b/internal/pkg/cli/command/config/set.go index 36af22a2..07d56b08 100644 --- a/internal/pkg/cli/command/config/set.go +++ b/internal/pkg/cli/command/config/set.go @@ -48,10 +48,11 @@ func NewSetCmd() *cobra.Command { return } + newVal := keyDesc.getStr() msg.SuccessMsg("%s updated", style.Emphasis(keyName)) if keyDesc.onChange != nil { - for _, line := range keyDesc.onChange(cmd.Context(), oldVal, value) { + for _, line := range keyDesc.onChange(cmd.Context(), oldVal, newVal) { msg.InfoMsg("%s", line) } } diff --git a/internal/pkg/cli/command/config/unset.go b/internal/pkg/cli/command/config/unset.go index 592e34c3..b8d982f2 100644 --- a/internal/pkg/cli/command/config/unset.go +++ b/internal/pkg/cli/command/config/unset.go @@ -27,8 +27,16 @@ func NewUnsetCmd() *cobra.Command { return } + oldVal := keyDesc.getStr() keyDesc.clearStr() + newVal := keyDesc.getStr() msg.SuccessMsg("%s cleared", style.Emphasis(keyName)) + + if keyDesc.onChange != nil && oldVal != newVal { + for _, line := range keyDesc.onChange(cmd.Context(), oldVal, newVal) { + msg.InfoMsg("%s", line) + } + } }, } From 62e6b692d89ac8d0b047ad5ba16f83f6084f7180 Mon Sep 17 00:00:00 2001 From: Austin DeNoble Date: Sun, 31 May 2026 16:57:03 -0400 Subject: [PATCH 03/17] Refactor pc config commands for testability and add unit tests Extract business logic from cobra Run blocks into run* functions that accept a ConfigService interface, enabling unit testing without touching the real viper config store. Add --json output to set and unset commands for machine-readable use. Add a Hidden field to keyDescriptor to suppress internal keys (environment) from list output and tab completion while keeping them accessible by name. --- .../pkg/cli/command/config/config_test.go | 58 ++++++ internal/pkg/cli/command/config/describe.go | 99 +++++----- .../pkg/cli/command/config/describe_test.go | 93 +++++++++ internal/pkg/cli/command/config/get.go | 48 +++-- internal/pkg/cli/command/config/get_test.go | 61 ++++++ internal/pkg/cli/command/config/list.go | 75 ++++---- internal/pkg/cli/command/config/list_test.go | 87 +++++++++ internal/pkg/cli/command/config/registry.go | 178 +++++++++++++++--- .../pkg/cli/command/config/registry_test.go | 54 ++++++ internal/pkg/cli/command/config/set.go | 76 +++++--- internal/pkg/cli/command/config/set_test.go | 93 +++++++++ internal/pkg/cli/command/config/unset.go | 58 ++++-- internal/pkg/cli/command/config/unset_test.go | 49 +++++ 13 files changed, 859 insertions(+), 170 deletions(-) create mode 100644 internal/pkg/cli/command/config/config_test.go create mode 100644 internal/pkg/cli/command/config/describe_test.go create mode 100644 internal/pkg/cli/command/config/get_test.go create mode 100644 internal/pkg/cli/command/config/list_test.go create mode 100644 internal/pkg/cli/command/config/registry_test.go create mode 100644 internal/pkg/cli/command/config/set_test.go create mode 100644 internal/pkg/cli/command/config/unset_test.go diff --git a/internal/pkg/cli/command/config/config_test.go b/internal/pkg/cli/command/config/config_test.go new file mode 100644 index 00000000..a3da8fc1 --- /dev/null +++ b/internal/pkg/cli/command/config/config_test.go @@ -0,0 +1,58 @@ +package config + +import "context" + +// mockConfigService implements ConfigService for unit tests. +// Each field controls what the corresponding method returns. +// The last* fields record the arguments of the most recent call. +type mockConfigService struct { + // Get + getValue string + getSensitive bool + getErr error + lastGetKey string + + // Set + setLines []string + setErr error + lastSetKey string + lastSetValue string + + // Unset + unsetLines []string + unsetErr error + lastUnsetKey string + + // List + listResult []ConfigEntry + + // Describe + describeResult ConfigDescription + describeErr error + lastDescribeKey string +} + +func (m *mockConfigService) Get(key string) (string, bool, error) { + m.lastGetKey = key + return m.getValue, m.getSensitive, m.getErr +} + +func (m *mockConfigService) Set(ctx context.Context, key, value string) ([]string, error) { + m.lastSetKey = key + m.lastSetValue = value + return m.setLines, m.setErr +} + +func (m *mockConfigService) Unset(ctx context.Context, key string) ([]string, error) { + m.lastUnsetKey = key + return m.unsetLines, m.unsetErr +} + +func (m *mockConfigService) List() []ConfigEntry { + return m.listResult +} + +func (m *mockConfigService) Describe(key string) (ConfigDescription, error) { + m.lastDescribeKey = key + return m.describeResult, m.describeErr +} diff --git a/internal/pkg/cli/command/config/describe.go b/internal/pkg/cli/command/config/describe.go index b1cf16b7..37ee317f 100644 --- a/internal/pkg/cli/command/config/describe.go +++ b/internal/pkg/cli/command/config/describe.go @@ -30,54 +30,12 @@ func NewDescribeCmd() *cobra.Command { pc config describe color --json `), Args: cobra.ExactArgs(1), - ValidArgs: configKeyOrder, + ValidArgs: visibleKeys(), Run: func(cmd *cobra.Command, args []string) { - keyName := args[0] - keyDesc, err := lookupKey(keyName) - if err != nil { + svc := newDefaultConfigService() + if err := runDescribeCmd(svc, args[0], options); err != nil { msg.FailMsg("%s", err) exit.ErrorMsg(err.Error()) - return - } - - value := keyDesc.getStr() - if keyDesc.Sensitive && !options.reveal { - value = presenters.MaskHeadTail(value, 4, 4) - } - - if options.json { - type payload struct { - Key string `json:"key"` - Value string `json:"value"` - Description string `json:"description"` - LongDescription string `json:"long_description,omitempty"` - Sensitive bool `json:"sensitive"` - ValidValues []string `json:"valid_values,omitempty"` - } - fmt.Fprintln(os.Stdout, text.IndentJSON(payload{ - Key: keyName, - Value: value, - Description: keyDesc.Description, - LongDescription: keyDesc.LongDescription, - Sensitive: keyDesc.Sensitive, - ValidValues: keyDesc.ValidValues, - })) - return - } - - w := presenters.NewTabWriter() - fmt.Fprintf(w, "KEY\t%s\n", keyName) - fmt.Fprintf(w, "VALUE\t%s\n", displayValue(value)) - fmt.Fprintf(w, "SENSITIVE\t%s\n", text.BoolToString(keyDesc.Sensitive)) - if len(keyDesc.ValidValues) > 0 { - fmt.Fprintf(w, "VALID VALUES\t%s\n", strings.Join(keyDesc.ValidValues, ", ")) - } - fmt.Fprintf(w, "DESCRIPTION\t%s\n", keyDesc.Description) - w.Flush() - - if keyDesc.LongDescription != "" { - fmt.Fprintln(os.Stdout) - fmt.Fprintln(os.Stdout, keyDesc.LongDescription) } }, } @@ -87,3 +45,54 @@ func NewDescribeCmd() *cobra.Command { return cmd } + +func runDescribeCmd(svc ConfigService, keyName string, opts DescribeCmdOptions) error { + // --json output for the describe command + type describeOutput struct { + Key string `json:"key"` + Value string `json:"value"` + Description string `json:"description"` + LongDescription string `json:"long_description,omitempty"` + Sensitive bool `json:"sensitive"` + ValidValues []string `json:"valid_values,omitempty"` + } + + desc, err := svc.Describe(keyName) + if err != nil { + return err + } + + value := desc.Value + if desc.Sensitive && !opts.reveal { + value = presenters.MaskHeadTail(value, 4, 4) + } + + if opts.json { + fmt.Fprintln(os.Stdout, text.IndentJSON(describeOutput{ + Key: desc.Key, + Value: value, + Description: desc.Description, + LongDescription: desc.LongDescription, + Sensitive: desc.Sensitive, + ValidValues: desc.ValidValues, + })) + return nil + } + + w := presenters.NewTabWriter() + fmt.Fprintf(w, "KEY\t%s\n", desc.Key) + fmt.Fprintf(w, "VALUE\t%s\n", displayValue(value)) + fmt.Fprintf(w, "SENSITIVE\t%s\n", text.BoolToString(desc.Sensitive)) + if len(desc.ValidValues) > 0 { + fmt.Fprintf(w, "VALID VALUES\t%s\n", strings.Join(desc.ValidValues, ", ")) + } + fmt.Fprintf(w, "DESCRIPTION\t%s\n", desc.Description) + w.Flush() + + if desc.LongDescription != "" { + fmt.Fprintln(os.Stdout) + fmt.Fprintln(os.Stdout, desc.LongDescription) + } + + return nil +} diff --git a/internal/pkg/cli/command/config/describe_test.go b/internal/pkg/cli/command/config/describe_test.go new file mode 100644 index 00000000..a31937f9 --- /dev/null +++ b/internal/pkg/cli/command/config/describe_test.go @@ -0,0 +1,93 @@ +package config + +import ( + "errors" + "testing" + + "github.com/pinecone-io/cli/internal/pkg/cli/testutils" + "github.com/stretchr/testify/assert" +) + +func Test_runDescribeCmd_ReturnsErrorOnUnknownKey(t *testing.T) { + svc := &mockConfigService{describeErr: errors.New("unknown config key")} + + err := runDescribeCmd(svc, "bad-key", DescribeCmdOptions{}) + + assert.Error(t, err) + assert.Equal(t, "bad-key", svc.lastDescribeKey) +} + +func Test_runDescribeCmd_TabularOutput(t *testing.T) { + svc := &mockConfigService{ + describeResult: ConfigDescription{ + Key: "environment", + Value: "production", + Description: "Pinecone environment", + Sensitive: false, + ValidValues: []string{"production", "staging"}, + }, + } + + out := testutils.CaptureStdout(t, func() { + err := runDescribeCmd(svc, "environment", DescribeCmdOptions{}) + assert.NoError(t, err) + }) + + assert.Contains(t, out, "environment") + assert.Contains(t, out, "production") +} + +func Test_runDescribeCmd_JSONOutput(t *testing.T) { + svc := &mockConfigService{ + describeResult: ConfigDescription{ + Key: "environment", + Value: "production", + Description: "Pinecone environment", + Sensitive: false, + ValidValues: []string{"production", "staging"}, + }, + } + + out := testutils.CaptureStdout(t, func() { + err := runDescribeCmd(svc, "environment", DescribeCmdOptions{json: true}) + assert.NoError(t, err) + }) + + assert.Contains(t, out, `"environment"`) + assert.Contains(t, out, `"production"`) + assert.Contains(t, out, `"valid_values"`) +} + +func Test_runDescribeCmd_MasksSensitiveKeyInJSON(t *testing.T) { + svc := &mockConfigService{ + describeResult: ConfigDescription{ + Key: "api-key", + Value: "supersecretvalue", + Sensitive: true, + }, + } + + out := testutils.CaptureStdout(t, func() { + err := runDescribeCmd(svc, "api-key", DescribeCmdOptions{json: true, reveal: false}) + assert.NoError(t, err) + }) + + assert.NotContains(t, out, "supersecretvalue") +} + +func Test_runDescribeCmd_RevealsSensitiveKeyInJSON(t *testing.T) { + svc := &mockConfigService{ + describeResult: ConfigDescription{ + Key: "api-key", + Value: "supersecretvalue", + Sensitive: true, + }, + } + + out := testutils.CaptureStdout(t, func() { + err := runDescribeCmd(svc, "api-key", DescribeCmdOptions{json: true, reveal: true}) + assert.NoError(t, err) + }) + + assert.Contains(t, out, "supersecretvalue") +} diff --git a/internal/pkg/cli/command/config/get.go b/internal/pkg/cli/command/config/get.go index 697cd66e..6ab79983 100644 --- a/internal/pkg/cli/command/config/get.go +++ b/internal/pkg/cli/command/config/get.go @@ -31,30 +31,13 @@ func NewGetCmd() *cobra.Command { pc config get color `), Args: cobra.ExactArgs(1), - ValidArgs: configKeyOrder, + ValidArgs: visibleKeys(), Run: func(cmd *cobra.Command, args []string) { - keyName := args[0] - keyDesc, err := lookupKey(keyName) - if err != nil { + svc := newDefaultConfigService() + if err := runGetCmd(svc, args[0], options); err != nil { msg.FailMsg("%s", err) exit.ErrorMsg(err.Error()) - return } - - value := keyDesc.getStr() - if keyDesc.Sensitive && !options.reveal { - value = presenters.MaskHeadTail(value, 4, 4) - } - - if options.json { - fmt.Fprintln(os.Stdout, text.IndentJSON(struct { - Key string `json:"key"` - Value string `json:"value"` - }{Key: keyName, Value: value})) - return - } - - msg.InfoMsg("%s: %s", style.Emphasis(keyName), displayValue(value)) }, } @@ -63,3 +46,28 @@ func NewGetCmd() *cobra.Command { return cmd } + +func runGetCmd(svc ConfigService, keyName string, opts GetCmdOptions) error { + // --json output for the get command + type getOutput struct { + Key string `json:"key"` + Value string `json:"value"` + } + + value, sensitive, err := svc.Get(keyName) + if err != nil { + return err + } + + if sensitive && !opts.reveal { + value = presenters.MaskHeadTail(value, 4, 4) + } + + if opts.json { + fmt.Fprintln(os.Stdout, text.IndentJSON(getOutput{Key: keyName, Value: value})) + return nil + } + + msg.InfoMsg("%s: %s", style.Emphasis(keyName), displayValue(value)) + return nil +} diff --git a/internal/pkg/cli/command/config/get_test.go b/internal/pkg/cli/command/config/get_test.go new file mode 100644 index 00000000..0a89f6ce --- /dev/null +++ b/internal/pkg/cli/command/config/get_test.go @@ -0,0 +1,61 @@ +package config + +import ( + "errors" + "testing" + + "github.com/pinecone-io/cli/internal/pkg/cli/testutils" + "github.com/stretchr/testify/assert" +) + +func Test_runGetCmd_ReturnsErrorOnUnknownKey(t *testing.T) { + svc := &mockConfigService{getErr: errors.New("unknown config key")} + + err := runGetCmd(svc, "bad-key", GetCmdOptions{}) + + assert.Error(t, err) + assert.Equal(t, "bad-key", svc.lastGetKey) +} + +func Test_runGetCmd_Succeeds(t *testing.T) { + svc := &mockConfigService{getValue: "production"} + + err := runGetCmd(svc, "environment", GetCmdOptions{}) + + assert.NoError(t, err) + assert.Equal(t, "environment", svc.lastGetKey) +} + +func Test_runGetCmd_JSONOutput(t *testing.T) { + svc := &mockConfigService{getValue: "production"} + + out := testutils.CaptureStdout(t, func() { + err := runGetCmd(svc, "environment", GetCmdOptions{json: true}) + assert.NoError(t, err) + }) + + assert.Contains(t, out, `"environment"`) + assert.Contains(t, out, `"production"`) +} + +func Test_runGetCmd_MasksSensitiveKeyInJSON(t *testing.T) { + svc := &mockConfigService{getValue: "supersecretvalue", getSensitive: true} + + out := testutils.CaptureStdout(t, func() { + err := runGetCmd(svc, "api-key", GetCmdOptions{json: true, reveal: false}) + assert.NoError(t, err) + }) + + assert.NotContains(t, out, "supersecretvalue") +} + +func Test_runGetCmd_RevealsSensitiveKeyInJSON(t *testing.T) { + svc := &mockConfigService{getValue: "supersecretvalue", getSensitive: true} + + out := testutils.CaptureStdout(t, func() { + err := runGetCmd(svc, "api-key", GetCmdOptions{json: true, reveal: true}) + assert.NoError(t, err) + }) + + assert.Contains(t, out, "supersecretvalue") +} diff --git a/internal/pkg/cli/command/config/list.go b/internal/pkg/cli/command/config/list.go index ef645a80..f1fa7bca 100644 --- a/internal/pkg/cli/command/config/list.go +++ b/internal/pkg/cli/command/config/list.go @@ -4,7 +4,9 @@ import ( "fmt" "os" + "github.com/pinecone-io/cli/internal/pkg/utils/exit" "github.com/pinecone-io/cli/internal/pkg/utils/help" + "github.com/pinecone-io/cli/internal/pkg/utils/msg" "github.com/pinecone-io/cli/internal/pkg/utils/presenters" "github.com/pinecone-io/cli/internal/pkg/utils/text" "github.com/spf13/cobra" @@ -28,40 +30,11 @@ func NewListCmd() *cobra.Command { `), Args: cobra.NoArgs, Run: func(cmd *cobra.Command, args []string) { - if options.json { - type entry struct { - Key string `json:"key"` - Value string `json:"value"` - Description string `json:"description"` - } - entries := make([]entry, 0, len(configKeyOrder)) - for _, keyName := range configKeyOrder { - keyDesc := configRegistry[keyName] - value := keyDesc.getStr() - if keyDesc.Sensitive && !options.reveal { - value = presenters.MaskHeadTail(value, 4, 4) - } - entries = append(entries, entry{ - Key: keyName, - Value: value, - Description: keyDesc.Description, - }) - } - fmt.Fprintln(os.Stdout, text.IndentJSON(entries)) - return + svc := newDefaultConfigService() + if err := runListCmd(svc, options); err != nil { + msg.FailMsg("%s", err) + exit.ErrorMsg(err.Error()) } - - w := presenters.NewTabWriter() - fmt.Fprintln(w, "KEY\tVALUE\tDESCRIPTION") - for _, keyName := range configKeyOrder { - keyDesc := configRegistry[keyName] - value := keyDesc.getStr() - if keyDesc.Sensitive && !options.reveal { - value = presenters.MaskHeadTail(value, 4, 4) - } - fmt.Fprintf(w, "%s\t%s\t%s\n", keyName, displayValue(value), keyDesc.Description) - } - w.Flush() }, } @@ -70,3 +43,39 @@ func NewListCmd() *cobra.Command { return cmd } + +func runListCmd(svc ConfigService, opts ListCmdOptions) error { + // --json output for the list command + type listOutput struct { + Key string `json:"key"` + Value string `json:"value"` + Description string `json:"description"` + } + + entries := svc.List() + + if opts.json { + jsonEntries := make([]listOutput, 0, len(entries)) + for _, e := range entries { + value := e.Value + if e.Sensitive && !opts.reveal { + value = presenters.MaskHeadTail(value, 4, 4) + } + jsonEntries = append(jsonEntries, listOutput{Key: e.Key, Value: value, Description: e.Description}) + } + fmt.Fprintln(os.Stdout, text.IndentJSON(jsonEntries)) + return nil + } + + w := presenters.NewTabWriter() + fmt.Fprintln(w, "KEY\tVALUE\tDESCRIPTION") + for _, e := range entries { + value := e.Value + if e.Sensitive && !opts.reveal { + value = presenters.MaskHeadTail(value, 4, 4) + } + fmt.Fprintf(w, "%s\t%s\t%s\n", e.Key, displayValue(value), e.Description) + } + w.Flush() + return nil +} diff --git a/internal/pkg/cli/command/config/list_test.go b/internal/pkg/cli/command/config/list_test.go new file mode 100644 index 00000000..423db539 --- /dev/null +++ b/internal/pkg/cli/command/config/list_test.go @@ -0,0 +1,87 @@ +package config + +import ( + "testing" + + "github.com/pinecone-io/cli/internal/pkg/cli/testutils" + "github.com/stretchr/testify/assert" +) + +func Test_runListCmd_TabularOutputIncludesHeader(t *testing.T) { + svc := &mockConfigService{listResult: []ConfigEntry{}} + + out := testutils.CaptureStdout(t, func() { + err := runListCmd(svc, ListCmdOptions{}) + assert.NoError(t, err) + }) + + assert.Contains(t, out, "KEY") + assert.Contains(t, out, "VALUE") + assert.Contains(t, out, "DESCRIPTION") +} + +func Test_runListCmd_TabularOutputMasksSensitiveKey(t *testing.T) { + svc := &mockConfigService{ + listResult: []ConfigEntry{ + {Key: "api-key", Value: "sk-supersecret", Description: "API key", Sensitive: true}, + }, + } + + out := testutils.CaptureStdout(t, func() { + err := runListCmd(svc, ListCmdOptions{}) + assert.NoError(t, err) + }) + + assert.Contains(t, out, "api-key") + assert.NotContains(t, out, "sk-supersecret") +} + +func Test_runListCmd_TabularOutputRevealsSensitiveKey(t *testing.T) { + svc := &mockConfigService{ + listResult: []ConfigEntry{ + {Key: "api-key", Value: "sk-supersecret", Sensitive: true}, + }, + } + + out := testutils.CaptureStdout(t, func() { + err := runListCmd(svc, ListCmdOptions{reveal: true}) + assert.NoError(t, err) + }) + + assert.Contains(t, out, "sk-supersecret") +} + +func Test_runListCmd_JSONOutput(t *testing.T) { + svc := &mockConfigService{ + listResult: []ConfigEntry{ + {Key: "api-key", Value: "sk-supersecret", Description: "API key", Sensitive: true}, + {Key: "color", Value: "true", Description: "Color output", Sensitive: false}, + }, + } + + out := testutils.CaptureStdout(t, func() { + err := runListCmd(svc, ListCmdOptions{json: true}) + assert.NoError(t, err) + }) + + // Sensitive key should be masked in JSON output + assert.NotContains(t, out, "sk-supersecret") + // Non-sensitive values should appear + assert.Contains(t, out, `"color"`) + assert.Contains(t, out, `"true"`) +} + +func Test_runListCmd_JSONOutputRevealsSensitiveKey(t *testing.T) { + svc := &mockConfigService{ + listResult: []ConfigEntry{ + {Key: "api-key", Value: "sk-supersecret", Sensitive: true}, + }, + } + + out := testutils.CaptureStdout(t, func() { + err := runListCmd(svc, ListCmdOptions{json: true, reveal: true}) + assert.NoError(t, err) + }) + + assert.Contains(t, out, "sk-supersecret") +} diff --git a/internal/pkg/cli/command/config/registry.go b/internal/pkg/cli/command/config/registry.go index 381f5a0c..1f825ff5 100644 --- a/internal/pkg/cli/command/config/registry.go +++ b/internal/pkg/cli/command/config/registry.go @@ -26,6 +26,7 @@ type keyDescriptor struct { Description string LongDescription string // optional multi-paragraph detail shown by `pc config describe` Sensitive bool + Hidden bool ValidValues []string // non-nil: values shown in help; nil: any non-empty string accepted getStr func() string setStr func(value string) error // returns ErrNoChange or a validation error @@ -35,11 +36,13 @@ type keyDescriptor struct { onChange func(ctx context.Context, oldVal, newVal string) []string } -// configKeyOrder is the stable iteration order used by pc config list. -var configKeyOrder = []string{ +// configKeys represent the set of valid configuration keys. +// This is used by lookupKey to validate keys on config commands, +// and the order of keys in the list command. +var configKeys = []string{ "api-key", - "environment", "color", + "environment", } // configRegistry is a map of all config keys and their descriptors. @@ -55,6 +58,7 @@ var configRegistry = map[string]keyDescriptor{ To clear the explicit API key, run 'pc config unset api-key'. `), Sensitive: true, + Hidden: false, getStr: func() string { return secrets.DefaultAPIKey.Get() }, @@ -69,7 +73,31 @@ var configRegistry = map[string]keyDescriptor{ secrets.DefaultAPIKey.Clear() }, }, - + "color": { + Description: "Enable or disable colored terminal output", + Sensitive: false, + Hidden: false, + ValidValues: []string{"true", "false"}, + getStr: func() string { + return text.BoolToString(conf.Color.Get()) + }, + setStr: func(value string) error { + var colorSetting bool + switch strings.ToLower(value) { + case "true", "on", "1": + colorSetting = true + case "false", "off", "0": + colorSetting = false + default: + return fmt.Errorf("invalid value %q for color; must be one of: true, false", value) + } + conf.Color.Set(colorSetting) + return nil + }, + clearStr: func() { + conf.Color.Clear() + }, + }, "environment": { Description: "Pinecone environment to target (production or staging)", LongDescription: help.Long(` @@ -83,6 +111,7 @@ var configRegistry = map[string]keyDescriptor{ and re-target after switching. `), Sensitive: false, + Hidden: true, ValidValues: []string{"production", "staging"}, getStr: func() string { return conf.Environment.Get() @@ -137,42 +166,28 @@ var configRegistry = map[string]keyDescriptor{ return lines }, }, - - "color": { - Description: "Enable or disable colored terminal output", - Sensitive: false, - ValidValues: []string{"true", "false"}, - getStr: func() string { - return text.BoolToString(conf.Color.Get()) - }, - setStr: func(value string) error { - var colorSetting bool - switch strings.ToLower(value) { - case "true", "on", "1": - colorSetting = true - case "false", "off", "0": - colorSetting = false - default: - return fmt.Errorf("invalid value %q for color; must be one of: true, false", value) - } - conf.Color.Set(colorSetting) - return nil - }, - clearStr: func() { - conf.Color.Clear() - }, - }, } // lookupKey returns the descriptor for name, or a descriptive error listing valid keys. func lookupKey(name string) (keyDescriptor, error) { desc, ok := configRegistry[name] if !ok { - return keyDescriptor{}, fmt.Errorf("unknown config key %q; valid keys are: %s", name, strings.Join(configKeyOrder, ", ")) + return keyDescriptor{}, fmt.Errorf("unknown config key %q; valid keys are: %s", name, strings.Join(configKeys, ", ")) } return desc, nil } +// visibleKeys returns the set of config keys that are surfaced to the user. +func visibleKeys() []string { + keys := make([]string, 0, len(configRegistry)) + for key, desc := range configRegistry { + if !desc.Hidden { + keys = append(keys, key) + } + } + return keys +} + // displayValue formats a config value for human-readable output, substituting // a placeholder when the value is empty. JSON output should use the raw value. func displayValue(value string) string { @@ -181,3 +196,106 @@ func displayValue(value string) string { } return value } + +// ConfigEntry holds the key, value, and metadata for a single config setting, +// used by the list command. +type ConfigEntry struct { + Key string + Value string + Description string + Sensitive bool +} + +// ConfigDescription holds full metadata for a config key, used by the describe command. +type ConfigDescription struct { + Key string + Value string + Description string + LongDescription string + Sensitive bool + ValidValues []string +} + +// ConfigService abstracts config registry operations for unit testing across +// the get, set, unset, list, and describe commands. +type ConfigService interface { + Get(key string) (value string, sensitive bool, err error) + Set(ctx context.Context, key, value string) (onChangeLines []string, err error) + Unset(ctx context.Context, key string) (onChangeLines []string, err error) + List() []ConfigEntry + Describe(key string) (ConfigDescription, error) +} + +type defaultConfigService struct{} + +func newDefaultConfigService() ConfigService { + return &defaultConfigService{} +} + +func (s *defaultConfigService) Get(key string) (string, bool, error) { + desc, err := lookupKey(key) + if err != nil { + return "", false, err + } + return desc.getStr(), desc.Sensitive, nil +} + +func (s *defaultConfigService) Set(ctx context.Context, key, value string) ([]string, error) { + desc, err := lookupKey(key) + if err != nil { + return nil, err + } + oldVal := desc.getStr() + if err := desc.setStr(value); err != nil { + return nil, err + } + newVal := desc.getStr() + if desc.onChange != nil { + return desc.onChange(ctx, oldVal, newVal), nil + } + return nil, nil +} + +func (s *defaultConfigService) Unset(ctx context.Context, key string) ([]string, error) { + desc, err := lookupKey(key) + if err != nil { + return nil, err + } + oldVal := desc.getStr() + desc.clearStr() + newVal := desc.getStr() + if desc.onChange != nil && oldVal != newVal { + return desc.onChange(ctx, oldVal, newVal), nil + } + return nil, nil +} + +func (s *defaultConfigService) List() []ConfigEntry { + visibleKeys := visibleKeys() + entries := make([]ConfigEntry, 0, len(visibleKeys)) + for _, key := range visibleKeys { + desc := configRegistry[key] + entries = append(entries, ConfigEntry{ + Key: key, + Value: desc.getStr(), + Description: desc.Description, + Sensitive: desc.Sensitive, + }) + } + return entries +} + +func (s *defaultConfigService) Describe(key string) (ConfigDescription, error) { + desc, err := lookupKey(key) + if err != nil { + return ConfigDescription{}, err + } + return ConfigDescription{ + Key: key, + Value: desc.getStr(), + Description: desc.Description, + LongDescription: desc.LongDescription, + Sensitive: desc.Sensitive, + ValidValues: desc.ValidValues, + }, nil +} diff --git a/internal/pkg/cli/command/config/registry_test.go b/internal/pkg/cli/command/config/registry_test.go new file mode 100644 index 00000000..75d2d23e --- /dev/null +++ b/internal/pkg/cli/command/config/registry_test.go @@ -0,0 +1,54 @@ +package config + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestLookupKey_ValidKeys(t *testing.T) { + for _, key := range configKeys { + t.Run(key, func(t *testing.T) { + desc, err := lookupKey(key) + assert.NoError(t, err) + assert.NotEmpty(t, desc.Description) + }) + } +} + +func TestLookupKey_InvalidKey(t *testing.T) { + _, err := lookupKey("not-a-real-key") + assert.Error(t, err) + assert.Contains(t, err.Error(), "not-a-real-key") + for _, key := range configKeys { + assert.Contains(t, err.Error(), key) + } +} + +func TestConfigKeysMatchRegistry(t *testing.T) { + for _, key := range configKeys { + _, err := lookupKey(key) + assert.NoError(t, err, "key %q is in configKeys but not in configRegistry", key) + } + assert.Equal(t, len(configKeys), len(configRegistry), + "configRegistry has keys not listed in configKeys") +} + +func TestVisibleKeysFiltersHiddenKeys(t *testing.T) { + visibleKeys := visibleKeys() + for key, desc := range configRegistry { + if desc.Hidden { + assert.NotContains(t, visibleKeys, key) + } else { + assert.Contains(t, visibleKeys, key) + } + } +} + +func TestDisplayValue_Empty(t *testing.T) { + assert.Equal(t, "", displayValue("")) +} + +func TestDisplayValue_NonEmpty(t *testing.T) { + assert.Equal(t, "production", displayValue("production")) +} diff --git a/internal/pkg/cli/command/config/set.go b/internal/pkg/cli/command/config/set.go index 07d56b08..87e14c2b 100644 --- a/internal/pkg/cli/command/config/set.go +++ b/internal/pkg/cli/command/config/set.go @@ -1,16 +1,26 @@ package config import ( + "context" "errors" + "fmt" + "os" "github.com/pinecone-io/cli/internal/pkg/utils/exit" "github.com/pinecone-io/cli/internal/pkg/utils/help" "github.com/pinecone-io/cli/internal/pkg/utils/msg" "github.com/pinecone-io/cli/internal/pkg/utils/style" + "github.com/pinecone-io/cli/internal/pkg/utils/text" "github.com/spf13/cobra" ) +type SetCmdOptions struct { + json bool +} + func NewSetCmd() *cobra.Command { + options := SetCmdOptions{} + cmd := &cobra.Command{ Use: "set ", Short: "Set a configuration value", @@ -22,42 +32,58 @@ func NewSetCmd() *cobra.Command { Args: cobra.ExactArgs(2), ValidArgsFunction: func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { if len(args) == 0 { - return configKeyOrder, cobra.ShellCompDirectiveNoFileComp + return configKeys, cobra.ShellCompDirectiveNoFileComp } return nil, cobra.ShellCompDirectiveNoFileComp }, Run: func(cmd *cobra.Command, args []string) { - keyName, value := args[0], args[1] - - keyDesc, err := lookupKey(keyName) - if err != nil { - msg.FailMsg("%s", err) + svc := newDefaultConfigService() + if err := runSetCmd(cmd.Context(), svc, args[0], args[1], options); err != nil { + msg.FailJSON(options.json, "%s", err) exit.ErrorMsg(err.Error()) - return } + }, + } - oldVal := keyDesc.getStr() + cmd.Flags().BoolVarP(&options.json, "json", "j", false, "Output as JSON") - if err := keyDesc.setStr(value); err != nil { - if errors.Is(err, ErrNoChange) { - msg.InfoMsg("%s is already set to %s", style.Emphasis(keyName), style.Emphasis(oldVal)) - return - } - msg.FailMsg("%s", err) - exit.ErrorMsg(err.Error()) - return - } + return cmd +} - newVal := keyDesc.getStr() - msg.SuccessMsg("%s updated", style.Emphasis(keyName)) +func runSetCmd(ctx context.Context, svc ConfigService, keyName, value string, opts SetCmdOptions) error { + // --json output for the set command + type setOutput struct { + Key string `json:"key"` + Value string `json:"value"` + } - if keyDesc.onChange != nil { - for _, line := range keyDesc.onChange(cmd.Context(), oldVal, newVal) { - msg.InfoMsg("%s", line) - } + // Fetch the current value up front so it can be shown in the ErrNoChange message. + currentValue, _, err := svc.Get(keyName) + if err != nil { + return err + } + + lines, err := svc.Set(ctx, keyName, value) + if err != nil { + if errors.Is(err, ErrNoChange) { + if opts.json { + fmt.Fprintln(os.Stdout, text.IndentJSON(setOutput{Key: keyName, Value: currentValue})) + return nil } - }, + msg.InfoMsg("%s is already set to %s", style.Emphasis(keyName), style.Emphasis(currentValue)) + return nil + } + return err } - return cmd + if opts.json { + fmt.Fprintln(os.Stdout, text.IndentJSON(setOutput{Key: keyName, Value: value})) + return nil + } + + msg.SuccessMsg("%s updated", style.Emphasis(keyName)) + for _, line := range lines { + msg.InfoMsg("%s", line) + } + return nil } diff --git a/internal/pkg/cli/command/config/set_test.go b/internal/pkg/cli/command/config/set_test.go new file mode 100644 index 00000000..94b5541f --- /dev/null +++ b/internal/pkg/cli/command/config/set_test.go @@ -0,0 +1,93 @@ +package config + +import ( + "context" + "errors" + "testing" + + "github.com/pinecone-io/cli/internal/pkg/cli/testutils" + "github.com/stretchr/testify/assert" +) + +func Test_runSetCmd_ReturnsErrorOnUnknownKey(t *testing.T) { + svc := &mockConfigService{getErr: errors.New("unknown config key")} + + err := runSetCmd(context.Background(), svc, "bad-key", "value", SetCmdOptions{}) + + assert.Error(t, err) + assert.Empty(t, svc.lastSetKey) +} + +func Test_runSetCmd_ReturnsNilOnNoChange(t *testing.T) { + svc := &mockConfigService{ + getValue: "production", + setErr: ErrNoChange, + } + + err := runSetCmd(context.Background(), svc, "environment", "production", SetCmdOptions{}) + + assert.NoError(t, err) + assert.Equal(t, "environment", svc.lastSetKey) + assert.Equal(t, "production", svc.lastSetValue) +} + +func Test_runSetCmd_ReturnsErrorOnValidationFailure(t *testing.T) { + svc := &mockConfigService{ + getValue: "production", + setErr: errors.New("invalid value"), + } + + err := runSetCmd(context.Background(), svc, "environment", "invalid", SetCmdOptions{}) + + assert.Error(t, err) + assert.Equal(t, "environment", svc.lastSetKey) +} + +func Test_runSetCmd_Succeeds(t *testing.T) { + svc := &mockConfigService{getValue: "production"} + + err := runSetCmd(context.Background(), svc, "environment", "staging", SetCmdOptions{}) + + assert.NoError(t, err) + assert.Equal(t, "environment", svc.lastSetKey) + assert.Equal(t, "staging", svc.lastSetValue) +} + +func Test_runSetCmd_SucceedsWithOnChangeLines(t *testing.T) { + svc := &mockConfigService{ + getValue: "production", + setLines: []string{"You have been logged out", "API key cleared"}, + } + + err := runSetCmd(context.Background(), svc, "environment", "staging", SetCmdOptions{}) + + assert.NoError(t, err) + assert.Equal(t, "staging", svc.lastSetValue) +} + +func Test_runSetCmd_JSONOutput(t *testing.T) { + svc := &mockConfigService{getValue: "production"} + + out := testutils.CaptureStdout(t, func() { + err := runSetCmd(context.Background(), svc, "environment", "staging", SetCmdOptions{json: true}) + assert.NoError(t, err) + }) + + assert.Contains(t, out, `"environment"`) + assert.Contains(t, out, `"staging"`) +} + +func Test_runSetCmd_JSONOutputOnNoChange(t *testing.T) { + svc := &mockConfigService{ + getValue: "production", + setErr: ErrNoChange, + } + + out := testutils.CaptureStdout(t, func() { + err := runSetCmd(context.Background(), svc, "environment", "production", SetCmdOptions{json: true}) + assert.NoError(t, err) + }) + + assert.Contains(t, out, `"environment"`) + assert.Contains(t, out, `"production"`) +} diff --git a/internal/pkg/cli/command/config/unset.go b/internal/pkg/cli/command/config/unset.go index b8d982f2..e45e1906 100644 --- a/internal/pkg/cli/command/config/unset.go +++ b/internal/pkg/cli/command/config/unset.go @@ -1,14 +1,25 @@ package config import ( + "context" + "fmt" + "os" + "github.com/pinecone-io/cli/internal/pkg/utils/exit" "github.com/pinecone-io/cli/internal/pkg/utils/help" "github.com/pinecone-io/cli/internal/pkg/utils/msg" "github.com/pinecone-io/cli/internal/pkg/utils/style" + "github.com/pinecone-io/cli/internal/pkg/utils/text" "github.com/spf13/cobra" ) +type UnsetCmdOptions struct { + json bool +} + func NewUnsetCmd() *cobra.Command { + options := UnsetCmdOptions{} + cmd := &cobra.Command{ Use: "unset ", Short: "Reset a configuration value to its default", @@ -17,28 +28,41 @@ func NewUnsetCmd() *cobra.Command { pc config unset color `), Args: cobra.ExactArgs(1), - ValidArgs: configKeyOrder, + ValidArgs: visibleKeys(), Run: func(cmd *cobra.Command, args []string) { - keyName := args[0] - keyDesc, err := lookupKey(keyName) - if err != nil { - msg.FailMsg("%s", err) + svc := newDefaultConfigService() + if err := runUnsetCmd(cmd.Context(), svc, args[0], options); err != nil { + msg.FailJSON(options.json, "%s", err) exit.ErrorMsg(err.Error()) - return - } - - oldVal := keyDesc.getStr() - keyDesc.clearStr() - newVal := keyDesc.getStr() - msg.SuccessMsg("%s cleared", style.Emphasis(keyName)) - - if keyDesc.onChange != nil && oldVal != newVal { - for _, line := range keyDesc.onChange(cmd.Context(), oldVal, newVal) { - msg.InfoMsg("%s", line) - } } }, } + cmd.Flags().BoolVarP(&options.json, "json", "j", false, "Output as JSON") + return cmd } + +func runUnsetCmd(ctx context.Context, svc ConfigService, keyName string, opts UnsetCmdOptions) error { + // --json output for the unset command + type unsetOutput struct { + Key string `json:"key"` + Cleared bool `json:"cleared"` + } + + lines, err := svc.Unset(ctx, keyName) + if err != nil { + return err + } + + if opts.json { + fmt.Fprintln(os.Stdout, text.IndentJSON(unsetOutput{Key: keyName, Cleared: true})) + return nil + } + + msg.SuccessMsg("%s cleared", style.Emphasis(keyName)) + for _, line := range lines { + msg.InfoMsg("%s", line) + } + return nil +} diff --git a/internal/pkg/cli/command/config/unset_test.go b/internal/pkg/cli/command/config/unset_test.go new file mode 100644 index 00000000..bdf5134b --- /dev/null +++ b/internal/pkg/cli/command/config/unset_test.go @@ -0,0 +1,49 @@ +package config + +import ( + "context" + "errors" + "testing" + + "github.com/pinecone-io/cli/internal/pkg/cli/testutils" + "github.com/stretchr/testify/assert" +) + +func Test_runUnsetCmd_ReturnsErrorOnUnknownKey(t *testing.T) { + svc := &mockConfigService{unsetErr: errors.New("unknown config key")} + + err := runUnsetCmd(context.Background(), svc, "bad-key", UnsetCmdOptions{}) + + assert.Error(t, err) +} + +func Test_runUnsetCmd_Succeeds(t *testing.T) { + svc := &mockConfigService{} + + err := runUnsetCmd(context.Background(), svc, "api-key", UnsetCmdOptions{}) + + assert.NoError(t, err) + assert.Equal(t, "api-key", svc.lastUnsetKey) +} + +func Test_runUnsetCmd_SucceedsWithOnChangeLines(t *testing.T) { + svc := &mockConfigService{ + unsetLines: []string{"You have been logged out"}, + } + + err := runUnsetCmd(context.Background(), svc, "environment", UnsetCmdOptions{}) + + assert.NoError(t, err) + assert.Equal(t, "environment", svc.lastUnsetKey) +} + +func Test_runUnsetCmd_JSONOutput(t *testing.T) { + svc := &mockConfigService{} + + out := testutils.CaptureStdout(t, func() { + err := runUnsetCmd(context.Background(), svc, "api-key", UnsetCmdOptions{json: true}) + assert.NoError(t, err) + }) + + assert.JSONEq(t, `{"key":"api-key","cleared":true}`, out) +} From 8b0af444df4865c2e9c9d1bf4e9507fc1a51f2c9 Mon Sep 17 00:00:00 2001 From: Austin DeNoble Date: Sun, 31 May 2026 17:27:22 -0400 Subject: [PATCH 04/17] correct bug in flow from config input normalization to persistence to onChange: add specific fields to keyDescriptor for validateStr, persistStr, and onChange. make sure config set|unset call validateStr, onChange, and then persistStr to avoid onChange failing after persistence has already occurred --- internal/pkg/cli/command/config/registry.go | 120 +++++++++++--------- 1 file changed, 64 insertions(+), 56 deletions(-) diff --git a/internal/pkg/cli/command/config/registry.go b/internal/pkg/cli/command/config/registry.go index 1f825ff5..5c32b43c 100644 --- a/internal/pkg/cli/command/config/registry.go +++ b/internal/pkg/cli/command/config/registry.go @@ -9,16 +9,14 @@ import ( conf "github.com/pinecone-io/cli/internal/pkg/utils/configuration/config" "github.com/pinecone-io/cli/internal/pkg/utils/configuration/secrets" "github.com/pinecone-io/cli/internal/pkg/utils/configuration/state" - "github.com/pinecone-io/cli/internal/pkg/utils/exit" "github.com/pinecone-io/cli/internal/pkg/utils/help" - "github.com/pinecone-io/cli/internal/pkg/utils/msg" "github.com/pinecone-io/cli/internal/pkg/utils/oauth" "github.com/pinecone-io/cli/internal/pkg/utils/style" "github.com/pinecone-io/cli/internal/pkg/utils/text" ) -// ErrNoChange is returned by a keyDescriptor's setStr when the incoming value -// is equivalent to the current stored value and no write is needed. +// ErrNoChange is returned by validateStr when the incoming value is equivalent +// to the current stored value and no write is needed. var ErrNoChange = errors.New("no change") // keyDescriptor describes a single user-configurable setting. @@ -28,12 +26,19 @@ type keyDescriptor struct { Sensitive bool Hidden bool ValidValues []string // non-nil: values shown in help; nil: any non-empty string accepted + defaultVal string // the value restored by Unset; must match what getStr returns at the default getStr func() string - setStr func(value string) error // returns ErrNoChange or a validation error - clearStr func() - // onChange is invoked after a successful setStr. It may call exit.Error for - // fatal side-effect failures. Returns human-readable info lines for the user. - onChange func(ctx context.Context, oldVal, newVal string) []string + // validateStr normalises the incoming value and checks whether it differs from + // the current value. It is pure (no I/O) and must be called before persistStr. + // Returns ErrNoChange when the value is already current, or a validation error. + // If nil, the value is passed through to persistStr unchanged. + validateStr func(value string) (normalizedValue string, err error) + // onChange is called with the prospective new value before it is persisted. + // Returning an error aborts the operation; nothing is written to disk. + onChange func(ctx context.Context, oldVal, newVal string) ([]string, error) + // persistStr writes a normalised value returned by validateStr. It is only + // called after validateStr and onChange have both succeeded. + persistStr func(normalizedValue string) } // configKeys represent the set of valid configuration keys. @@ -57,47 +62,38 @@ var configRegistry = map[string]keyDescriptor{ To clear the explicit API key, run 'pc config unset api-key'. `), - Sensitive: true, - Hidden: false, + Sensitive: true, + defaultVal: "", getStr: func() string { return secrets.DefaultAPIKey.Get() }, - setStr: func(value string) error { - if value == "" { - return fmt.Errorf("api-key value cannot be empty") - } + persistStr: func(value string) { secrets.DefaultAPIKey.Set(value) - return nil - }, - clearStr: func() { - secrets.DefaultAPIKey.Clear() }, }, + "color": { Description: "Enable or disable colored terminal output", - Sensitive: false, - Hidden: false, ValidValues: []string{"true", "false"}, + defaultVal: "true", getStr: func() string { return text.BoolToString(conf.Color.Get()) }, - setStr: func(value string) error { - var colorSetting bool + validateStr: func(value string) (string, error) { switch strings.ToLower(value) { case "true", "on", "1": - colorSetting = true + return "true", nil case "false", "off", "0": - colorSetting = false + return "false", nil default: - return fmt.Errorf("invalid value %q for color; must be one of: true, false", value) + return "", fmt.Errorf("invalid value %q for color; must be one of: true, false", value) } - conf.Color.Set(colorSetting) - return nil }, - clearStr: func() { - conf.Color.Clear() + persistStr: func(value string) { + conf.Color.Set(value == "true") }, }, + "environment": { Description: "Pinecone environment to target (production or staging)", LongDescription: help.Long(` @@ -110,38 +106,36 @@ var configRegistry = map[string]keyDescriptor{ target organization and project are reset. You will need to re-authenticate and re-target after switching. `), - Sensitive: false, Hidden: true, ValidValues: []string{"production", "staging"}, + defaultVal: "production", getStr: func() string { return conf.Environment.Get() }, - setStr: func(value string) error { + validateStr: func(value string) (string, error) { switch value { case "production", "prod": value = "production" case "staging": // already the canonical value default: - return fmt.Errorf("invalid environment %q; must be one of: production, staging", value) + return "", fmt.Errorf("invalid environment %q; must be one of: production, staging", value) } if conf.Environment.Get() == value { - return ErrNoChange + return "", ErrNoChange } - conf.Environment.Set(value) - return nil + return value, nil }, - clearStr: func() { - conf.Environment.Clear() + persistStr: func(value string) { + conf.Environment.Set(value) }, - onChange: func(ctx context.Context, _, _ string) []string { + onChange: func(ctx context.Context, _, _ string) ([]string, error) { var lines []string + // Check for existing OAuth sessions and login credentials and clear them when the environment is changed. token, err := oauth.Token(ctx) if err != nil { - msg.FailMsg("Error retrieving oauth token: %s", err) - exit.Error(err, "error retrieving oauth token") - return nil // unreachable + return nil, fmt.Errorf("error retrieving oauth token: %w", err) } if token != nil && (token.AccessToken != "" || token.RefreshToken != "") { oauth.Logout() @@ -163,7 +157,7 @@ var configRegistry = map[string]keyDescriptor{ lines = append(lines, fmt.Sprintf("Target organization and project cleared; to set a new target, run %s", style.Code("pc target -o myorg -p myproj"))) } - return lines + return lines, nil }, }, } @@ -246,14 +240,21 @@ func (s *defaultConfigService) Set(ctx context.Context, key, value string) ([]st return nil, err } oldVal := desc.getStr() - if err := desc.setStr(value); err != nil { - return nil, err + normalizedVal := value + if desc.validateStr != nil { + if normalizedVal, err = desc.validateStr(value); err != nil { + return nil, err + } } - newVal := desc.getStr() + // Run onChange before persisting so the config file is not written if onChange fails. + var lines []string if desc.onChange != nil { - return desc.onChange(ctx, oldVal, newVal), nil + if lines, err = desc.onChange(ctx, oldVal, normalizedVal); err != nil { + return nil, err + } } - return nil, nil + desc.persistStr(normalizedVal) + return lines, nil } func (s *defaultConfigService) Unset(ctx context.Context, key string) ([]string, error) { @@ -262,18 +263,25 @@ func (s *defaultConfigService) Unset(ctx context.Context, key string) ([]string, return nil, err } oldVal := desc.getStr() - desc.clearStr() - newVal := desc.getStr() - if desc.onChange != nil && oldVal != newVal { - return desc.onChange(ctx, oldVal, newVal), nil + newVal := desc.defaultVal + if oldVal == newVal { + return nil, nil + } + // Run onChange before persisting so the config file is not written if onChange fails. + var lines []string + if desc.onChange != nil { + if lines, err = desc.onChange(ctx, oldVal, newVal); err != nil { + return nil, err + } } - return nil, nil + desc.persistStr(newVal) + return lines, nil } func (s *defaultConfigService) List() []ConfigEntry { - visibleKeys := visibleKeys() - entries := make([]ConfigEntry, 0, len(visibleKeys)) - for _, key := range visibleKeys { + visible := visibleKeys() + entries := make([]ConfigEntry, 0, len(visible)) + for _, key := range visible { desc := configRegistry[key] entries = append(entries, ConfigEntry{ Key: key, From 5bfe55cecb207a11510c234327a2c1e92055c925 Mon Sep 17 00:00:00 2001 From: Austin DeNoble Date: Sun, 31 May 2026 17:33:19 -0400 Subject: [PATCH 05/17] use msg.FailJSON in get, describe, and list config commands --- internal/pkg/cli/command/config/describe.go | 2 +- internal/pkg/cli/command/config/get.go | 2 +- internal/pkg/cli/command/config/list.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/pkg/cli/command/config/describe.go b/internal/pkg/cli/command/config/describe.go index 37ee317f..57b6b293 100644 --- a/internal/pkg/cli/command/config/describe.go +++ b/internal/pkg/cli/command/config/describe.go @@ -34,7 +34,7 @@ func NewDescribeCmd() *cobra.Command { Run: func(cmd *cobra.Command, args []string) { svc := newDefaultConfigService() if err := runDescribeCmd(svc, args[0], options); err != nil { - msg.FailMsg("%s", err) + msg.FailJSON(options.json, "%s", err) exit.ErrorMsg(err.Error()) } }, diff --git a/internal/pkg/cli/command/config/get.go b/internal/pkg/cli/command/config/get.go index 6ab79983..e9e049da 100644 --- a/internal/pkg/cli/command/config/get.go +++ b/internal/pkg/cli/command/config/get.go @@ -35,7 +35,7 @@ func NewGetCmd() *cobra.Command { Run: func(cmd *cobra.Command, args []string) { svc := newDefaultConfigService() if err := runGetCmd(svc, args[0], options); err != nil { - msg.FailMsg("%s", err) + msg.FailJSON(options.json, "%s", err) exit.ErrorMsg(err.Error()) } }, diff --git a/internal/pkg/cli/command/config/list.go b/internal/pkg/cli/command/config/list.go index f1fa7bca..419d01a3 100644 --- a/internal/pkg/cli/command/config/list.go +++ b/internal/pkg/cli/command/config/list.go @@ -32,7 +32,7 @@ func NewListCmd() *cobra.Command { Run: func(cmd *cobra.Command, args []string) { svc := newDefaultConfigService() if err := runListCmd(svc, options); err != nil { - msg.FailMsg("%s", err) + msg.FailJSON(options.json, "%s", err) exit.ErrorMsg(err.Error()) } }, From f499f0a84c92709eb5746ee93d87321f488bfd17 Mon Sep 17 00:00:00 2001 From: Austin DeNoble Date: Sun, 31 May 2026 17:36:40 -0400 Subject: [PATCH 06/17] make sure visibleKeys() preserves the ordering of the listed keys --- internal/pkg/cli/command/config/registry.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/pkg/cli/command/config/registry.go b/internal/pkg/cli/command/config/registry.go index 5c32b43c..251897a2 100644 --- a/internal/pkg/cli/command/config/registry.go +++ b/internal/pkg/cli/command/config/registry.go @@ -173,9 +173,9 @@ func lookupKey(name string) (keyDescriptor, error) { // visibleKeys returns the set of config keys that are surfaced to the user. func visibleKeys() []string { - keys := make([]string, 0, len(configRegistry)) - for key, desc := range configRegistry { - if !desc.Hidden { + keys := make([]string, 0, len(configKeys)) + for _, key := range configKeys { + if !configRegistry[key].Hidden { keys = append(keys, key) } } From 1a6a4f9879e6f0f738f608be52183ccc5bd48806 Mon Sep 17 00:00:00 2001 From: Austin DeNoble Date: Sun, 31 May 2026 17:43:04 -0400 Subject: [PATCH 07/17] clean up confusing aliasing around ValidValues and how things are actually checked in ValidateStr --- internal/pkg/cli/command/config/registry.go | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/internal/pkg/cli/command/config/registry.go b/internal/pkg/cli/command/config/registry.go index 251897a2..436138df 100644 --- a/internal/pkg/cli/command/config/registry.go +++ b/internal/pkg/cli/command/config/registry.go @@ -74,19 +74,19 @@ var configRegistry = map[string]keyDescriptor{ "color": { Description: "Enable or disable colored terminal output", - ValidValues: []string{"true", "false"}, + ValidValues: []string{"true", "false", "on", "off"}, defaultVal: "true", getStr: func() string { return text.BoolToString(conf.Color.Get()) }, validateStr: func(value string) (string, error) { switch strings.ToLower(value) { - case "true", "on", "1": + case "true", "on": return "true", nil - case "false", "off", "0": + case "false", "off": return "false", nil default: - return "", fmt.Errorf("invalid value %q for color; must be one of: true, false", value) + return "", fmt.Errorf("invalid value %q for color; must be one of: true, false, on, off", value) } }, persistStr: func(value string) { @@ -114,10 +114,8 @@ var configRegistry = map[string]keyDescriptor{ }, validateStr: func(value string) (string, error) { switch value { - case "production", "prod": - value = "production" - case "staging": - // already the canonical value + case "production", "staging": + // canonical values default: return "", fmt.Errorf("invalid environment %q; must be one of: production, staging", value) } From 8936497653cbaa8b563bc63ef24c4f8c4536af1c Mon Sep 17 00:00:00 2001 From: Austin DeNoble Date: Sun, 31 May 2026 19:06:51 -0400 Subject: [PATCH 08/17] clean up error message when using unset and the value is unchanged --- internal/pkg/cli/command/config/registry.go | 2 +- internal/pkg/cli/command/config/unset.go | 9 +++++++++ internal/pkg/cli/command/config/unset_test.go | 19 +++++++++++++++++++ 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/internal/pkg/cli/command/config/registry.go b/internal/pkg/cli/command/config/registry.go index 436138df..8cbb58a0 100644 --- a/internal/pkg/cli/command/config/registry.go +++ b/internal/pkg/cli/command/config/registry.go @@ -263,7 +263,7 @@ func (s *defaultConfigService) Unset(ctx context.Context, key string) ([]string, oldVal := desc.getStr() newVal := desc.defaultVal if oldVal == newVal { - return nil, nil + return nil, ErrNoChange } // Run onChange before persisting so the config file is not written if onChange fails. var lines []string diff --git a/internal/pkg/cli/command/config/unset.go b/internal/pkg/cli/command/config/unset.go index e45e1906..fd27191d 100644 --- a/internal/pkg/cli/command/config/unset.go +++ b/internal/pkg/cli/command/config/unset.go @@ -2,6 +2,7 @@ package config import ( "context" + "errors" "fmt" "os" @@ -52,6 +53,14 @@ func runUnsetCmd(ctx context.Context, svc ConfigService, keyName string, opts Un lines, err := svc.Unset(ctx, keyName) if err != nil { + if errors.Is(err, ErrNoChange) { + if opts.json { + fmt.Fprintln(os.Stdout, text.IndentJSON(unsetOutput{Key: keyName, Cleared: false})) + return nil + } + msg.InfoMsg("%s is already at its default value", style.Emphasis(keyName)) + return nil + } return err } diff --git a/internal/pkg/cli/command/config/unset_test.go b/internal/pkg/cli/command/config/unset_test.go index bdf5134b..c2f2b793 100644 --- a/internal/pkg/cli/command/config/unset_test.go +++ b/internal/pkg/cli/command/config/unset_test.go @@ -37,6 +37,14 @@ func Test_runUnsetCmd_SucceedsWithOnChangeLines(t *testing.T) { assert.Equal(t, "environment", svc.lastUnsetKey) } +func Test_runUnsetCmd_ReturnsNilOnNoChange(t *testing.T) { + svc := &mockConfigService{unsetErr: ErrNoChange} + + err := runUnsetCmd(context.Background(), svc, "color", UnsetCmdOptions{}) + + assert.NoError(t, err) +} + func Test_runUnsetCmd_JSONOutput(t *testing.T) { svc := &mockConfigService{} @@ -47,3 +55,14 @@ func Test_runUnsetCmd_JSONOutput(t *testing.T) { assert.JSONEq(t, `{"key":"api-key","cleared":true}`, out) } + +func Test_runUnsetCmd_JSONOutputOnNoChange(t *testing.T) { + svc := &mockConfigService{unsetErr: ErrNoChange} + + out := testutils.CaptureStdout(t, func() { + err := runUnsetCmd(context.Background(), svc, "color", UnsetCmdOptions{json: true}) + assert.NoError(t, err) + }) + + assert.JSONEq(t, `{"key":"color","cleared":false}`, out) +} From f455c641c5822f3eb7a8ed524be2a4f44fda83b0 Mon Sep 17 00:00:00 2001 From: Austin DeNoble Date: Mon, 1 Jun 2026 10:42:14 -0400 Subject: [PATCH 09/17] add 'prod' back to environment configuration --- internal/pkg/cli/command/config/registry.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/pkg/cli/command/config/registry.go b/internal/pkg/cli/command/config/registry.go index 8cbb58a0..05e727ab 100644 --- a/internal/pkg/cli/command/config/registry.go +++ b/internal/pkg/cli/command/config/registry.go @@ -107,13 +107,15 @@ var configRegistry = map[string]keyDescriptor{ and re-target after switching. `), Hidden: true, - ValidValues: []string{"production", "staging"}, + ValidValues: []string{"production", "prod", "staging"}, defaultVal: "production", getStr: func() string { return conf.Environment.Get() }, validateStr: func(value string) (string, error) { switch value { + case "prod": + value = "production" case "production", "staging": // canonical values default: From 12ba17168c8d308eb0103486d9e986baa845b07c Mon Sep 17 00:00:00 2001 From: Austin DeNoble Date: Mon, 1 Jun 2026 11:34:00 -0400 Subject: [PATCH 10/17] add keyDescriptor.getStoredStr and ConfigProperty.GetStored to support reading from viper and bypassing any configured env setting --- internal/pkg/cli/command/config/registry.go | 25 ++++++++++++++++--- .../pkg/utils/configuration/properties.go | 16 ++++++++++++ 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/internal/pkg/cli/command/config/registry.go b/internal/pkg/cli/command/config/registry.go index 05e727ab..35f4dab1 100644 --- a/internal/pkg/cli/command/config/registry.go +++ b/internal/pkg/cli/command/config/registry.go @@ -27,7 +27,11 @@ type keyDescriptor struct { Hidden bool ValidValues []string // non-nil: values shown in help; nil: any non-empty string accepted defaultVal string // the value restored by Unset; must match what getStr returns at the default - getStr func() string + getStr func() string + // getStoredStr reads the value from the config file only, bypassing env var + // overrides. Used for change-detection in Set and Unset. If nil, getStr is + // used for comparisons instead. + getStoredStr func() string // validateStr normalises the incoming value and checks whether it differs from // the current value. It is pure (no I/O) and must be called before persistStr. // Returns ErrNoChange when the value is already current, or a validation error. @@ -112,6 +116,9 @@ var configRegistry = map[string]keyDescriptor{ getStr: func() string { return conf.Environment.Get() }, + getStoredStr: func() string { + return conf.Environment.GetStored() + }, validateStr: func(value string) (string, error) { switch value { case "prod": @@ -121,7 +128,7 @@ var configRegistry = map[string]keyDescriptor{ default: return "", fmt.Errorf("invalid environment %q; must be one of: production, staging", value) } - if conf.Environment.Get() == value { + if conf.Environment.GetStored() == value { return "", ErrNoChange } return value, nil @@ -239,7 +246,11 @@ func (s *defaultConfigService) Set(ctx context.Context, key, value string) ([]st if err != nil { return nil, err } - oldVal := desc.getStr() + getStored := desc.getStr + if desc.getStoredStr != nil { + getStored = desc.getStoredStr + } + oldVal := getStored() normalizedVal := value if desc.validateStr != nil { if normalizedVal, err = desc.validateStr(value); err != nil { @@ -262,7 +273,13 @@ func (s *defaultConfigService) Unset(ctx context.Context, key string) ([]string, if err != nil { return nil, err } - oldVal := desc.getStr() + // Use the stored (file) value for the no-op check so that an env var + // overriding the effective value does not cause a spurious onChange. + getStored := desc.getStr + if desc.getStoredStr != nil { + getStored = desc.getStoredStr + } + oldVal := getStored() newVal := desc.defaultVal if oldVal == newVal { return nil, ErrNoChange diff --git a/internal/pkg/utils/configuration/properties.go b/internal/pkg/utils/configuration/properties.go index 9814be57..1796385a 100644 --- a/internal/pkg/utils/configuration/properties.go +++ b/internal/pkg/utils/configuration/properties.go @@ -38,6 +38,22 @@ func (c ConfigProperty[T]) Get() T { return c.ViperStore.Get(c.KeyName).(T) } +// GetStored reads the value directly from the config file, bypassing any +// environment variable overrides. Use this for change-detection comparisons +// where the persisted value matters rather than the effective runtime value. +func (c ConfigProperty[T]) GetStored() T { + v := viper.New() + v.SetConfigFile(c.ViperStore.ConfigFileUsed()) + if err := v.ReadInConfig(); err != nil { + return c.DefaultValue + } + result := v.Get(c.KeyName) + if result == nil { + return c.DefaultValue + } + return result.(T) +} + func (c ConfigProperty[T]) Clear() { c.Set(c.DefaultValue) } From b90885b8b991be44296d17bfd6983cfdaaed6443 Mon Sep 17 00:00:00 2001 From: Austin DeNoble Date: Mon, 1 Jun 2026 12:02:39 -0400 Subject: [PATCH 11/17] add onChange to the api-key keyDescriptor to make sure we're updating state.AuthedUser appropriately if configuring an API key --- internal/pkg/cli/command/auth/configure.go | 2 +- internal/pkg/cli/command/config/registry.go | 25 +++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/internal/pkg/cli/command/auth/configure.go b/internal/pkg/cli/command/auth/configure.go index 96586d0c..dfae2c2b 100644 --- a/internal/pkg/cli/command/auth/configure.go +++ b/internal/pkg/cli/command/auth/configure.go @@ -43,7 +43,7 @@ var ( When you configure a service account, the CLI automatically targets the organization associated with that account, and prompts you to select a project if multiple exist. - An API overrides any explicitly targeted organization and project, instead targeting + An API key overrides any explicitly targeted organization and project, instead targeting the organization and project associated with the API key itself. API keys do not grant Admin API access. diff --git a/internal/pkg/cli/command/config/registry.go b/internal/pkg/cli/command/config/registry.go index 35f4dab1..e0d479b6 100644 --- a/internal/pkg/cli/command/config/registry.go +++ b/internal/pkg/cli/command/config/registry.go @@ -74,6 +74,31 @@ var configRegistry = map[string]keyDescriptor{ persistStr: func(value string) { secrets.DefaultAPIKey.Set(value) }, + // Reconcile the auth context whenever the API key changes, matching the + // behaviour of pc auth clear --api-key. + onChange: func(_ context.Context, _, newVal string) ([]string, error) { + if newVal != "" { + state.AuthedUser.Update(func(u *state.TargetUser) { + u.AuthContext = state.AuthDefaultAPIKey + }) + return nil, nil + } + // Key was cleared — fall back to whichever credential remains. + if secrets.ClientId.Get() != "" && secrets.ClientSecret.Get() != "" { + state.AuthedUser.Update(func(u *state.TargetUser) { + u.AuthContext = state.AuthServiceAccount + }) + } else if secrets.GetOAuth2Token().AccessToken != "" { + state.AuthedUser.Update(func(u *state.TargetUser) { + u.AuthContext = state.AuthUserToken + }) + } else { + state.AuthedUser.Update(func(u *state.TargetUser) { + u.AuthContext = state.AuthNone + }) + } + return nil, nil + }, }, "color": { From 4759118b99d9ba1a672976749f550d4ae4d3a7cb Mon Sep 17 00:00:00 2001 From: Austin DeNoble Date: Mon, 1 Jun 2026 12:13:27 -0400 Subject: [PATCH 12/17] Fix JSON output correctness for config set and unset - Output the normalized stored value rather than the raw user input in pc config set --json (e.g. true instead of on for color aliases) - Include onChange side-effect messages in JSON output for both set and unset so environment changes (logout, key/target clear) are visible to machine consumers, not silently dropped - Treat an unreadable OAuth token record as no active session in the environment onChange hook rather than blocking the operation entirely, unblocking users who authenticate via API key --- internal/pkg/cli/command/config/registry.go | 8 +++--- internal/pkg/cli/command/config/set.go | 10 ++++--- internal/pkg/cli/command/config/set_test.go | 26 ++++++++++++++++--- internal/pkg/cli/command/config/unset.go | 7 ++--- internal/pkg/cli/command/config/unset_test.go | 15 +++++++++++ 5 files changed, 53 insertions(+), 13 deletions(-) diff --git a/internal/pkg/cli/command/config/registry.go b/internal/pkg/cli/command/config/registry.go index e0d479b6..9a88ecf2 100644 --- a/internal/pkg/cli/command/config/registry.go +++ b/internal/pkg/cli/command/config/registry.go @@ -165,10 +165,10 @@ var configRegistry = map[string]keyDescriptor{ var lines []string // Check for existing OAuth sessions and login credentials and clear them when the environment is changed. - token, err := oauth.Token(ctx) - if err != nil { - return nil, fmt.Errorf("error retrieving oauth token: %w", err) - } + // A read error means the OAuth record is absent or unreadable — + // treat it as no active session and proceed rather than blocking + // users who authenticate via API key. + token, _ := oauth.Token(ctx) if token != nil && (token.AccessToken != "" || token.RefreshToken != "") { oauth.Logout() lines = append(lines, fmt.Sprintf("You have been logged out; to login again, run %s", style.Code("pc login"))) diff --git a/internal/pkg/cli/command/config/set.go b/internal/pkg/cli/command/config/set.go index 87e14c2b..1787775b 100644 --- a/internal/pkg/cli/command/config/set.go +++ b/internal/pkg/cli/command/config/set.go @@ -53,8 +53,9 @@ func NewSetCmd() *cobra.Command { func runSetCmd(ctx context.Context, svc ConfigService, keyName, value string, opts SetCmdOptions) error { // --json output for the set command type setOutput struct { - Key string `json:"key"` - Value string `json:"value"` + Key string `json:"key"` + Value string `json:"value"` + Messages []string `json:"messages,omitempty"` } // Fetch the current value up front so it can be shown in the ErrNoChange message. @@ -77,7 +78,10 @@ func runSetCmd(ctx context.Context, svc ConfigService, keyName, value string, op } if opts.json { - fmt.Fprintln(os.Stdout, text.IndentJSON(setOutput{Key: keyName, Value: value})) + // Re-read the stored value so the output reflects what was actually + // persisted (e.g. "true" rather than the user-supplied alias "on"). + storedValue, _, _ := svc.Get(keyName) + fmt.Fprintln(os.Stdout, text.IndentJSON(setOutput{Key: keyName, Value: storedValue, Messages: lines})) return nil } diff --git a/internal/pkg/cli/command/config/set_test.go b/internal/pkg/cli/command/config/set_test.go index 94b5541f..64608783 100644 --- a/internal/pkg/cli/command/config/set_test.go +++ b/internal/pkg/cli/command/config/set_test.go @@ -66,15 +66,35 @@ func Test_runSetCmd_SucceedsWithOnChangeLines(t *testing.T) { } func Test_runSetCmd_JSONOutput(t *testing.T) { - svc := &mockConfigService{getValue: "production"} + // getValue is returned by the post-set Get call and represents the normalized + // stored value — distinct from the raw user input to catch normalization bugs. + svc := &mockConfigService{getValue: "true"} + + out := testutils.CaptureStdout(t, func() { + err := runSetCmd(context.Background(), svc, "color", "on", SetCmdOptions{json: true}) + assert.NoError(t, err) + }) + + assert.Contains(t, out, `"color"`) + // "true" is the normalized form; the raw input "on" must not appear + assert.Contains(t, out, `"true"`) + assert.NotContains(t, out, `"on"`) +} + +func Test_runSetCmd_JSONOutputIncludesOnChangeMessages(t *testing.T) { + svc := &mockConfigService{ + getValue: "staging", + setLines: []string{"You have been logged out", "API key cleared"}, + } out := testutils.CaptureStdout(t, func() { err := runSetCmd(context.Background(), svc, "environment", "staging", SetCmdOptions{json: true}) assert.NoError(t, err) }) - assert.Contains(t, out, `"environment"`) - assert.Contains(t, out, `"staging"`) + assert.Contains(t, out, `"messages"`) + assert.Contains(t, out, "You have been logged out") + assert.Contains(t, out, "API key cleared") } func Test_runSetCmd_JSONOutputOnNoChange(t *testing.T) { diff --git a/internal/pkg/cli/command/config/unset.go b/internal/pkg/cli/command/config/unset.go index fd27191d..f479c971 100644 --- a/internal/pkg/cli/command/config/unset.go +++ b/internal/pkg/cli/command/config/unset.go @@ -47,8 +47,9 @@ func NewUnsetCmd() *cobra.Command { func runUnsetCmd(ctx context.Context, svc ConfigService, keyName string, opts UnsetCmdOptions) error { // --json output for the unset command type unsetOutput struct { - Key string `json:"key"` - Cleared bool `json:"cleared"` + Key string `json:"key"` + Cleared bool `json:"cleared"` + Messages []string `json:"messages,omitempty"` } lines, err := svc.Unset(ctx, keyName) @@ -65,7 +66,7 @@ func runUnsetCmd(ctx context.Context, svc ConfigService, keyName string, opts Un } if opts.json { - fmt.Fprintln(os.Stdout, text.IndentJSON(unsetOutput{Key: keyName, Cleared: true})) + fmt.Fprintln(os.Stdout, text.IndentJSON(unsetOutput{Key: keyName, Cleared: true, Messages: lines})) return nil } diff --git a/internal/pkg/cli/command/config/unset_test.go b/internal/pkg/cli/command/config/unset_test.go index c2f2b793..3303764a 100644 --- a/internal/pkg/cli/command/config/unset_test.go +++ b/internal/pkg/cli/command/config/unset_test.go @@ -56,6 +56,21 @@ func Test_runUnsetCmd_JSONOutput(t *testing.T) { assert.JSONEq(t, `{"key":"api-key","cleared":true}`, out) } +func Test_runUnsetCmd_JSONOutputIncludesOnChangeMessages(t *testing.T) { + svc := &mockConfigService{ + unsetLines: []string{"You have been logged out", "Target cleared"}, + } + + out := testutils.CaptureStdout(t, func() { + err := runUnsetCmd(context.Background(), svc, "environment", UnsetCmdOptions{json: true}) + assert.NoError(t, err) + }) + + assert.Contains(t, out, `"messages"`) + assert.Contains(t, out, "You have been logged out") + assert.Contains(t, out, "Target cleared") +} + func Test_runUnsetCmd_JSONOutputOnNoChange(t *testing.T) { svc := &mockConfigService{unsetErr: ErrNoChange} From 12866738e41078f7172b6817a0bb6f3146bb1368 Mon Sep 17 00:00:00 2001 From: Austin DeNoble Date: Mon, 1 Jun 2026 14:24:15 -0400 Subject: [PATCH 13/17] add an -all flag to config list to allow showing hidden configuration settings if present, update tests and documentation --- .../pkg/cli/command/config/config_test.go | 2 +- internal/pkg/cli/command/config/list.go | 8 +++-- internal/pkg/cli/command/config/list_test.go | 36 +++++++++++++++++++ internal/pkg/cli/command/config/registry.go | 18 +++++++--- 4 files changed, 56 insertions(+), 8 deletions(-) diff --git a/internal/pkg/cli/command/config/config_test.go b/internal/pkg/cli/command/config/config_test.go index a3da8fc1..7e6225ba 100644 --- a/internal/pkg/cli/command/config/config_test.go +++ b/internal/pkg/cli/command/config/config_test.go @@ -48,7 +48,7 @@ func (m *mockConfigService) Unset(ctx context.Context, key string) ([]string, er return m.unsetLines, m.unsetErr } -func (m *mockConfigService) List() []ConfigEntry { +func (m *mockConfigService) List(includeHidden bool) []ConfigEntry { return m.listResult } diff --git a/internal/pkg/cli/command/config/list.go b/internal/pkg/cli/command/config/list.go index 419d01a3..6bdafae1 100644 --- a/internal/pkg/cli/command/config/list.go +++ b/internal/pkg/cli/command/config/list.go @@ -15,6 +15,7 @@ import ( type ListCmdOptions struct { reveal bool json bool + all bool } func NewListCmd() *cobra.Command { @@ -25,6 +26,7 @@ func NewListCmd() *cobra.Command { Short: "List all configuration settings and their current values", Example: help.Examples(` pc config list + pc config list --all pc config list --reveal pc config list --json `), @@ -40,6 +42,7 @@ func NewListCmd() *cobra.Command { cmd.Flags().BoolVar(&options.reveal, "reveal", false, "Reveal the full value for sensitive settings like api-key") cmd.Flags().BoolVarP(&options.json, "json", "j", false, "Output as JSON") + cmd.Flags().BoolVarP(&options.all, "all", "a", false, "Include hidden settings such as environment") return cmd } @@ -50,9 +53,10 @@ func runListCmd(svc ConfigService, opts ListCmdOptions) error { Key string `json:"key"` Value string `json:"value"` Description string `json:"description"` + Hidden bool `json:"hidden,omitempty"` } - entries := svc.List() + entries := svc.List(opts.all) if opts.json { jsonEntries := make([]listOutput, 0, len(entries)) @@ -61,7 +65,7 @@ func runListCmd(svc ConfigService, opts ListCmdOptions) error { if e.Sensitive && !opts.reveal { value = presenters.MaskHeadTail(value, 4, 4) } - jsonEntries = append(jsonEntries, listOutput{Key: e.Key, Value: value, Description: e.Description}) + jsonEntries = append(jsonEntries, listOutput{Key: e.Key, Value: value, Description: e.Description, Hidden: e.Hidden}) } fmt.Fprintln(os.Stdout, text.IndentJSON(jsonEntries)) return nil diff --git a/internal/pkg/cli/command/config/list_test.go b/internal/pkg/cli/command/config/list_test.go index 423db539..f47ce742 100644 --- a/internal/pkg/cli/command/config/list_test.go +++ b/internal/pkg/cli/command/config/list_test.go @@ -71,6 +71,42 @@ func Test_runListCmd_JSONOutput(t *testing.T) { assert.Contains(t, out, `"true"`) } +func Test_runListCmd_AllFlagIncludesHiddenKeys(t *testing.T) { + svc := &mockConfigService{ + listResult: []ConfigEntry{ + {Key: "api-key", Value: "", Description: "API key", Sensitive: true}, + {Key: "color", Value: "true", Description: "Color output"}, + {Key: "environment", Value: "production", Description: "Environment", Hidden: true}, + }, + } + + out := testutils.CaptureStdout(t, func() { + err := runListCmd(svc, ListCmdOptions{all: true}) + assert.NoError(t, err) + }) + + assert.Contains(t, out, "environment") +} + +func Test_runListCmd_JSONAllFlagIncludesHiddenField(t *testing.T) { + svc := &mockConfigService{ + listResult: []ConfigEntry{ + {Key: "color", Value: "true", Description: "Color output", Hidden: false}, + {Key: "environment", Value: "production", Description: "Environment", Hidden: true}, + }, + } + + out := testutils.CaptureStdout(t, func() { + err := runListCmd(svc, ListCmdOptions{json: true, all: true}) + assert.NoError(t, err) + }) + + assert.Contains(t, out, "environment") + assert.Contains(t, out, `"hidden": true`) + // Non-hidden keys should not have the hidden field (omitempty) + assert.NotContains(t, out, `"hidden": false`) +} + func Test_runListCmd_JSONOutputRevealsSensitiveKey(t *testing.T) { svc := &mockConfigService{ listResult: []ConfigEntry{ diff --git a/internal/pkg/cli/command/config/registry.go b/internal/pkg/cli/command/config/registry.go index 9a88ecf2..2ba1d91c 100644 --- a/internal/pkg/cli/command/config/registry.go +++ b/internal/pkg/cli/command/config/registry.go @@ -130,6 +130,9 @@ var configRegistry = map[string]keyDescriptor{ leave this set to 'production'; 'staging' is intended for Pinecone internal development. + This setting is hidden from 'pc config list' by default. Use + 'pc config list --all' to include it. + Changing the environment clears your existing authentication state: any OAuth session is logged out, the default API key is cleared, and the target organization and project are reset. You will need to re-authenticate @@ -230,6 +233,7 @@ type ConfigEntry struct { Value string Description string Sensitive bool + Hidden bool } // ConfigDescription holds full metadata for a config key, used by the describe command. @@ -248,7 +252,7 @@ type ConfigService interface { Get(key string) (value string, sensitive bool, err error) Set(ctx context.Context, key, value string) (onChangeLines []string, err error) Unset(ctx context.Context, key string) (onChangeLines []string, err error) - List() []ConfigEntry + List(includeHidden bool) []ConfigEntry Describe(key string) (ConfigDescription, error) } @@ -320,16 +324,20 @@ func (s *defaultConfigService) Unset(ctx context.Context, key string) ([]string, return lines, nil } -func (s *defaultConfigService) List() []ConfigEntry { - visible := visibleKeys() - entries := make([]ConfigEntry, 0, len(visible)) - for _, key := range visible { +func (s *defaultConfigService) List(includeHidden bool) []ConfigEntry { + keys := visibleKeys() + if includeHidden { + keys = configKeys + } + entries := make([]ConfigEntry, 0, len(keys)) + for _, key := range keys { desc := configRegistry[key] entries = append(entries, ConfigEntry{ Key: key, Value: desc.getStr(), Description: desc.Description, Sensitive: desc.Sensitive, + Hidden: desc.Hidden, }) } return entries From 31cbc81833c35fb16dd65acce496fcf9ab3a3c6b Mon Sep 17 00:00:00 2001 From: Austin DeNoble Date: Mon, 1 Jun 2026 14:38:41 -0400 Subject: [PATCH 14/17] make sure config set output is showing what's saved or passed rather than any environment variable overrides --- internal/pkg/cli/command/config/config_test.go | 15 +++++++++++++++ internal/pkg/cli/command/config/registry.go | 14 ++++++++++++++ internal/pkg/cli/command/config/set.go | 9 ++++----- 3 files changed, 33 insertions(+), 5 deletions(-) diff --git a/internal/pkg/cli/command/config/config_test.go b/internal/pkg/cli/command/config/config_test.go index 7e6225ba..d5d3135b 100644 --- a/internal/pkg/cli/command/config/config_test.go +++ b/internal/pkg/cli/command/config/config_test.go @@ -12,6 +12,13 @@ type mockConfigService struct { getErr error lastGetKey string + // GetStored — defaults to getValue/getSensitive/getErr when not explicitly set + getStoredValue string + getStoredSensitive bool + getStoredErr error + getStoredOverride bool // set to true to use getStored* fields instead of get* fields + lastGetStoredKey string + // Set setLines []string setErr error @@ -37,6 +44,14 @@ func (m *mockConfigService) Get(key string) (string, bool, error) { return m.getValue, m.getSensitive, m.getErr } +func (m *mockConfigService) GetStored(key string) (string, bool, error) { + m.lastGetStoredKey = key + if m.getStoredOverride { + return m.getStoredValue, m.getStoredSensitive, m.getStoredErr + } + return m.getValue, m.getSensitive, m.getErr +} + func (m *mockConfigService) Set(ctx context.Context, key, value string) ([]string, error) { m.lastSetKey = key m.lastSetValue = value diff --git a/internal/pkg/cli/command/config/registry.go b/internal/pkg/cli/command/config/registry.go index 2ba1d91c..0107422b 100644 --- a/internal/pkg/cli/command/config/registry.go +++ b/internal/pkg/cli/command/config/registry.go @@ -249,7 +249,10 @@ type ConfigDescription struct { // ConfigService abstracts config registry operations for unit testing across // the get, set, unset, list, and describe commands. type ConfigService interface { + // Get returns the effective value, including any env var override. Get(key string) (value string, sensitive bool, err error) + // GetStored returns the value persisted in the config file, bypassing env var overrides. + GetStored(key string) (value string, sensitive bool, err error) Set(ctx context.Context, key, value string) (onChangeLines []string, err error) Unset(ctx context.Context, key string) (onChangeLines []string, err error) List(includeHidden bool) []ConfigEntry @@ -270,6 +273,17 @@ func (s *defaultConfigService) Get(key string) (string, bool, error) { return desc.getStr(), desc.Sensitive, nil } +func (s *defaultConfigService) GetStored(key string) (string, bool, error) { + desc, err := lookupKey(key) + if err != nil { + return "", false, err + } + if desc.getStoredStr != nil { + return desc.getStoredStr(), desc.Sensitive, nil + } + return desc.getStr(), desc.Sensitive, nil +} + func (s *defaultConfigService) Set(ctx context.Context, key, value string) ([]string, error) { desc, err := lookupKey(key) if err != nil { diff --git a/internal/pkg/cli/command/config/set.go b/internal/pkg/cli/command/config/set.go index 1787775b..ed501a4e 100644 --- a/internal/pkg/cli/command/config/set.go +++ b/internal/pkg/cli/command/config/set.go @@ -58,8 +58,9 @@ func runSetCmd(ctx context.Context, svc ConfigService, keyName, value string, op Messages []string `json:"messages,omitempty"` } - // Fetch the current value up front so it can be shown in the ErrNoChange message. - currentValue, _, err := svc.Get(keyName) + // Use the stored (file) value throughout so output reflects what is actually + // persisted rather than any env var override. + currentValue, _, err := svc.GetStored(keyName) if err != nil { return err } @@ -78,9 +79,7 @@ func runSetCmd(ctx context.Context, svc ConfigService, keyName, value string, op } if opts.json { - // Re-read the stored value so the output reflects what was actually - // persisted (e.g. "true" rather than the user-supplied alias "on"). - storedValue, _, _ := svc.Get(keyName) + storedValue, _, _ := svc.GetStored(keyName) fmt.Fprintln(os.Stdout, text.IndentJSON(setOutput{Key: keyName, Value: storedValue, Messages: lines})) return nil } From af41457a330f3edbb6d64126268f726d09784454 Mon Sep 17 00:00:00 2001 From: Austin DeNoble Date: Mon, 1 Jun 2026 16:10:24 -0400 Subject: [PATCH 15/17] add getStoredStr for api-key keyDescriptor --- internal/pkg/cli/command/config/registry.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/pkg/cli/command/config/registry.go b/internal/pkg/cli/command/config/registry.go index 0107422b..a180df84 100644 --- a/internal/pkg/cli/command/config/registry.go +++ b/internal/pkg/cli/command/config/registry.go @@ -71,6 +71,9 @@ var configRegistry = map[string]keyDescriptor{ getStr: func() string { return secrets.DefaultAPIKey.Get() }, + getStoredStr: func() string { + return secrets.DefaultAPIKey.GetStored() + }, persistStr: func(value string) { secrets.DefaultAPIKey.Set(value) }, From d3178b2be60a6abacfa048a2108320b929c2924b Mon Sep 17 00:00:00 2001 From: Austin DeNoble Date: Mon, 1 Jun 2026 16:20:04 -0400 Subject: [PATCH 16/17] always call oauth.Logout() when environment configuration is changed --- internal/pkg/cli/command/config/registry.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/internal/pkg/cli/command/config/registry.go b/internal/pkg/cli/command/config/registry.go index a180df84..f655ee70 100644 --- a/internal/pkg/cli/command/config/registry.go +++ b/internal/pkg/cli/command/config/registry.go @@ -171,12 +171,14 @@ var configRegistry = map[string]keyDescriptor{ var lines []string // Check for existing OAuth sessions and login credentials and clear them when the environment is changed. - // A read error means the OAuth record is absent or unreadable — - // treat it as no active session and proceed rather than blocking - // users who authenticate via API key. + // Always clear any stored OAuth session when switching environments. + // oauth.Token may fail (e.g. expired refresh, parse error) even when + // tokens are still on disk, so we cannot rely on its success to decide + // whether to call Logout — we call it unconditionally. The result is + // only used to pick the right message. token, _ := oauth.Token(ctx) + oauth.Logout() if token != nil && (token.AccessToken != "" || token.RefreshToken != "") { - oauth.Logout() lines = append(lines, fmt.Sprintf("You have been logged out; to login again, run %s", style.Code("pc login"))) } else { lines = append(lines, fmt.Sprintf("To login, run %s", style.Code("pc login"))) From 2be3a515463ebb1dc5efb0bca386d06d82d1e3d4 Mon Sep 17 00:00:00 2001 From: Austin DeNoble Date: Mon, 1 Jun 2026 17:09:38 -0400 Subject: [PATCH 17/17] allow color aliases --- internal/pkg/cli/command/config/registry.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/pkg/cli/command/config/registry.go b/internal/pkg/cli/command/config/registry.go index f655ee70..5fd11f1b 100644 --- a/internal/pkg/cli/command/config/registry.go +++ b/internal/pkg/cli/command/config/registry.go @@ -27,7 +27,7 @@ type keyDescriptor struct { Hidden bool ValidValues []string // non-nil: values shown in help; nil: any non-empty string accepted defaultVal string // the value restored by Unset; must match what getStr returns at the default - getStr func() string + getStr func() string // getStoredStr reads the value from the config file only, bypassing env var // overrides. Used for change-detection in Set and Unset. If nil, getStr is // used for comparisons instead. @@ -106,16 +106,16 @@ var configRegistry = map[string]keyDescriptor{ "color": { Description: "Enable or disable colored terminal output", - ValidValues: []string{"true", "false", "on", "off"}, + ValidValues: []string{"true", "false", "on", "off", "1", "0"}, defaultVal: "true", getStr: func() string { return text.BoolToString(conf.Color.Get()) }, validateStr: func(value string) (string, error) { switch strings.ToLower(value) { - case "true", "on": + case "true", "on", "1": return "true", nil - case "false", "off": + case "false", "off", "0": return "false", nil default: return "", fmt.Errorf("invalid value %q for color; must be one of: true, false, on, off", value)