diff --git a/cmd/omni/config_commands.go b/cmd/omni/config_commands.go index 269a67d..828bf21 100644 --- a/cmd/omni/config_commands.go +++ b/cmd/omni/config_commands.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "os" + "sort" "strings" "time" @@ -31,7 +32,10 @@ func addConfigCommands(root *cobra.Command) { configCmd.AddCommand(configInitCmd()) configCmd.AddCommand(configShowCmd()) + configCmd.AddCommand(configListCmd()) configCmd.AddCommand(configUseCmd()) + configCmd.AddCommand(configRenameCmd()) + configCmd.AddCommand(configDeleteCmd()) configCmd.AddCommand(configLoginCmd()) configCmd.AddCommand(configLogoutCmd()) configCmd.AddCommand(configSetFormatCmd()) @@ -227,7 +231,8 @@ func configUseCmd() *cobra.Command { return &cobra.Command{ Use: "use ", Short: "Switch the default profile", - Args: cobra.ExactArgs(1), + Long: "Switch the default profile.\n\nIf the profile name contains spaces, quote it: `omni config use \"My Profile\"`. Run `omni config list` to see profile names.", + Args: profileNameArgs(1, 1), RunE: func(cmd *cobra.Command, args []string) error { cfg, err := config.Load() if err != nil { @@ -236,11 +241,7 @@ func configUseCmd() *cobra.Command { name := args[0] if _, ok := cfg.Profiles[name]; !ok { - available := make([]string, 0, len(cfg.Profiles)) - for k := range cfg.Profiles { - available = append(available, k) - } - return fmt.Errorf("profile %q not found. Available: %s", name, strings.Join(available, ", ")) + return fmt.Errorf("profile %q not found. Available: %s", name, formatProfileList(cfg)) } cfg.DefaultProfile = name @@ -254,11 +255,152 @@ func configUseCmd() *cobra.Command { } } +// profileNameArgs validates the profile-name positional argument, accepting +// between min and max args. A profile name is a single token, so when the shell +// hands us more than max args it almost always means the user typed a name with +// spaces without quoting it (the reported failure mode in #45). Rather than +// cobra's opaque "accepts 1 arg(s), received 2", we reconstruct the likely +// intended name and show how to quote it. +func profileNameArgs(min, max int) cobra.PositionalArgs { + return func(cmd *cobra.Command, args []string) error { + if len(args) > max { + return fmt.Errorf("got %d arguments — a profile name is a single value; if it contains spaces, quote it: %s %q", + len(args), cmd.CommandPath(), strings.Join(args, " ")) + } + if len(args) < min { + return fmt.Errorf("%s requires a profile name", cmd.CommandPath()) + } + return nil + } +} + +// formatProfileList returns a comma-separated list of profile names, quoted so +// that names containing spaces are visually distinguishable. +func formatProfileList(cfg *config.Config) string { + names := make([]string, 0, len(cfg.Profiles)) + for k := range cfg.Profiles { + names = append(names, fmt.Sprintf("%q", k)) + } + sort.Strings(names) + return strings.Join(names, ", ") +} + +func configListCmd() *cobra.Command { + return &cobra.Command{ + Use: "list", + Short: "List configured profiles", + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, args []string) error { + cfg, err := config.Load() + if err != nil { + return fmt.Errorf("no config found — run `omni config init`") + } + if len(cfg.Profiles) == 0 { + fmt.Println("(no profiles)") + return nil + } + names := make([]string, 0, len(cfg.Profiles)) + for k := range cfg.Profiles { + names = append(names, k) + } + sort.Strings(names) + for _, n := range names { + marker := " " + if n == cfg.DefaultProfile { + marker = "* " + } + p := cfg.Profiles[n] + fmt.Printf("%s%s\t%s\t%s\n", marker, n, p.AuthMethod, p.APIEndpoint) + } + return nil + }, + } +} + +func configRenameCmd() *cobra.Command { + return &cobra.Command{ + Use: "rename ", + Short: "Rename a profile", + Args: cobra.ExactArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { + oldName, newName := args[0], args[1] + + cfg, err := config.Load() + if err != nil { + return fmt.Errorf("no config found — run `omni config init`") + } + p, ok := cfg.Profiles[oldName] + if !ok { + return fmt.Errorf("profile %q not found. Available: %s", oldName, formatProfileList(cfg)) + } + if oldName == newName { + return nil + } + if _, exists := cfg.Profiles[newName]; exists { + return fmt.Errorf("profile %q already exists", newName) + } + + delete(cfg.Profiles, oldName) + cfg.Profiles[newName] = p + if cfg.DefaultProfile == oldName { + cfg.DefaultProfile = newName + } + if err := config.Save(cfg); err != nil { + return fmt.Errorf("saving config: %w", err) + } + fmt.Printf("Renamed profile %q to %q\n", oldName, newName) + return nil + }, + } +} + +func configDeleteCmd() *cobra.Command { + var assumeYes bool + cmd := &cobra.Command{ + Use: "delete ", + Short: "Delete a profile", + Args: profileNameArgs(1, 1), + RunE: func(cmd *cobra.Command, args []string) error { + name := args[0] + cfg, err := config.Load() + if err != nil { + return fmt.Errorf("no config found — run `omni config init`") + } + if _, ok := cfg.Profiles[name]; !ok { + return fmt.Errorf("profile %q not found. Available: %s", name, formatProfileList(cfg)) + } + + if !assumeYes { + fmt.Printf("Delete profile %q? This cannot be undone. [y/N]: ", name) + reader := bufio.NewReader(os.Stdin) + answer, _ := reader.ReadString('\n') + answer = strings.ToLower(strings.TrimSpace(answer)) + if answer != "y" && answer != "yes" { + fmt.Println("Aborted.") + return nil + } + } + + delete(cfg.Profiles, name) + if cfg.DefaultProfile == name { + cfg.DefaultProfile = "" + } + if err := config.Save(cfg); err != nil { + return fmt.Errorf("saving config: %w", err) + } + fmt.Printf("Deleted profile %q\n", name) + return nil + }, + } + cmd.Flags().BoolVarP(&assumeYes, "yes", "y", false, "Skip confirmation prompt") + return cmd +} + func configLoginCmd() *cobra.Command { return &cobra.Command{ Use: "login [profile]", Short: "Log in via OAuth browser flow", - Args: cobra.MaximumNArgs(1), + Args: profileNameArgs(0, 1), RunE: func(cmd *cobra.Command, args []string) error { cfg, err := config.Load() if err != nil { @@ -306,7 +448,7 @@ func configLogoutCmd() *cobra.Command { return &cobra.Command{ Use: "logout [profile]", Short: "Clear OAuth tokens from a profile", - Args: cobra.MaximumNArgs(1), + Args: profileNameArgs(0, 1), RunE: func(cmd *cobra.Command, args []string) error { cfg, err := config.Load() if err != nil { diff --git a/cmd/omni/config_commands_test.go b/cmd/omni/config_commands_test.go index 53ec6fe..3c8da59 100644 --- a/cmd/omni/config_commands_test.go +++ b/cmd/omni/config_commands_test.go @@ -390,3 +390,237 @@ func TestConfigShow_NoConfig(t *testing.T) { t.Errorf("error = %q, want it to suggest `config init`", err.Error()) } } + +// --- config use (the #45 quoting UX) --- + +// The reported failure: `omni config use Playground Org-scoped` (unquoted) was +// split by the shell into two args. Instead of cobra's opaque +// "accepts 1 arg(s), received 2", we now hint at quoting and echo the likely +// intended name so the user can copy-paste the fix. +func TestConfigUse_MultiArgHint(t *testing.T) { + withConfig(t, &config.Config{ + Version: 1, + Profiles: map[string]config.Profile{"Playground Org-scoped": {APIEndpoint: "https://x.omniapp.co"}}, + }) + + cmd := configUseCmd() + cmd.SilenceUsage = true + cmd.SilenceErrors = true + cmd.SetArgs([]string{"Playground", "Org-scoped"}) + err := cmd.Execute() + if err == nil { + t.Fatal("expected an args error for unquoted multi-word name, got nil") + } + if !strings.Contains(err.Error(), "quote it") { + t.Errorf("error = %q, want a quoting hint", err.Error()) + } + // The hint echoes the reconstructed name, quoted, so it's directly usable. + if !strings.Contains(err.Error(), `"Playground Org-scoped"`) { + t.Errorf("error = %q, want it to show the quoted name", err.Error()) + } +} + +// The counterpart: a correctly quoted spaced name arrives as one arg and +// switches profiles. Confirms we didn't break the legitimate path. +func TestConfigUse_QuotedSpacedNameWorks(t *testing.T) { + withConfig(t, &config.Config{ + Version: 1, + Profiles: map[string]config.Profile{"Playground Org-scoped": {APIEndpoint: "https://x.omniapp.co"}}, + }) + + cmd := configUseCmd() + cmd.SetArgs([]string{"Playground Org-scoped"}) + out := captureStdout(t, func() { + if err := cmd.Execute(); err != nil { + t.Fatalf("Execute: %v", err) + } + }) + if !strings.Contains(out, `Switched to profile "Playground Org-scoped"`) { + t.Errorf("stdout = %q, want it to switch to the spaced profile", out) + } + + cfg, _ := config.Load() + if cfg.DefaultProfile != "Playground Org-scoped" { + t.Errorf("DefaultProfile = %q, want the spaced name", cfg.DefaultProfile) + } +} + +// --- config list --- + +func TestConfigList_MarksDefault(t *testing.T) { + withConfig(t, &config.Config{ + Version: 1, + DefaultProfile: "prod", + Profiles: map[string]config.Profile{ + "prod": {APIEndpoint: "https://prod.omniapp.co", AuthMethod: "oauth"}, + "staging": {APIEndpoint: "https://staging.omniapp.co", AuthMethod: "api-key"}, + }, + }) + + cmd := configListCmd() + out := captureStdout(t, func() { + if err := cmd.RunE(cmd, nil); err != nil { + t.Fatalf("RunE: %v", err) + } + }) + if !strings.Contains(out, "* prod") { + t.Errorf("expected default profile to be marked with *, got:\n%s", out) + } + if !strings.Contains(out, " staging") { + t.Errorf("expected non-default profile to be unmarked, got:\n%s", out) + } +} + +// --- config rename --- + +// The motivating bug (#45): a profile saved with spaces is unusable. Rename +// must let the user fix it without sudo-editing the config file. +func TestConfigRename_FixesLegacySpacedName(t *testing.T) { + withConfig(t, &config.Config{ + Version: 1, + DefaultProfile: "Playground Org-scoped", + Profiles: map[string]config.Profile{ + "Playground Org-scoped": { + APIEndpoint: "https://playground.omniapp.co", + AuthMethod: "api-key", + APIKey: "secret", + }, + }, + }) + + cmd := configRenameCmd() + out := captureStdout(t, func() { + if err := cmd.RunE(cmd, []string{"Playground Org-scoped", "playground"}); err != nil { + t.Fatalf("RunE: %v", err) + } + }) + if !strings.Contains(out, "Renamed") { + t.Errorf("stdout = %q, want it to confirm rename", out) + } + + cfg, _ := config.Load() + if _, exists := cfg.Profiles["Playground Org-scoped"]; exists { + t.Error("old profile name still present after rename") + } + p, ok := cfg.Profiles["playground"] + if !ok { + t.Fatal("new profile name missing after rename") + } + if p.APIKey != "secret" { + t.Errorf("profile contents lost in rename: %+v", p) + } + if cfg.DefaultProfile != "playground" { + t.Errorf("DefaultProfile = %q, want %q (default should follow the rename)", cfg.DefaultProfile, "playground") + } +} + +// Spaced names are allowed — the user just has to quote them when passing them +// as args. Renaming TO a spaced name must succeed (and works with rename's two +// positional args, which are unambiguous unlike a single-name command). +func TestConfigRename_AllowsSpacedName(t *testing.T) { + withConfig(t, &config.Config{ + Version: 1, + Profiles: map[string]config.Profile{"prod": {APIEndpoint: "https://x.omniapp.co"}}, + }) + + cmd := configRenameCmd() + if err := cmd.RunE(cmd, []string{"prod", "Prod (US)"}); err != nil { + t.Fatalf("RunE: %v", err) + } + + cfg, _ := config.Load() + if _, ok := cfg.Profiles["Prod (US)"]; !ok { + t.Errorf("spaced profile name not created; profiles: %v", cfg.Profiles) + } +} + +func TestConfigRename_RefusesToOverwrite(t *testing.T) { + withConfig(t, &config.Config{ + Version: 1, + Profiles: map[string]config.Profile{ + "prod": {APIEndpoint: "https://prod.omniapp.co"}, + "staging": {APIEndpoint: "https://staging.omniapp.co"}, + }, + }) + + cmd := configRenameCmd() + err := cmd.RunE(cmd, []string{"prod", "staging"}) + if err == nil { + t.Fatal("expected error when renaming over an existing profile, got nil") + } + if !strings.Contains(err.Error(), "already exists") { + t.Errorf("error = %q, want it to mention name conflict", err.Error()) + } +} + +func TestConfigRename_UnknownProfile(t *testing.T) { + withConfig(t, &config.Config{ + Version: 1, + Profiles: map[string]config.Profile{"prod": {APIEndpoint: "https://prod.omniapp.co"}}, + }) + + cmd := configRenameCmd() + err := cmd.RunE(cmd, []string{"nope", "newname"}) + if err == nil { + t.Fatal("expected error for unknown profile, got nil") + } + if !strings.Contains(err.Error(), `"nope" not found`) { + t.Errorf("error = %q, want it to mention the missing profile", err.Error()) + } +} + +// --- config delete --- + +func TestConfigDelete_RemovesProfileWithYesFlag(t *testing.T) { + withConfig(t, &config.Config{ + Version: 1, + DefaultProfile: "prod", + Profiles: map[string]config.Profile{ + "prod": {APIEndpoint: "https://prod.omniapp.co"}, + "staging": {APIEndpoint: "https://staging.omniapp.co"}, + }, + }) + + cmd := configDeleteCmd() + if err := cmd.Flags().Set("yes", "true"); err != nil { + t.Fatalf("setting --yes flag: %v", err) + } + out := captureStdout(t, func() { + if err := cmd.RunE(cmd, []string{"prod"}); err != nil { + t.Fatalf("RunE: %v", err) + } + }) + if !strings.Contains(out, `Deleted profile "prod"`) { + t.Errorf("stdout = %q, want it to confirm deletion", out) + } + + cfg, _ := config.Load() + if _, exists := cfg.Profiles["prod"]; exists { + t.Error("prod profile still present after delete") + } + if _, exists := cfg.Profiles["staging"]; !exists { + t.Error("staging profile incorrectly deleted") + } + if cfg.DefaultProfile != "" { + t.Errorf("DefaultProfile = %q, want empty (deleted profile was the default)", cfg.DefaultProfile) + } +} + +func TestConfigDelete_UnknownProfile(t *testing.T) { + withConfig(t, &config.Config{ + Version: 1, + Profiles: map[string]config.Profile{"prod": {APIEndpoint: "https://prod.omniapp.co"}}, + }) + + cmd := configDeleteCmd() + if err := cmd.Flags().Set("yes", "true"); err != nil { + t.Fatalf("setting --yes flag: %v", err) + } + err := cmd.RunE(cmd, []string{"nope"}) + if err == nil { + t.Fatal("expected error for unknown profile, got nil") + } + if !strings.Contains(err.Error(), `"nope" not found`) { + t.Errorf("error = %q, want it to mention the missing profile", err.Error()) + } +}