From 68555a6ccae9861f1cf36cbe77005dc886014d29 Mon Sep 17 00:00:00 2001 From: Kush Date: Fri, 29 May 2026 15:22:19 -0400 Subject: [PATCH 01/23] ci: enforce gofmt via golangci-lint formatters block and reformat tree --- .golangci.yml | 4 ++++ cmd/account/register.go | 2 +- cmd/app/create.go | 7 +++---- cmd/auth/login.go | 2 +- cmd/auth/switch.go | 2 +- cmd/location/create.go | 1 - cmd/message/send.go | 16 ++++++++-------- cmd/number/service_activation.go | 16 ++++++++-------- cmd/shortcode/get.go | 2 +- cmd/site/create.go | 1 - cmd/tendlc/campaign_numbers.go | 10 +++++----- cmd/tendlc/numbers.go | 10 +++++----- cmd/tfv/submit.go | 32 ++++++++++++++++---------------- cmd/vcp/create.go | 1 - internal/api/client_test.go | 1 - internal/cmdutil/helpers.go | 1 - internal/cmdutil/numbertype.go | 8 ++++---- internal/config/config_test.go | 4 ++-- internal/output/output_test.go | 2 +- 19 files changed, 60 insertions(+), 62 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 2841fc4..35e2e52 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -4,3 +4,7 @@ linters: default: standard disable: - errcheck + +formatters: + enable: + - gofmt diff --git a/cmd/account/register.go b/cmd/account/register.go index 54b677e..7652782 100644 --- a/cmd/account/register.go +++ b/cmd/account/register.go @@ -49,7 +49,7 @@ After registration, complete account setup in your browser: 4. Go to Account > API Credentials to generate OAuth2 credentials 5. Run "band auth login" with those credentials`, Example: ` band account register --phone +19195551234 --email user@example.com --first-name John --last-name Doe`, - RunE: runRegister, + RunE: runRegister, } func runRegister(cmd *cobra.Command, args []string) error { diff --git a/cmd/app/create.go b/cmd/app/create.go index 0b4a59d..02551ca 100644 --- a/cmd/app/create.go +++ b/cmd/app/create.go @@ -107,9 +107,9 @@ func runCreate(cmd *cobra.Command, args []string) error { var result interface{} if err := client.Post(fmt.Sprintf("/accounts/%s/applications", acctID), api.XMLBody{RootElement: "Application", Data: bodyData}, &result); err != nil { if strings.Contains(err.Error(), "HTTP voice feature is required") { - return fmt.Errorf("creating voice application: this account requires the HTTP Voice feature to be enabled.\n"+ - "Contact Bandwidth support to enable it, or check if your account is on the Universal Platform.\n"+ - "If you already have VCPs configured, you may need to link a voice app to them via:\n"+ + return fmt.Errorf("creating voice application: this account requires the HTTP Voice feature to be enabled.\n" + + "Contact Bandwidth support to enable it, or check if your account is on the Universal Platform.\n" + + "If you already have VCPs configured, you may need to link a voice app to them via:\n" + " band vcp create --name --app-id ") } return fmt.Errorf("creating application: %w", err) @@ -117,4 +117,3 @@ func runCreate(cmd *cobra.Command, args []string) error { return output.StdoutAuto(format, plain, result) } - diff --git a/cmd/auth/login.go b/cmd/auth/login.go index 23f8545..2e4af3f 100644 --- a/cmd/auth/login.go +++ b/cmd/auth/login.go @@ -5,9 +5,9 @@ import ( "encoding/base64" "encoding/json" "fmt" + "github.com/spf13/cobra" "os" "strings" - "github.com/spf13/cobra" intauth "github.com/Bandwidth/cli/internal/auth" "github.com/Bandwidth/cli/internal/cmdutil" diff --git a/cmd/auth/switch.go b/cmd/auth/switch.go index af27037..80938d0 100644 --- a/cmd/auth/switch.go +++ b/cmd/auth/switch.go @@ -3,9 +3,9 @@ package auth import ( "bufio" "fmt" + "github.com/spf13/cobra" "os" "strings" - "github.com/spf13/cobra" "github.com/Bandwidth/cli/internal/cmdutil" "github.com/Bandwidth/cli/internal/config" diff --git a/cmd/location/create.go b/cmd/location/create.go index 4db37e3..0b0db85 100644 --- a/cmd/location/create.go +++ b/cmd/location/create.go @@ -62,4 +62,3 @@ func runCreate(cmd *cobra.Command, args []string) error { return output.StdoutAuto(format, plain, result) } - diff --git a/cmd/message/send.go b/cmd/message/send.go index b16ab70..7d3fbc8 100644 --- a/cmd/message/send.go +++ b/cmd/message/send.go @@ -61,14 +61,14 @@ var sendCmd = &cobra.Command{ // SendOpts holds the parameters for sending a message. type SendOpts struct { - To []string - From string - Text string - Media []string - AppID string - Tag string - Priority string - Expiration string + To []string + From string + Text string + Media []string + AppID string + Tag string + Priority string + Expiration string } // ValidateSendOpts validates the send options before making the API call. diff --git a/cmd/number/service_activation.go b/cmd/number/service_activation.go index d61f3d0..558105a 100644 --- a/cmd/number/service_activation.go +++ b/cmd/number/service_activation.go @@ -13,14 +13,14 @@ import ( // Flags shared by `number activate` and `number deactivate`. var ( - saVoiceInbound bool - saVoiceOutNational bool - saVoiceOutInternat bool - saMessaging bool - saDryRun bool - saWait bool - saTimeout time.Duration - saCustomerOrderID string + saVoiceInbound bool + saVoiceOutNational bool + saVoiceOutInternat bool + saMessaging bool + saDryRun bool + saWait bool + saTimeout time.Duration + saCustomerOrderID string ) // registerServiceActivationFlags wires the shared flag set onto a command. diff --git a/cmd/shortcode/get.go b/cmd/shortcode/get.go index d786258..61a9557 100644 --- a/cmd/shortcode/get.go +++ b/cmd/shortcode/get.go @@ -43,7 +43,7 @@ func runGet(cmd *cobra.Command, args []string) error { if apiErr, ok := err.(*api.APIError); ok { switch apiErr.StatusCode { case 403: - return fmt.Errorf("access denied — your credentials may not have short code access.\n"+ + return fmt.Errorf("access denied — your credentials may not have short code access.\n" + "Contact your Bandwidth account manager to verify") case 404: return fmt.Errorf("short code %s not found for country %s on this account", args[0], getCountry) diff --git a/cmd/site/create.go b/cmd/site/create.go index d9166bc..0e79add 100644 --- a/cmd/site/create.go +++ b/cmd/site/create.go @@ -62,4 +62,3 @@ func runCreate(cmd *cobra.Command, args []string) error { return output.StdoutAuto(format, plain, result) } - diff --git a/cmd/tendlc/campaign_numbers.go b/cmd/tendlc/campaign_numbers.go index 767f5c8..e10dc95 100644 --- a/cmd/tendlc/campaign_numbers.go +++ b/cmd/tendlc/campaign_numbers.go @@ -22,12 +22,12 @@ func init() { } var campaignNumbersCmd = &cobra.Command{ - Use: "numbers ", - Short: "List phone numbers assigned to a campaign", - Long: "Shows all phone numbers associated with a specific 10DLC campaign, including numbers with provisioning errors.", + Use: "numbers ", + Short: "List phone numbers assigned to a campaign", + Long: "Shows all phone numbers associated with a specific 10DLC campaign, including numbers with provisioning errors.", Example: ` band tendlc campaigns numbers CR8HFN0`, - Args: cobra.ExactArgs(1), - RunE: runCampaignNumbers, + Args: cobra.ExactArgs(1), + RunE: runCampaignNumbers, } func runCampaignNumbers(cmd *cobra.Command, args []string) error { diff --git a/cmd/tendlc/numbers.go b/cmd/tendlc/numbers.go index a458eea..70d64a0 100644 --- a/cmd/tendlc/numbers.go +++ b/cmd/tendlc/numbers.go @@ -68,12 +68,12 @@ func runNumbers(cmd *cobra.Command, args []string) error { } var numberGetCmd = &cobra.Command{ - Use: "number ", - Short: "Get 10DLC registration details for a phone number", - Long: "Shows the 10DLC registration status, campaign assignment, and brand for a specific phone number.", + Use: "number ", + Short: "Get 10DLC registration details for a phone number", + Long: "Shows the 10DLC registration status, campaign assignment, and brand for a specific phone number.", Example: ` band tendlc number +19195551234`, - Args: cobra.ExactArgs(1), - RunE: runNumberGet, + Args: cobra.ExactArgs(1), + RunE: runNumberGet, } func runNumberGet(cmd *cobra.Command, args []string) error { diff --git a/cmd/tfv/submit.go b/cmd/tfv/submit.go index daf5bd3..53b4150 100644 --- a/cmd/tfv/submit.go +++ b/cmd/tfv/submit.go @@ -12,22 +12,22 @@ import ( ) var ( - submitBusinessName string - submitBusinessAddr string - submitBusinessCity string - submitBusinessState string - submitBusinessZip string - submitContactFirst string - submitContactLast string - submitContactEmail string - submitContactPhone string - submitMessageVolume int - submitUseCase string - submitUseCaseSummary string - submitSampleMessage string - submitPrivacyURL string - submitTermsURL string - submitEntityType string + submitBusinessName string + submitBusinessAddr string + submitBusinessCity string + submitBusinessState string + submitBusinessZip string + submitContactFirst string + submitContactLast string + submitContactEmail string + submitContactPhone string + submitMessageVolume int + submitUseCase string + submitUseCaseSummary string + submitSampleMessage string + submitPrivacyURL string + submitTermsURL string + submitEntityType string ) func init() { diff --git a/cmd/vcp/create.go b/cmd/vcp/create.go index 3188286..b0a67d4 100644 --- a/cmd/vcp/create.go +++ b/cmd/vcp/create.go @@ -91,4 +91,3 @@ func runCreate(cmd *cobra.Command, args []string) error { return output.StdoutAuto(format, plain, result) } - diff --git a/internal/api/client_test.go b/internal/api/client_test.go index 4b22406..c500303 100644 --- a/internal/api/client_test.go +++ b/internal/api/client_test.go @@ -10,7 +10,6 @@ import ( "github.com/Bandwidth/cli/internal/auth" ) - func TestClient_Get(t *testing.T) { type response struct { Name string `json:"name"` diff --git a/internal/cmdutil/helpers.go b/internal/cmdutil/helpers.go index d727bf9..7913cf8 100644 --- a/internal/cmdutil/helpers.go +++ b/internal/cmdutil/helpers.go @@ -180,4 +180,3 @@ func PlatformClient(accountIDOverride string) (*api.Client, string, error) { func MessagingClient(accountIDOverride string) (*api.Client, string, error) { return BuildClient("https://messaging.bandwidth.com/api/v2", accountIDOverride) } - diff --git a/internal/cmdutil/numbertype.go b/internal/cmdutil/numbertype.go index 9c0e0a1..3d979b4 100644 --- a/internal/cmdutil/numbertype.go +++ b/internal/cmdutil/numbertype.go @@ -6,10 +6,10 @@ import "strings" type NumberType int const ( - NumberTypeUnknown NumberType = iota - NumberType10DLC // Local 10-digit US/CA number - NumberTypeTollFree // US/CA toll-free (800, 888, 877, 866, 855, 844, 833) - NumberTypeShortCode // 5-6 digit short code + NumberTypeUnknown NumberType = iota + NumberType10DLC // Local 10-digit US/CA number + NumberTypeTollFree // US/CA toll-free (800, 888, 877, 866, 855, 844, 833) + NumberTypeShortCode // 5-6 digit short code ) func (t NumberType) String() string { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 81da655..2c9a065 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -302,7 +302,7 @@ func TestHasMultipleEnvironments(t *testing.T) { { name: "prod and custom", profiles: map[string]*Profile{ - "default": {ClientID: "id1", Environment: ""}, + "default": {ClientID: "id1", Environment: ""}, "secondary": {ClientID: "id2", Environment: "custom"}, }, want: true, @@ -310,7 +310,7 @@ func TestHasMultipleEnvironments(t *testing.T) { { name: "test and custom", profiles: map[string]*Profile{ - "test": {ClientID: "id1", Environment: "test"}, + "test": {ClientID: "id1", Environment: "test"}, "custom": {ClientID: "id2", Environment: "custom"}, }, want: true, diff --git a/internal/output/output_test.go b/internal/output/output_test.go index a4b4421..d638d2a 100644 --- a/internal/output/output_test.go +++ b/internal/output/output_test.go @@ -387,7 +387,7 @@ func TestFlattenResponse_NumberList(t *testing.T) { input := map[string]interface{}{ "TNs": map[string]interface{}{ "TelephoneNumbers": map[string]interface{}{ - "Count": "3", + "Count": "3", "TelephoneNumber": []interface{}{"+19191234567", "+19197654321", "+19190001111"}, }, "TotalCount": "3", From 70b096d5120131c38e2f75c425d85404df0d60c4 Mon Sep 17 00:00:00 2001 From: Kush Date: Fri, 29 May 2026 15:25:07 -0400 Subject: [PATCH 02/23] fix(wait): map --wait timeouts to exit code 5 via ErrPollTimeout sentinel --- internal/cmdutil/exitcodes.go | 3 ++ internal/cmdutil/exitcodes_test.go | 1 + internal/cmdutil/poll.go | 8 ++++- internal/cmdutil/poll_test.go | 50 ++++++++++++++++++++++++++++++ 4 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 internal/cmdutil/poll_test.go diff --git a/internal/cmdutil/exitcodes.go b/internal/cmdutil/exitcodes.go index 09c8eca..e67d370 100644 --- a/internal/cmdutil/exitcodes.go +++ b/internal/cmdutil/exitcodes.go @@ -28,6 +28,9 @@ func ExitCodeForError(err error) int { if err == nil { return ExitOK } + if errors.Is(err, ErrPollTimeout) { + return ExitTimeout + } var fle *FeatureLimitError if errors.As(err, &fle) { return ExitConflict diff --git a/internal/cmdutil/exitcodes_test.go b/internal/cmdutil/exitcodes_test.go index d22970e..72736a4 100644 --- a/internal/cmdutil/exitcodes_test.go +++ b/internal/cmdutil/exitcodes_test.go @@ -26,6 +26,7 @@ func TestExitCodeForError(t *testing.T) { {"feature limit wraps 403", NewFeatureLimit("nope", &api.APIError{StatusCode: 403}), ExitConflict}, {"feature limit precedence beats raw 401", NewFeatureLimit("nope", &api.APIError{StatusCode: 401}), ExitConflict}, {"wrapped 429 keeps rate limit", fmt.Errorf("wrap: %w", &api.APIError{StatusCode: 429}), ExitRateLimit}, + {"wrapped ErrPollTimeout", fmt.Errorf("timed out: %w", ErrPollTimeout), ExitTimeout}, } for _, tt := range tests { diff --git a/internal/cmdutil/poll.go b/internal/cmdutil/poll.go index 275522e..37e7f79 100644 --- a/internal/cmdutil/poll.go +++ b/internal/cmdutil/poll.go @@ -1,10 +1,16 @@ package cmdutil import ( + "errors" "fmt" "time" ) +// ErrPollTimeout is returned (wrapped) by Poll when cfg.Timeout elapses before +// Check reports done. ExitCodeForError maps it to ExitTimeout (5) so agents can +// distinguish "still running, re-poll" from a hard failure. +var ErrPollTimeout = errors.New("operation did not complete in time") + // PollConfig configures a polling loop. type PollConfig struct { Interval time.Duration @@ -29,7 +35,7 @@ func Poll(cfg PollConfig) (interface{}, error) { return result, nil } if time.Now().After(deadline) { - return nil, fmt.Errorf("timed out after %s waiting for operation to complete", cfg.Timeout) + return nil, fmt.Errorf("timed out after %s: %w", cfg.Timeout, ErrPollTimeout) } time.Sleep(cfg.Interval) } diff --git a/internal/cmdutil/poll_test.go b/internal/cmdutil/poll_test.go new file mode 100644 index 0000000..3ac1906 --- /dev/null +++ b/internal/cmdutil/poll_test.go @@ -0,0 +1,50 @@ +package cmdutil + +import ( + "errors" + "testing" + "time" +) + +func TestPollReturnsErrPollTimeout(t *testing.T) { + _, err := Poll(PollConfig{ + Interval: time.Millisecond, + Timeout: 5 * time.Millisecond, + Check: func() (bool, interface{}, error) { + return false, nil, nil // never done + }, + }) + if !errors.Is(err, ErrPollTimeout) { + t.Fatalf("expected ErrPollTimeout, got %v", err) + } +} + +func TestPollReturnsResultWhenDone(t *testing.T) { + got, err := Poll(PollConfig{ + Interval: time.Millisecond, + Timeout: time.Second, + Check: func() (bool, interface{}, error) { + return true, "done", nil + }, + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != "done" { + t.Fatalf("got %v, want done", got) + } +} + +func TestPollPropagatesCheckError(t *testing.T) { + sentinel := errors.New("boom") + _, err := Poll(PollConfig{ + Interval: time.Millisecond, + Timeout: time.Second, + Check: func() (bool, interface{}, error) { + return false, nil, sentinel + }, + }) + if !errors.Is(err, sentinel) { + t.Fatalf("expected boom, got %v", err) + } +} From 3be4e8f44a2a864969f421e4cc639984e56c3c5b Mon Sep 17 00:00:00 2001 From: Kush Date: Fri, 29 May 2026 15:30:08 -0400 Subject: [PATCH 03/23] fix(messaging): route messaging client through environment-aware host --- AGENTS.md | 1 + README.md | 1 + cmd/message/media/upload.go | 2 +- internal/cmdutil/helpers.go | 20 +++++++++++++++- internal/cmdutil/helpers_test.go | 39 ++++++++++++++++++++++++++++++++ 5 files changed, 61 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 97f5b7c..c35b84d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -79,6 +79,7 @@ This is stderr only — it won't break piped output parsing. | `BW_ENVIRONMENT` | API environment: `prod` (default), `test` | | `BW_API_URL` | Override API base URL (overrides environment-based default) | | `BW_VOICE_URL` | Override Voice API base URL (overrides environment-based default) | +| `BW_MESSAGING_URL` | Override Messaging API base URL (overrides environment-based default) | | `BW_FORMAT` | Output format override | **Config file location:** `~/.config/band/config.json` (XDG). Falls back to `~/.band/config.json` if the XDG path doesn't exist. diff --git a/README.md b/README.md index a4e53e2..d8682c7 100644 --- a/README.md +++ b/README.md @@ -498,6 +498,7 @@ Sub-accounts (formerly known as sites) are the top-level container. Locations (f | `BW_FORMAT` | Default output format | | `BW_API_URL` | Override the API base URL | | `BW_VOICE_URL` | Override the Voice API base URL | +| `BW_MESSAGING_URL` | Override the Messaging API base URL (for testing or local proxies) | --- diff --git a/cmd/message/media/upload.go b/cmd/message/media/upload.go index 0624346..5f26638 100644 --- a/cmd/message/media/upload.go +++ b/cmd/message/media/upload.go @@ -72,7 +72,7 @@ func runUpload(cmd *cobra.Command, args []string) error { } // Print the media URL that can be used with `message send --media` - mediaURL := fmt.Sprintf("https://messaging.bandwidth.com/api/v2/users/%s/media/%s", acctID, mediaID) + mediaURL := fmt.Sprintf("%s/users/%s/media/%s", client.BaseURL, acctID, mediaID) fmt.Fprintln(cmd.OutOrStdout(), mediaURL) ui.Successf("Use with: band message send --media %s", mediaURL) return nil diff --git a/internal/cmdutil/helpers.go b/internal/cmdutil/helpers.go index 7913cf8..b1d985b 100644 --- a/internal/cmdutil/helpers.go +++ b/internal/cmdutil/helpers.go @@ -41,6 +41,20 @@ func voiceHostForEnvironment(env string) string { } } +// messagingHostForEnvironment maps an environment name to its Messaging API host. +// Non-production environments can be overridden with BW_MESSAGING_URL. +func messagingHostForEnvironment(env string) string { + if v := os.Getenv("BW_MESSAGING_URL"); v != "" { + return strings.TrimRight(v, "/") + } + switch env { + case "test", "uat": + return "https://test.messaging.bandwidth.com" + default: + return "https://messaging.bandwidth.com" + } +} + // loadConfigAndAuth loads the config, retrieves the client secret, and returns // everything needed to build an API client. func loadConfigAndAuth() (*config.Config, *config.Profile, string, error) { @@ -178,5 +192,9 @@ func PlatformClient(accountIDOverride string) (*api.Client, string, error) { // MessagingClient returns a client for the Bandwidth Messaging API v2. func MessagingClient(accountIDOverride string) (*api.Client, string, error) { - return BuildClient("https://messaging.bandwidth.com/api/v2", accountIDOverride) + tm, acctID, env, err := authenticate(accountIDOverride) + if err != nil { + return nil, "", err + } + return api.NewClient(messagingHostForEnvironment(env)+"/api/v2", tm), acctID, nil } diff --git a/internal/cmdutil/helpers_test.go b/internal/cmdutil/helpers_test.go index 224fc6d..8c0e6fb 100644 --- a/internal/cmdutil/helpers_test.go +++ b/internal/cmdutil/helpers_test.go @@ -41,6 +41,45 @@ func TestVoiceHostForEnvironment_BW_VOICE_URL_TrailingSlash(t *testing.T) { } } +func TestMessagingHostForEnvironment(t *testing.T) { + tests := []struct { + name string + env string + want string + }{ + {"prod default", "", "https://messaging.bandwidth.com"}, + {"prod explicit", "prod", "https://messaging.bandwidth.com"}, + {"unknown env falls back to prod", "staging", "https://messaging.bandwidth.com"}, + {"test", "test", "https://test.messaging.bandwidth.com"}, + {"uat", "uat", "https://test.messaging.bandwidth.com"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := messagingHostForEnvironment(tt.env); got != tt.want { + t.Errorf("messagingHostForEnvironment(%q) = %q, want %q", tt.env, got, tt.want) + } + }) + } +} + +func TestMessagingHostForEnvironment_BW_MESSAGING_URL(t *testing.T) { + t.Setenv("BW_MESSAGING_URL", "https://custom.messaging.example.com") + for _, env := range []string{"", "prod", "test"} { + got := messagingHostForEnvironment(env) + if got != "https://custom.messaging.example.com" { + t.Errorf("messagingHostForEnvironment(%q) with BW_MESSAGING_URL = %q, want override", env, got) + } + } +} + +func TestMessagingHostForEnvironment_BW_MESSAGING_URL_TrailingSlash(t *testing.T) { + t.Setenv("BW_MESSAGING_URL", "http://localhost:8080/") + got := messagingHostForEnvironment("") + if got != "http://localhost:8080" { + t.Errorf("messagingHostForEnvironment with trailing slash = %q, want without slash", got) + } +} + func TestAPIHostForEnvironment(t *testing.T) { tests := []struct { name string From 8ceacbac7dd66dd0755b7a8d010121389bf9bee1 Mon Sep 17 00:00:00 2001 From: Kush Date: Fri, 29 May 2026 15:34:38 -0400 Subject: [PATCH 04/23] refactor(cmdutil): make VoiceClient an overridable var returning api.Requester --- internal/cmdutil/client_seam.go | 10 ++++++++++ internal/cmdutil/helpers.go | 7 +++++-- 2 files changed, 15 insertions(+), 2 deletions(-) create mode 100644 internal/cmdutil/client_seam.go diff --git a/internal/cmdutil/client_seam.go b/internal/cmdutil/client_seam.go new file mode 100644 index 0000000..a43e728 --- /dev/null +++ b/internal/cmdutil/client_seam.go @@ -0,0 +1,10 @@ +package cmdutil + +import "github.com/Bandwidth/cli/internal/api" + +// ClientFunc builds an authenticated API client for an optional account-id +// override, returning the resolved account ID. The client constructors +// (e.g. VoiceClient) are vars of this type so tests can substitute a fake +// that implements api.Requester. api.Requester is the interface *api.Client +// already satisfies. +type ClientFunc func(accountIDOverride string) (api.Requester, string, error) diff --git a/internal/cmdutil/helpers.go b/internal/cmdutil/helpers.go index b1d985b..2e0cc63 100644 --- a/internal/cmdutil/helpers.go +++ b/internal/cmdutil/helpers.go @@ -172,8 +172,7 @@ func DashboardClient(accountIDOverride string) (*api.Client, string, error) { return api.NewXMLClient(apiHostForEnvironment(env)+"/api/v2", tm), acctID, nil } -// VoiceClient returns a client for the Bandwidth Voice API v2. -func VoiceClient(accountIDOverride string) (*api.Client, string, error) { +func voiceClient(accountIDOverride string) (api.Requester, string, error) { tm, acctID, env, err := authenticate(accountIDOverride) if err != nil { return nil, "", err @@ -181,6 +180,10 @@ func VoiceClient(accountIDOverride string) (*api.Client, string, error) { return api.NewClient(voiceHostForEnvironment(env)+"/api/v2", tm), acctID, nil } +// VoiceClient returns a client for the Bandwidth Voice API v2. +// It is a var so tests can substitute a fake that implements api.Requester. +var VoiceClient ClientFunc = voiceClient + // PlatformClient creates a JSON API client for Universal Platform v2 endpoints (e.g. VCP). func PlatformClient(accountIDOverride string) (*api.Client, string, error) { tm, acctID, env, err := authenticate(accountIDOverride) From b5e3f999037c80011e9e11d7407c83c2f6901f9a Mon Sep 17 00:00:00 2001 From: Kush Date: Fri, 29 May 2026 15:41:31 -0400 Subject: [PATCH 05/23] test: add golden --plain contract tests for call list and recording list --- cmd/call/golden_test.go | 98 ++++++++++++++++++++++++++++++++++++ cmd/recording/golden_test.go | 96 +++++++++++++++++++++++++++++++++++ 2 files changed, 194 insertions(+) create mode 100644 cmd/call/golden_test.go create mode 100644 cmd/recording/golden_test.go diff --git a/cmd/call/golden_test.go b/cmd/call/golden_test.go new file mode 100644 index 0000000..a1624b1 --- /dev/null +++ b/cmd/call/golden_test.go @@ -0,0 +1,98 @@ +package call + +import ( + "bytes" + "encoding/json" + "io" + "os" + "testing" + + "github.com/spf13/cobra" + + "github.com/Bandwidth/cli/internal/api" + "github.com/Bandwidth/cli/internal/cmdutil" +) + +// fakeClient implements api.Requester. Get writes a canned fixture. +type fakeClient struct { + getResult interface{} +} + +func (f *fakeClient) Get(path string, result interface{}) error { + b, _ := json.Marshal(f.getResult) + return json.Unmarshal(b, result) +} +func (f *fakeClient) Post(string, interface{}, interface{}) error { return nil } +func (f *fakeClient) Put(string, interface{}, interface{}) error { return nil } +func (f *fakeClient) Patch(string, interface{}, interface{}) error { return nil } +func (f *fakeClient) Delete(string, interface{}) error { return nil } +func (f *fakeClient) GetRaw(string) ([]byte, error) { return nil, nil } +func (f *fakeClient) PutRaw(string, []byte, string) error { return nil } + +// newTestRoot builds a minimal root with the persistent flags commands read. +func newTestRoot(child *cobra.Command) *cobra.Command { + root := &cobra.Command{Use: "band", SilenceUsage: true, SilenceErrors: true} + root.PersistentFlags().String("format", "json", "") + root.PersistentFlags().Bool("plain", false, "") + root.PersistentFlags().String("account-id", "", "") + root.PersistentFlags().String("environment", "", "") + root.AddCommand(child) + return root +} + +// captureStdout runs fn while capturing everything written to os.Stdout. +func captureStdout(t *testing.T, fn func()) string { + t.Helper() + orig := os.Stdout + r, w, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + os.Stdout = w + fn() + w.Close() + os.Stdout = orig + var out []byte + out, err = io.ReadAll(r) + if err != nil { + t.Fatal(err) + } + return string(out) +} + +func TestCallListPlainOutput(t *testing.T) { + // Fixture is an API-shaped WRAPPER object ({"calls": [...]}), so a passing + // assertion on got[0]["callId"] proves FlattenResponse stripped the wrapper. + // No t.Parallel(): these tests mutate the global cmdutil.VoiceClient. + orig := cmdutil.VoiceClient + t.Cleanup(func() { cmdutil.VoiceClient = orig }) + cmdutil.VoiceClient = func(string) (api.Requester, string, error) { + return &fakeClient{getResult: map[string]interface{}{ + "calls": []interface{}{ + map[string]interface{}{"callId": "c-1", "state": "active"}, + }, + }}, "acct-123", nil + } + + root := newTestRoot(listCmd) + root.SetArgs([]string{"list", "--plain"}) + + out := captureStdout(t, func() { + if err := root.Execute(); err != nil { + t.Fatalf("execute: %v", err) + } + }) + + var got []map[string]interface{} + if err := json.Unmarshal(bytes.TrimSpace([]byte(out)), &got); err != nil { + t.Fatalf("plain output is not a JSON array: %q (%v)", out, err) + } + if len(got) != 1 || got[0]["callId"] != "c-1" { + t.Fatalf("flatten/normalize did not produce the expected array: %q", out) + } + + want := "[\n {\n \"callId\": \"c-1\",\n \"state\": \"active\"\n }\n]\n" + if out != want { + t.Fatalf("golden mismatch:\n got: %q\nwant: %q", out, want) + } +} diff --git a/cmd/recording/golden_test.go b/cmd/recording/golden_test.go new file mode 100644 index 0000000..3e62b31 --- /dev/null +++ b/cmd/recording/golden_test.go @@ -0,0 +1,96 @@ +package recording + +import ( + "bytes" + "encoding/json" + "io" + "os" + "testing" + + "github.com/spf13/cobra" + + "github.com/Bandwidth/cli/internal/api" + "github.com/Bandwidth/cli/internal/cmdutil" +) + +// fakeClient implements api.Requester. Get writes a canned fixture. +type fakeClient struct { + getResult interface{} +} + +func (f *fakeClient) Get(path string, result interface{}) error { + b, _ := json.Marshal(f.getResult) + return json.Unmarshal(b, result) +} +func (f *fakeClient) Post(string, interface{}, interface{}) error { return nil } +func (f *fakeClient) Put(string, interface{}, interface{}) error { return nil } +func (f *fakeClient) Patch(string, interface{}, interface{}) error { return nil } +func (f *fakeClient) Delete(string, interface{}) error { return nil } +func (f *fakeClient) GetRaw(string) ([]byte, error) { return nil, nil } +func (f *fakeClient) PutRaw(string, []byte, string) error { return nil } + +// newTestRoot builds a minimal root with the persistent flags commands read. +func newTestRoot(child *cobra.Command) *cobra.Command { + root := &cobra.Command{Use: "band", SilenceUsage: true, SilenceErrors: true} + root.PersistentFlags().String("format", "json", "") + root.PersistentFlags().Bool("plain", false, "") + root.PersistentFlags().String("account-id", "", "") + root.PersistentFlags().String("environment", "", "") + root.AddCommand(child) + return root +} + +// captureStdout runs fn while capturing everything written to os.Stdout. +func captureStdout(t *testing.T, fn func()) string { + t.Helper() + orig := os.Stdout + r, w, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + os.Stdout = w + fn() + w.Close() + os.Stdout = orig + var out []byte + out, err = io.ReadAll(r) + if err != nil { + t.Fatal(err) + } + return string(out) +} + +func TestRecordingListPlainOutput(t *testing.T) { + // No t.Parallel(): these tests mutate the global cmdutil.VoiceClient. + orig := cmdutil.VoiceClient + t.Cleanup(func() { cmdutil.VoiceClient = orig }) + cmdutil.VoiceClient = func(string) (api.Requester, string, error) { + return &fakeClient{getResult: map[string]interface{}{ + "recordings": []interface{}{ + map[string]interface{}{"recordingId": "r-1", "status": "complete"}, + }, + }}, "acct-123", nil + } + + root := newTestRoot(listCmd) + root.SetArgs([]string{"list", "c-abc123", "--plain"}) // callId positional arg required + + out := captureStdout(t, func() { + if err := root.Execute(); err != nil { + t.Fatalf("execute: %v", err) + } + }) + + var got []map[string]interface{} + if err := json.Unmarshal(bytes.TrimSpace([]byte(out)), &got); err != nil { + t.Fatalf("plain output is not a JSON array: %q (%v)", out, err) + } + if len(got) != 1 || got[0]["recordingId"] != "r-1" { + t.Fatalf("flatten/normalize did not produce the expected array: %q", out) + } + + want := "[\n {\n \"recordingId\": \"r-1\",\n \"status\": \"complete\"\n }\n]\n" + if out != want { + t.Fatalf("golden mismatch:\n got: %q\nwant: %q", out, want) + } +} From afd8a54bbc52d31482936fef961bbd8ef41de482 Mon Sep 17 00:00:00 2001 From: Kush Date: Fri, 29 May 2026 15:50:52 -0400 Subject: [PATCH 06/23] ci: enforce documented command/flag correctness, drop toothless docs-check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds cmd/doccontract_test.go (package cmd) which parses every `band …` usage in README.md and AGENTS.md and asserts the command path and flags exist in the live Cobra tree. Known rename-tracked drift (--site vs --subaccount on location create) is listed in knownDrift. Removes the docs-check CI job that only warned on file-diff heuristics; the new test is a hard gate that runs as part of `go test ./...` in the existing test job on all three OS targets. --- .github/workflows/ci.yml | 34 +------- cmd/doccontract_test.go | 169 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 171 insertions(+), 32 deletions(-) create mode 100644 cmd/doccontract_test.go diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5afde80..b4dffc4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -7,6 +7,8 @@ on: branches: [main] jobs: + # Doc/flag correctness is enforced as a hard gate by cmd/doccontract_test.go, + # which runs as part of `go test ./...` below. test: strategy: matrix: @@ -57,35 +59,3 @@ jobs: - name: Run govulncheck run: govulncheck ./... - - docs-check: - if: github.event_name == 'pull_request' - runs-on: ubuntu-latest - steps: - - name: Checkout - uses: actions/checkout@v6 - with: - fetch-depth: 0 - - - name: Check for doc updates - run: | - BASE=${{ github.event.pull_request.base.sha }} - HEAD=${{ github.event.pull_request.head.sha }} - CHANGED=$(git diff --name-only "$BASE" "$HEAD") - - CODE_CHANGED=false - DOCS_CHANGED=false - - # Check if command surface or flags changed - if echo "$CHANGED" | grep -qE '^cmd/|^internal/cmdutil/'; then - CODE_CHANGED=true - fi - - # Check if any docs were touched - if echo "$CHANGED" | grep -qE '^README\.md$|^AGENTS\.md$'; then - DOCS_CHANGED=true - fi - - if [ "$CODE_CHANGED" = true ] && [ "$DOCS_CHANGED" = false ]; then - echo "::warning::Command code changed without documentation updates. If this PR adds, removes, or changes commands/flags, please update README.md and/or AGENTS.md." - fi diff --git a/cmd/doccontract_test.go b/cmd/doccontract_test.go new file mode 100644 index 0000000..77d15c0 --- /dev/null +++ b/cmd/doccontract_test.go @@ -0,0 +1,169 @@ +package cmd + +import ( + "os" + "regexp" + "strings" + "testing" + + "github.com/spf13/cobra" +) + +// knownDrift lists "command path::flag" entries intentionally not yet reconciled. +// REMOVE entries here as the underlying drift is fixed. +// - "location create::subaccount": tracked by the --site→--subaccount rename +// spec (docs/superpowers/specs/2026-05-07-subaccount-rename.md). Remove when +// that PR lands. +var knownDrift = map[string]bool{ + "location create::subaccount": true, +} + +// knownDriftCommands lists command paths documented in tables that are known +// rename-related drift and intentionally not yet reconciled. +// REMOVE entries here as the underlying drift is fixed. +var knownDriftCommands = map[string]bool{} + +// bandUsageRe captures everything after "band " to end of line (GREEDY — a +// non-greedy capture would stop at the first space and truncate multi-word +// command paths like "message send" down to "message"). Tokenization below +// then trims the capture down to the command path. +var bandUsageRe = regexp.MustCompile(`\bband ([a-z].*)$`) +var flagRe = regexp.MustCompile(`--([a-z][\w-]*)`) + +// backtickBandRe matches a backtick-quoted span whose content starts with "band ": +// e.g. `band app list` or `band call get `. Used to extract the command +// in column 1 of markdown table rows. +var backtickBandRe = regexp.MustCompile("`(band [^`]+)`") + +// commandTokenRe matches a token that can be part of a command path: a +// lowercase command/subcommand name. Flags (--x), placeholders (/[x]), +// IDs (c-abc123), phone numbers (+1...), and shell operators all fail to match, +// so the first non-matching token ends the command path. +var commandTokenRe = regexp.MustCompile(`^[a-z][a-z-]*$`) + +// resolveCommand walks rootCmd by the command-path tokens, descending into +// subcommands. It returns the deepest command matched and how many leading +// tokens were matched as ACTUAL subcommands. matched==0 means the first token +// is not a real subcommand of `band`. +func resolveCommand(path []string) (cmd *cobra.Command, matched int) { + cur := rootCmd + for _, tok := range path { + next, _, err := cur.Find([]string{tok}) + if err != nil || next == cur { + break // positional arg, or (at depth 0) a bogus command name + } + cur = next + matched++ + } + return cur, matched +} + +func flagExists(c *cobra.Command, name string) bool { + if c.Flags().Lookup(name) != nil { + return true + } + if c.InheritedFlags().Lookup(name) != nil { + return true + } + return rootCmd.PersistentFlags().Lookup(name) != nil +} + +func TestDocumentedCommandsAndFlagsExist(t *testing.T) { + for _, doc := range []string{"../README.md", "../AGENTS.md"} { + raw, err := os.ReadFile(doc) + if err != nil { + t.Fatalf("reading %s: %v", doc, err) + } + for _, line := range strings.Split(string(raw), "\n") { + // Table rows: only the command in column 1 is validated for existence; + // description-column flags are intentionally NOT checked (they're prose + // mentions, not usage). This avoids false positives from flags named in + // the "What it does" column being attributed to the column-1 command. + if strings.HasPrefix(strings.TrimSpace(line), "|") { + // Split on "|" and take the first non-empty cell (column 1). + var col1 string + for _, cell := range strings.Split(line, "|") { + if strings.TrimSpace(cell) != "" { + col1 = cell + break + } + } + // Look for a backtick-quoted `band ...` span in column 1 only. + bm := backtickBandRe.FindStringSubmatch(col1) + if bm == nil { + continue // not a command row (e.g. header or non-command cell) + } + // bm[1] is the content inside the backticks, e.g. "band app list" + // Strip the leading "band " and tokenize the command path. + rest := strings.TrimPrefix(bm[1], "band ") + fields := strings.Fields(rest) + var path []string + for _, f := range fields { + if !commandTokenRe.MatchString(f) { + break + } + path = append(path, f) + } + if len(path) == 0 { + continue + } + cmdName := strings.Join(path, " ") + if knownDriftCommands[cmdName] { + continue + } + _, matched := resolveCommand(path) + if matched == 0 { + t.Errorf("%s documents `band %s …` but %q is not a command under `band`", doc, cmdName, path[0]) + } + continue + } + + m := bandUsageRe.FindStringSubmatch(line) + if m == nil { + continue + } + // Restrict flag extraction to the text captured after "band " and + // trim at the first backtick — commands in inline code spans like + // `MEDIA_URL=$(band ... image.png)` then pass to `--flag` would + // otherwise attribute the trailing flag to the wrong command. + capture := m[1] + if idx := strings.IndexByte(capture, '`'); idx >= 0 { + capture = capture[:idx] + } + + // Command path = leading command-name tokens before the first + // flag/placeholder/arg. + fields := strings.Fields(capture) + var path []string + for _, f := range fields { + if !commandTokenRe.MatchString(f) { + break + } + path = append(path, f) + } + if len(path) == 0 { + continue + } + cmd, matched := resolveCommand(path) + cmdName := strings.Join(path, " ") + if matched == 0 { + t.Errorf("%s documents `band %s …` but %q is not a command under `band`", doc, cmdName, path[0]) + continue + } + for _, fm := range flagRe.FindAllStringSubmatch(capture, -1) { + flag := fm[1] + // Cobra auto-injects --help on every command; skip it. + if flag == "help" { + continue + } + if knownDrift[cmdName+"::"+flag] { + continue + } + if !flagExists(cmd, flag) { + t.Errorf("%s documents `band %s --%s` but that flag does not exist on command %q", + doc, cmdName, flag, cmd.CommandPath()) + } + } + } + } +} From 957370da564703fe9cf2ce32c335ce0c6d9c7a17 Mon Sep 17 00:00:00 2001 From: Kush Date: Fri, 29 May 2026 16:04:36 -0400 Subject: [PATCH 07/23] fix(quickstart): correct number-order body, scope legacy orders to created sub-account --- cmd/quickstart/quickstart.go | 34 ++++++++++++++++++------------- cmd/quickstart/quickstart_test.go | 30 +++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 14 deletions(-) diff --git a/cmd/quickstart/quickstart.go b/cmd/quickstart/quickstart.go index c680ebe..c7dfd1a 100644 --- a/cmd/quickstart/quickstart.go +++ b/cmd/quickstart/quickstart.go @@ -121,7 +121,7 @@ func runVCPQuickstart(cmd *cobra.Command) error { ui.Successf("VCP: %s", ui.ID(vcpID)) // Step 3: Search and order a number - phoneNumber, err := searchAndOrderNumber(dashClient, acctID) + phoneNumber, err := searchAndOrderNumber(dashClient, acctID, "") if err != nil { result.Status = "complete_no_number" ui.Warnf("%v", err) @@ -226,7 +226,7 @@ func runLegacyQuickstart(cmd *cobra.Command) error { ui.Successf("Application: %s", ui.ID(appID)) // Step 4: Search and order a number - phoneNumber, err := searchAndOrderNumber(client, acctID) + phoneNumber, err := searchAndOrderNumber(client, acctID, siteID) if err != nil { result.Status = "complete_no_number" ui.Warnf("%v", err) @@ -247,7 +247,23 @@ func runLegacyQuickstart(cmd *cobra.Command) error { return printResult(result) } -func searchAndOrderNumber(client *api.Client, acctID string) (string, error) { +// buildQuickstartOrderBody mirrors number.BuildOrderBody — a top-level +// TelephoneNumberList with no order-type wrapper, exactly like the proven +// `number order` command. siteID scopes the order to a sub-account on the +// legacy path; pass "" on the VCP path (no sub-account) to omit SiteId. +func buildQuickstartOrderBody(phoneNumber, siteID string) api.XMLBody { + data := map[string]interface{}{ + "TelephoneNumberList": map[string]interface{}{ + "TelephoneNumber": []string{phoneNumber}, // []string, exactly like number.BuildOrderBody + }, + } + if siteID != "" { + data["SiteId"] = siteID + } + return api.XMLBody{RootElement: "Order", Data: data} +} + +func searchAndOrderNumber(client *api.Client, acctID, siteID string) (string, error) { searchSpin := ui.NewSpinner(fmt.Sprintf("Searching for number in area code %s...", qsAreaCode)) searchSpin.Start() var searchResp interface{} @@ -265,17 +281,7 @@ func searchAndOrderNumber(client *api.Client, acctID string) (string, error) { orderSpin := ui.NewSpinner(fmt.Sprintf("Ordering %s...", phoneNumber)) orderSpin.Start() var orderResp interface{} - orderBody := api.XMLBody{ - RootElement: "Order", - Data: map[string]interface{}{ - "ExistingTelephoneNumberOrderType": map[string]interface{}{ - "TelephoneNumberList": map[string]interface{}{ - "TelephoneNumber": phoneNumber, - }, - }, - "SiteId": acctID, // orders need a site ID; for VCP path this may need adjustment - }, - } + orderBody := buildQuickstartOrderBody(phoneNumber, siteID) orderErr := client.Post(fmt.Sprintf("/accounts/%s/orders", acctID), orderBody, &orderResp) orderSpin.Stop() if orderErr != nil { diff --git a/cmd/quickstart/quickstart_test.go b/cmd/quickstart/quickstart_test.go index 0a8d4f0..6be2530 100644 --- a/cmd/quickstart/quickstart_test.go +++ b/cmd/quickstart/quickstart_test.go @@ -128,6 +128,36 @@ func TestExtractPhoneNumber(t *testing.T) { } } +func TestBuildQuickstartOrderBody(t *testing.T) { + t.Run("mirrors number.BuildOrderBody shape (top-level TelephoneNumberList)", func(t *testing.T) { + body := buildQuickstartOrderBody("+19195551234", "") + if _, wrong := body.Data["ExistingTelephoneNumberOrderType"]; wrong { + t.Fatalf("must not use the ExistingTelephoneNumberOrderType wrapper: %#v", body.Data) + } + tnl, ok := body.Data["TelephoneNumberList"].(map[string]interface{}) + if !ok { + t.Fatalf("missing top-level TelephoneNumberList: %#v", body.Data) + } + // number.BuildOrderBody uses a []string even for a single TN — mirror it. + got, ok := tnl["TelephoneNumber"].([]string) + if !ok || len(got) != 1 || got[0] != "+19195551234" { + t.Fatalf("TelephoneNumber = %#v, want []string{\"+19195551234\"}", tnl["TelephoneNumber"]) + } + }) + t.Run("vcp path omits SiteId", func(t *testing.T) { + body := buildQuickstartOrderBody("+19195551234", "") + if _, ok := body.Data["SiteId"]; ok { + t.Fatalf("vcp order body must not include SiteId: %#v", body.Data) + } + }) + t.Run("legacy path scopes to the created sub-account", func(t *testing.T) { + body := buildQuickstartOrderBody("+19195551234", "site-9") + if body.Data["SiteId"] != "site-9" { + t.Fatalf("legacy order body SiteId = %v, want site-9", body.Data["SiteId"]) + } + }) +} + func TestFindInMap(t *testing.T) { tests := []struct { name string From 0946541b806bdb2a2cd030b3375c6b78f8dd966d Mon Sep 17 00:00:00 2001 From: Kush Date: Fri, 29 May 2026 16:09:15 -0400 Subject: [PATCH 08/23] feat(quickstart): make all resource provisioning idempotent, incl. number ordering --- cmd/quickstart/quickstart.go | 389 ++++++++++++++++++++++++----------- 1 file changed, 271 insertions(+), 118 deletions(-) diff --git a/cmd/quickstart/quickstart.go b/cmd/quickstart/quickstart.go index c7dfd1a..860e9ed 100644 --- a/cmd/quickstart/quickstart.go +++ b/cmd/quickstart/quickstart.go @@ -3,12 +3,15 @@ package quickstart import ( "encoding/json" "fmt" + "net/url" "os" + "regexp" "github.com/spf13/cobra" "github.com/Bandwidth/cli/internal/api" "github.com/Bandwidth/cli/internal/cmdutil" + "github.com/Bandwidth/cli/internal/output" "github.com/Bandwidth/cli/internal/ui" ) @@ -26,7 +29,12 @@ var Cmd = &cobra.Command{ Long: `Quickstart creates everything you need to make voice calls. By default, it uses the Universal Platform path (VCP). If your account -is on the legacy platform, use --legacy for the sub-account/location path.`, +is on the legacy platform, use --legacy for the sub-account/location path. + +Re-running quickstart is safe on the default (VCP) path: existing resources +are reused and a second number is never ordered. NOTE: re-running --legacy +may order an additional paid number because legacy number ordering is not +idempotent; prefer the default VCP path, which is.`, Example: ` # Universal Platform (default) band quickstart --callback-url https://example.com/voice @@ -77,75 +85,105 @@ func runVCPQuickstart(cmd *cobra.Command) error { result := quickstartResult{CallbackURL: qsCallbackURL, Path: "vcp"} - // Step 1: Create voice application - appSpin := ui.NewSpinner("Creating voice application...") - appSpin.Start() - var appResp interface{} - appBody := api.XMLBody{ - RootElement: "Application", - Data: map[string]interface{}{ - "ServiceType": "Voice-V2", - "AppName": qsName + " App", - "CallInitiatedCallbackUrl": qsCallbackURL, - }, - } - appErr := dashClient.Post(fmt.Sprintf("/accounts/%s/applications", acctID), appBody, &appResp) - appSpin.Stop() - if appErr != nil { - // If voice app creation fails with 409, suggest --legacy - fmt.Fprintf(os.Stderr, "\nVoice application creation failed. If this is a legacy account, try:\n") - fmt.Fprintf(os.Stderr, " band quickstart --callback-url %s --legacy\n\n", qsCallbackURL) - return fmt.Errorf("creating voice application: %w", appErr) - } - appID := extractIDFromResponse(appResp, "ApplicationId", "applicationId") - result.AppID = appID - ui.Successf("Application: %s", ui.ID(appID)) - - // Step 2: Create VCP linked to the app - vcpSpin := ui.NewSpinner("Creating Voice Configuration Package...") - vcpSpin.Start() - var vcpResp interface{} - vcpBody := map[string]interface{}{ - "name": qsName + " VCP", - "httpVoiceV2ApplicationId": appID, - } - vcpErr := platClient.Post(fmt.Sprintf("/v2/accounts/%s/voiceConfigurationPackages", acctID), vcpBody, &vcpResp) - vcpSpin.Stop() - if vcpErr != nil { - fmt.Fprintf(os.Stderr, "\nVCP creation failed. If this is a legacy account, try:\n") - fmt.Fprintf(os.Stderr, " band quickstart --callback-url %s --legacy\n\n", qsCallbackURL) - return fmt.Errorf("creating VCP: %w", vcpErr) - } - vcpID := extractIDFromResponse(vcpResp, "voiceConfigurationPackageId") - result.VCPID = vcpID - ui.Successf("VCP: %s", ui.ID(vcpID)) - - // Step 3: Search and order a number - phoneNumber, err := searchAndOrderNumber(dashClient, acctID, "") + // Step 1: Create voice application (idempotent: reuse if already exists) + appName := qsName + " App" + existingApp, err := findExistingID(dashClient, fmt.Sprintf("/accounts/%s/applications", acctID), "AppName", appName, "ApplicationId", "applicationId") if err != nil { - result.Status = "complete_no_number" - ui.Warnf("%v", err) + return failWithPartial(result, err) + } + result.AppID = existingApp + if result.AppID != "" { + ui.Successf("Application (existing): %s", ui.ID(result.AppID)) } else { - result.PhoneNumber = phoneNumber - ui.Successf("Number: %s", ui.ID(phoneNumber)) + appSpin := ui.NewSpinner("Creating voice application...") + appSpin.Start() + var appResp interface{} + appBody := api.XMLBody{ + RootElement: "Application", + Data: map[string]interface{}{ + "ServiceType": "Voice-V2", + "AppName": appName, + "CallInitiatedCallbackUrl": qsCallbackURL, + }, + } + appErr := dashClient.Post(fmt.Sprintf("/accounts/%s/applications", acctID), appBody, &appResp) + appSpin.Stop() + if appErr != nil { + // If voice app creation fails with 409, suggest --legacy + fmt.Fprintf(os.Stderr, "\nVoice application creation failed. If this is a legacy account, try:\n") + fmt.Fprintf(os.Stderr, " band quickstart --callback-url %s --legacy\n\n", qsCallbackURL) + return failWithPartial(result, fmt.Errorf("creating voice application: %w", appErr)) + } + result.AppID = extractIDFromResponse(appResp, "ApplicationId", "applicationId") + ui.Successf("Application: %s", ui.ID(result.AppID)) + } + appID := result.AppID - // Step 4: Assign number to VCP - assignSpin := ui.NewSpinner("Assigning number to VCP...") - assignSpin.Start() - assignBody := map[string]interface{}{ - "action": "ADD", - "phoneNumbers": []string{phoneNumber}, + // Step 2: Create VCP linked to the app (idempotent: reuse if already exists) + vcpName := qsName + " VCP" + existingVCP, err := findExistingID(platClient, fmt.Sprintf("/v2/accounts/%s/voiceConfigurationPackages", acctID), "name", vcpName, "voiceConfigurationPackageId") + if err != nil { + return failWithPartial(result, err) + } + result.VCPID = existingVCP + if result.VCPID != "" { + ui.Successf("VCP (existing): %s", ui.ID(result.VCPID)) + } else { + vcpSpin := ui.NewSpinner("Creating Voice Configuration Package...") + vcpSpin.Start() + var vcpResp interface{} + vcpBody := map[string]interface{}{ + "name": vcpName, + "httpVoiceV2ApplicationId": appID, } - var assignResp interface{} - assignErr := platClient.Post(fmt.Sprintf("/v2/accounts/%s/voiceConfigurationPackages/%s/phoneNumbers/bulk", acctID, vcpID), assignBody, &assignResp) - assignSpin.Stop() - if assignErr != nil { - ui.Warnf("Failed to assign number to VCP: %v", assignErr) - } else { - ui.Successf("Number assigned to VCP") + vcpErr := platClient.Post(fmt.Sprintf("/v2/accounts/%s/voiceConfigurationPackages", acctID), vcpBody, &vcpResp) + vcpSpin.Stop() + if vcpErr != nil { + fmt.Fprintf(os.Stderr, "\nVCP creation failed. If this is a legacy account, try:\n") + fmt.Fprintf(os.Stderr, " band quickstart --callback-url %s --legacy\n\n", qsCallbackURL) + return failWithPartial(result, fmt.Errorf("creating VCP: %w", vcpErr)) } + result.VCPID = extractIDFromResponse(vcpResp, "voiceConfigurationPackageId") + ui.Successf("VCP: %s", ui.ID(result.VCPID)) + } + vcpID := result.VCPID + // Step 3: Search and order a number (idempotent: skip if VCP already has one) + existingNum, err := firstAssignedNumber(platClient, acctID, vcpID) + if err != nil { + return failWithPartial(result, err) + } + if existingNum != "" { + result.PhoneNumber = existingNum result.Status = "complete" + ui.Successf("Number (existing): %s", ui.ID(existingNum)) + } else { + phoneNumber, err := searchAndOrderNumber(dashClient, acctID, "") + if err != nil { + result.Status = "complete_no_number" + ui.Warnf("%v", err) + } else { + result.PhoneNumber = phoneNumber + ui.Successf("Number: %s", ui.ID(phoneNumber)) + + // Step 4: Assign number to VCP + assignSpin := ui.NewSpinner("Assigning number to VCP...") + assignSpin.Start() + assignBody := map[string]interface{}{ + "action": "ADD", + "phoneNumbers": []string{phoneNumber}, + } + var assignResp interface{} + assignErr := platClient.Post(fmt.Sprintf("/v2/accounts/%s/voiceConfigurationPackages/%s/phoneNumbers/bulk", acctID, vcpID), assignBody, &assignResp) + assignSpin.Stop() + if assignErr != nil { + ui.Warnf("Failed to assign number to VCP: %v", assignErr) + } else { + ui.Successf("Number assigned to VCP") + } + + result.Status = "complete" + } } fmt.Fprintln(os.Stderr, "") @@ -167,65 +205,103 @@ func runLegacyQuickstart(cmd *cobra.Command) error { result := quickstartResult{CallbackURL: qsCallbackURL, Path: "legacy"} - // Step 1: Create sub-account - siteSpin := ui.NewSpinner("Creating sub-account...") - siteSpin.Start() - var siteResp interface{} - siteBody := api.XMLBody{ - RootElement: "Site", - Data: map[string]interface{}{"Name": qsName + " Sub-account"}, - } - siteErr := client.Post(fmt.Sprintf("/accounts/%s/sites", acctID), siteBody, &siteResp) - siteSpin.Stop() - if siteErr != nil { - return fmt.Errorf("creating sub-account: %w", siteErr) - } - siteID := extractIDFromResponse(siteResp, "Id", "id", "siteId") - result.SiteID = siteID - ui.Successf("Sub-account: %s", ui.ID(siteID)) - - // Step 2: Create SIP peer - sipSpin := ui.NewSpinner("Creating location...") - sipSpin.Start() - var sipResp interface{} - sipBody := api.XMLBody{ - RootElement: "SipPeer", - Data: map[string]interface{}{ - "PeerName": qsName + " Location", - "IsDefaultPeer": "true", - }, + // Step 1: Create sub-account (idempotent: reuse if already exists) + siteName := qsName + " Sub-account" + existingSite, err := findExistingID(client, fmt.Sprintf("/accounts/%s/sites", acctID), "Name", siteName, "Id", "id", "siteId") + if err != nil { + return failWithPartial(result, err) } - sipErr := client.Post(fmt.Sprintf("/accounts/%s/sites/%s/sippeers", acctID, siteID), sipBody, &sipResp) - sipSpin.Stop() - if sipErr != nil { - return fmt.Errorf("creating location: %w", sipErr) - } - sipPeerID := extractIDFromResponse(sipResp, "PeerId", "Id", "id") - result.SIPPeerID = sipPeerID - ui.Successf("Location: %s", ui.ID(sipPeerID)) - - // Step 3: Create voice application - appSpin := ui.NewSpinner("Creating voice application...") - appSpin.Start() - var appResp interface{} - appBody := api.XMLBody{ - RootElement: "Application", - Data: map[string]interface{}{ - "ServiceType": "Voice-V2", - "AppName": qsName + " App", - "CallInitiatedCallbackUrl": qsCallbackURL, - }, + var siteID string + if existingSite != "" { + result.SiteID = existingSite + siteID = existingSite + ui.Successf("Sub-account (existing): %s", ui.ID(existingSite)) + } else { + siteSpin := ui.NewSpinner("Creating sub-account...") + siteSpin.Start() + var siteResp interface{} + siteBody := api.XMLBody{ + RootElement: "Site", + Data: map[string]interface{}{"Name": siteName}, + } + siteErr := client.Post(fmt.Sprintf("/accounts/%s/sites", acctID), siteBody, &siteResp) + siteSpin.Stop() + if siteErr != nil { + return failWithPartial(result, fmt.Errorf("creating sub-account: %w", siteErr)) + } + siteID = extractIDFromResponse(siteResp, "Id", "id", "siteId") + result.SiteID = siteID + ui.Successf("Sub-account: %s", ui.ID(siteID)) } - appErr := client.Post(fmt.Sprintf("/accounts/%s/applications", acctID), appBody, &appResp) - appSpin.Stop() - if appErr != nil { - return fmt.Errorf("creating application: %w", appErr) + + // Step 2: Create SIP peer / location (idempotent: reuse if already exists) + peerName := qsName + " Location" + existingPeer, err := findExistingID(client, fmt.Sprintf("/accounts/%s/sites/%s/sippeers", acctID, siteID), "PeerName", peerName, "PeerId", "Id", "id") + if err != nil { + return failWithPartial(result, err) + } + if existingPeer != "" { + result.SIPPeerID = existingPeer + ui.Successf("Location (existing): %s", ui.ID(existingPeer)) + } else { + sipSpin := ui.NewSpinner("Creating location...") + sipSpin.Start() + var sipResp interface{} + sipBody := api.XMLBody{ + RootElement: "SipPeer", + Data: map[string]interface{}{ + "PeerName": peerName, + "IsDefaultPeer": "true", + }, + } + sipErr := client.Post(fmt.Sprintf("/accounts/%s/sites/%s/sippeers", acctID, siteID), sipBody, &sipResp) + sipSpin.Stop() + if sipErr != nil { + return failWithPartial(result, fmt.Errorf("creating location: %w", sipErr)) + } + result.SIPPeerID = extractIDFromResponse(sipResp, "PeerId", "Id", "id") + ui.Successf("Location: %s", ui.ID(result.SIPPeerID)) } - appID := extractIDFromResponse(appResp, "ApplicationId", "applicationId") - result.AppID = appID - ui.Successf("Application: %s", ui.ID(appID)) - // Step 4: Search and order a number + // Step 3: Create voice application (idempotent: reuse if already exists) + appName := qsName + " App" + existingApp, err := findExistingID(client, fmt.Sprintf("/accounts/%s/applications", acctID), "AppName", appName, "ApplicationId", "applicationId") + if err != nil { + return failWithPartial(result, err) + } + result.AppID = existingApp + if result.AppID != "" { + ui.Successf("Application (existing): %s", ui.ID(result.AppID)) + } else { + appSpin := ui.NewSpinner("Creating voice application...") + appSpin.Start() + var appResp interface{} + appBody := api.XMLBody{ + RootElement: "Application", + Data: map[string]interface{}{ + "ServiceType": "Voice-V2", + "AppName": appName, + "CallInitiatedCallbackUrl": qsCallbackURL, + }, + } + appErr := client.Post(fmt.Sprintf("/accounts/%s/applications", acctID), appBody, &appResp) + appSpin.Stop() + if appErr != nil { + return failWithPartial(result, fmt.Errorf("creating application: %w", appErr)) + } + result.AppID = extractIDFromResponse(appResp, "ApplicationId", "applicationId") + ui.Successf("Application: %s", ui.ID(result.AppID)) + } + appID := result.AppID + + // Step 4: Search and order a number. + // TODO: Legacy number ordering cannot be made idempotent here because there is no + // sub-account-scoped in-service TN listing endpoint. The account-wide /tns endpoint + // (used by number.fetchAccountNumbers) would wrongly skip ordering on accounts that + // already have unrelated numbers assigned to different sub-accounts. Close this TODO + // if Bandwidth exposes a sub-account-scoped in-service TN endpoint, or if + // number.fetchAccountNumbers is exported and a heuristic is deemed acceptable. + ui.Warnf("Note: the legacy number-ordering step is not idempotent — each time you re-run quickstart --legacy, another number may be ordered. The default (VCP) path does not have this limitation.") phoneNumber, err := searchAndOrderNumber(client, acctID, siteID) if err != nil { result.Status = "complete_no_number" @@ -247,6 +323,83 @@ func runLegacyQuickstart(cmd *cobra.Command) error { return printResult(result) } +// failWithPartial prints the partial result (so created resource IDs aren't +// lost) and returns the wrapped error. Re-running quickstart reuses those +// resources via the idempotency checks. +func failWithPartial(result quickstartResult, err error) error { + result.Status = "partial" + _ = printResult(result) + return err +} + +// findExistingID lists resources at listPath and returns the id of the first +// whose nameField matches name (or "" if none). It FAILS CLOSED: a list error +// is returned to the caller rather than swallowed, because quickstart spends +// money — a transient list failure must NOT cause us to create a duplicate. +func findExistingID(client *api.Client, listPath, nameField, name string, idKeys ...string) (string, error) { + var resp interface{} + if err := client.Get(listPath, &resp); err != nil { + return "", fmt.Errorf("checking for existing resource at %s: %w", listPath, err) + } + match := output.FindByName(resp, nameField, name) + if match == nil { + return "", nil + } + return extractIDFromResponse(match, idKeys...), nil +} + +var e164Re = regexp.MustCompile(`^\+?\d{10,15}$`) + +// phoneKeyRe matches JSON keys that may hold a phone number (number, phoneNumber). +// It can also match keys like orderNumber, but those won't contain E.164 values +// in the VCP phoneNumbers/voice response, so firstE164 still picks the right one. +var phoneKeyRe = regexp.MustCompile(`(?i)(phone)?number`) + +// firstE164 walks a decoded response and returns the first value that looks +// like an E.164 phone number, preferring phone-number-looking keys before a +// field-agnostic fallback. Account IDs are <10 digits and won't match e164Re. +func firstE164(v interface{}) string { + switch x := v.(type) { + case map[string]interface{}: + for k, child := range x { // prefer phone-number-looking keys + if phoneKeyRe.MatchString(k) { + if s, ok := child.(string); ok && e164Re.MatchString(s) { + return s + } + } + } + for _, child := range x { // field-agnostic fallback + if s := firstE164(child); s != "" { + return s + } + } + case []interface{}: + for _, item := range x { + if s := firstE164(item); s != "" { + return s + } + } + case string: + if e164Re.MatchString(x) { + return x + } + } + return "" +} + +// firstAssignedNumber returns the first phone number already assigned to vcpID. +// It FAILS CLOSED: a list error is returned, not swallowed, so the caller does +// NOT order a duplicate paid number on a transient failure. Endpoint confirmed +// from cmd/vcp/numbers.go. +func firstAssignedNumber(client *api.Client, acctID, vcpID string) (string, error) { + var resp interface{} + path := fmt.Sprintf("/v2/accounts/%s/phoneNumbers/voice?voiceConfigurationPackageId=%s", acctID, url.QueryEscape(vcpID)) + if err := client.Get(path, &resp); err != nil { + return "", fmt.Errorf("checking existing VCP numbers for %s: %w", vcpID, err) + } + return firstE164(resp), nil +} + // buildQuickstartOrderBody mirrors number.BuildOrderBody — a top-level // TelephoneNumberList with no order-type wrapper, exactly like the proven // `number order` command. siteID scopes the order to a sub-account on the From 6657e7051a7e2c762c821201f1e233b1fd92f8e0 Mon Sep 17 00:00:00 2001 From: Kush Date: Fri, 29 May 2026 16:15:54 -0400 Subject: [PATCH 09/23] fix(quickstart): emit partial state on failed VCP assignment instead of false success --- AGENTS.md | 5 +++++ cmd/quickstart/quickstart.go | 7 +++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index c35b84d..a7bfb85 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -205,6 +205,11 @@ For full flag/argument reference, use `band --help`. This section cove - **Agents should not use `band quickstart`.** It creates real resources that cost money (orders a phone number), doesn't support `--if-not-exists` (running it twice creates duplicate resources and orders a second number), doesn't return structured output for each step, and can't be partially retried if it fails midway. Use the step-by-step provisioning workflows in the [Agent Workflows](#agent-workflows) section instead. +- **`band quickstart` output `status` values** (VCP path only — `--legacy` is not idempotent): + - `complete` — all resources created and number assigned; ready to use. + - `complete_no_number` — resources created but no number was available in the requested area code; re-run with `--area-code` to try a different code. + - `partial` — quickstart stopped after a failure but printed the resource IDs it created so far (app, VCP, and possibly an ordered phone number). Re-running quickstart reuses those resources via idempotency checks. If the assignment specifically failed, the ordered number is included in the partial output under `phoneNumber` so it isn't lost. + --- ## Timeout Recovery diff --git a/cmd/quickstart/quickstart.go b/cmd/quickstart/quickstart.go index 860e9ed..4ed0cff 100644 --- a/cmd/quickstart/quickstart.go +++ b/cmd/quickstart/quickstart.go @@ -177,11 +177,10 @@ func runVCPQuickstart(cmd *cobra.Command) error { assignErr := platClient.Post(fmt.Sprintf("/v2/accounts/%s/voiceConfigurationPackages/%s/phoneNumbers/bulk", acctID, vcpID), assignBody, &assignResp) assignSpin.Stop() if assignErr != nil { - ui.Warnf("Failed to assign number to VCP: %v", assignErr) - } else { - ui.Successf("Number assigned to VCP") + result.PhoneNumber = phoneNumber + return failWithPartial(result, fmt.Errorf("assigning number %s to VCP %s: %w", phoneNumber, vcpID, assignErr)) } - + ui.Successf("Number assigned to VCP") result.Status = "complete" } } From 7b345b94f6bdc85ead76d9402d90ebc7fd166705 Mon Sep 17 00:00:00 2001 From: Kush Date: Fri, 29 May 2026 16:23:21 -0400 Subject: [PATCH 10/23] docs(quickstart): reconcile agent guidance with new VCP-path idempotency --- AGENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index a7bfb85..bfe2434 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -203,7 +203,7 @@ For full flag/argument reference, use `band --help`. This section cove ### Quickstart -- **Agents should not use `band quickstart`.** It creates real resources that cost money (orders a phone number), doesn't support `--if-not-exists` (running it twice creates duplicate resources and orders a second number), doesn't return structured output for each step, and can't be partially retried if it fails midway. Use the step-by-step provisioning workflows in the [Agent Workflows](#agent-workflows) section instead. +- **Agents should prefer the step-by-step provisioning workflows over `band quickstart`.** Quickstart creates real resources that cost money (it orders a phone number). The default (VCP) path is idempotent — re-running reuses existing resources via find-or-create and will not order a second number — and on failure it prints the resource IDs created so far (`status: partial`, see below) so a re-run can resume. The `--legacy` path is NOT idempotent (re-running it may order an additional number). Because quickstart bundles several steps behind one command, prefer the step-by-step provisioning workflows in the [Agent Workflows](#agent-workflows) section when you need per-step structured output or fine-grained control. - **`band quickstart` output `status` values** (VCP path only — `--legacy` is not idempotent): - `complete` — all resources created and number assigned; ready to use. From f27302810b2f8a23b414329943038bd3e918aa10 Mon Sep 17 00:00:00 2001 From: Kush Date: Fri, 29 May 2026 16:36:09 -0400 Subject: [PATCH 11/23] fix(messaging): treat Messaging API as production-only (no test host) Research against all six Bandwidth SDKs and internal Confluence confirmed the Messaging API has no public test/sandbox host: the prod endpoint is the entry point for UAT too, and messaging is tested with test accounts/numbers, not a separate host. The earlier change invented test.messaging.bandwidth.com by analogy to test.api/test.voice, which would make --environment test point at a non-existent host. Replace messagingHostForEnvironment(env) with messagingHost() that always returns prod unless BW_MESSAGING_URL overrides it. --- AGENTS.md | 2 +- README.md | 2 +- internal/cmdutil/helpers.go | 24 ++++++++++-------- internal/cmdutil/helpers_test.go | 43 +++++++++++--------------------- 4 files changed, 29 insertions(+), 42 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index bfe2434..2f17af7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -79,7 +79,7 @@ This is stderr only — it won't break piped output parsing. | `BW_ENVIRONMENT` | API environment: `prod` (default), `test` | | `BW_API_URL` | Override API base URL (overrides environment-based default) | | `BW_VOICE_URL` | Override Voice API base URL (overrides environment-based default) | -| `BW_MESSAGING_URL` | Override Messaging API base URL (overrides environment-based default) | +| `BW_MESSAGING_URL` | Override Messaging API base URL. Messaging is production-only — `--environment test` does NOT change the host (no test messaging endpoint exists); only this override does. | | `BW_FORMAT` | Output format override | **Config file location:** `~/.config/band/config.json` (XDG). Falls back to `~/.band/config.json` if the XDG path doesn't exist. diff --git a/README.md b/README.md index d8682c7..08f3a4c 100644 --- a/README.md +++ b/README.md @@ -498,7 +498,7 @@ Sub-accounts (formerly known as sites) are the top-level container. Locations (f | `BW_FORMAT` | Default output format | | `BW_API_URL` | Override the API base URL | | `BW_VOICE_URL` | Override the Voice API base URL | -| `BW_MESSAGING_URL` | Override the Messaging API base URL (for testing or local proxies) | +| `BW_MESSAGING_URL` | Override the Messaging API base URL. Messaging is production-only (no test host), so `--environment`/`BW_ENVIRONMENT` does not change it; use this for local proxies or the internal lab. | --- diff --git a/internal/cmdutil/helpers.go b/internal/cmdutil/helpers.go index 2e0cc63..3da2a28 100644 --- a/internal/cmdutil/helpers.go +++ b/internal/cmdutil/helpers.go @@ -41,18 +41,18 @@ func voiceHostForEnvironment(env string) string { } } -// messagingHostForEnvironment maps an environment name to its Messaging API host. -// Non-production environments can be overridden with BW_MESSAGING_URL. -func messagingHostForEnvironment(env string) string { +// messagingHost returns the Messaging API base host. The Bandwidth Messaging +// API is PRODUCTION-ONLY — there is no public test/sandbox host, so unlike the +// api/voice clients it does NOT vary by --environment. (Confirmed against all +// six Bandwidth SDKs, which define only the prod server, and internal docs: UAT +// shares the prod entry point, and messaging is tested with test +// accounts/numbers rather than a separate host.) BW_MESSAGING_URL overrides the +// base URL for local proxies or the internal lab environment. +func messagingHost() string { if v := os.Getenv("BW_MESSAGING_URL"); v != "" { return strings.TrimRight(v, "/") } - switch env { - case "test", "uat": - return "https://test.messaging.bandwidth.com" - default: - return "https://messaging.bandwidth.com" - } + return "https://messaging.bandwidth.com" } // loadConfigAndAuth loads the config, retrieves the client secret, and returns @@ -194,10 +194,12 @@ func PlatformClient(accountIDOverride string) (*api.Client, string, error) { } // MessagingClient returns a client for the Bandwidth Messaging API v2. +// Messaging is production-only (see messagingHost), so the environment from +// authenticate is intentionally ignored for host selection. func MessagingClient(accountIDOverride string) (*api.Client, string, error) { - tm, acctID, env, err := authenticate(accountIDOverride) + tm, acctID, _, err := authenticate(accountIDOverride) if err != nil { return nil, "", err } - return api.NewClient(messagingHostForEnvironment(env)+"/api/v2", tm), acctID, nil + return api.NewClient(messagingHost()+"/api/v2", tm), acctID, nil } diff --git a/internal/cmdutil/helpers_test.go b/internal/cmdutil/helpers_test.go index 8c0e6fb..a0816d0 100644 --- a/internal/cmdutil/helpers_test.go +++ b/internal/cmdutil/helpers_test.go @@ -41,42 +41,27 @@ func TestVoiceHostForEnvironment_BW_VOICE_URL_TrailingSlash(t *testing.T) { } } -func TestMessagingHostForEnvironment(t *testing.T) { - tests := []struct { - name string - env string - want string - }{ - {"prod default", "", "https://messaging.bandwidth.com"}, - {"prod explicit", "prod", "https://messaging.bandwidth.com"}, - {"unknown env falls back to prod", "staging", "https://messaging.bandwidth.com"}, - {"test", "test", "https://test.messaging.bandwidth.com"}, - {"uat", "uat", "https://test.messaging.bandwidth.com"}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := messagingHostForEnvironment(tt.env); got != tt.want { - t.Errorf("messagingHostForEnvironment(%q) = %q, want %q", tt.env, got, tt.want) - } - }) - } +func TestMessagingHost(t *testing.T) { + // Messaging is production-only — there is no test/sandbox host, so the host + // never varies by --environment. Only BW_MESSAGING_URL can override it. + t.Run("prod default", func(t *testing.T) { + if got := messagingHost(); got != "https://messaging.bandwidth.com" { + t.Errorf("messagingHost() = %q, want https://messaging.bandwidth.com", got) + } + }) } -func TestMessagingHostForEnvironment_BW_MESSAGING_URL(t *testing.T) { +func TestMessagingHost_BW_MESSAGING_URL(t *testing.T) { t.Setenv("BW_MESSAGING_URL", "https://custom.messaging.example.com") - for _, env := range []string{"", "prod", "test"} { - got := messagingHostForEnvironment(env) - if got != "https://custom.messaging.example.com" { - t.Errorf("messagingHostForEnvironment(%q) with BW_MESSAGING_URL = %q, want override", env, got) - } + if got := messagingHost(); got != "https://custom.messaging.example.com" { + t.Errorf("messagingHost() with BW_MESSAGING_URL = %q, want override", got) } } -func TestMessagingHostForEnvironment_BW_MESSAGING_URL_TrailingSlash(t *testing.T) { +func TestMessagingHost_BW_MESSAGING_URL_TrailingSlash(t *testing.T) { t.Setenv("BW_MESSAGING_URL", "http://localhost:8080/") - got := messagingHostForEnvironment("") - if got != "http://localhost:8080" { - t.Errorf("messagingHostForEnvironment with trailing slash = %q, want without slash", got) + if got := messagingHost(); got != "http://localhost:8080" { + t.Errorf("messagingHost() with trailing slash = %q, want without slash", got) } } From ddb26322be32fa4eef2d21bde623d7da401aadd2 Mon Sep 17 00:00:00 2001 From: Kush Date: Fri, 29 May 2026 16:59:49 -0400 Subject: [PATCH 12/23] feat(env): wire --environment flag into host selection; warn that messaging is production-only --- cmd/root.go | 6 +++++ internal/cmdutil/helpers.go | 42 ++++++++++++++++++++++++++++---- internal/cmdutil/helpers_test.go | 29 ++++++++++++++++++++++ 3 files changed, 72 insertions(+), 5 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index 57f0b03..0d68962 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -9,6 +9,7 @@ import ( "golang.org/x/term" "github.com/Bandwidth/cli/internal/api" + "github.com/Bandwidth/cli/internal/cmdutil" "github.com/Bandwidth/cli/internal/config" "github.com/Bandwidth/cli/internal/ui" versionpkg "github.com/Bandwidth/cli/internal/version" @@ -50,6 +51,11 @@ var rootCmd = &cobra.Command{ Short: "Bandwidth CLI — manage voice, messaging, numbers, and more from the command line", Long: "The official Bandwidth CLI. Build and debug voice applications, send messages, manage phone numbers, and control calls.", PersistentPreRun: func(cmd *cobra.Command, args []string) { + // Wire the --environment flag into host/environment resolution. + if environment != "" { + cmdutil.EnvironmentOverride = environment + } + // Kick off version check in background so it doesn't slow down the command. updateResult = make(chan *versionpkg.CheckResult, 1) go func() { diff --git a/internal/cmdutil/helpers.go b/internal/cmdutil/helpers.go index 3da2a28..037a36c 100644 --- a/internal/cmdutil/helpers.go +++ b/internal/cmdutil/helpers.go @@ -11,8 +11,25 @@ import ( "github.com/Bandwidth/cli/internal/api" "github.com/Bandwidth/cli/internal/auth" "github.com/Bandwidth/cli/internal/config" + "github.com/Bandwidth/cli/internal/ui" ) +// EnvironmentOverride, when non-empty, overrides the profile/BW_ENVIRONMENT +// environment used for host selection. It is set from the --environment +// persistent flag by the root command's PersistentPreRun. Flag beats +// BW_ENVIRONMENT beats profile config (matching how --account-id overrides). +var EnvironmentOverride string + +// resolveEnvironment applies EnvironmentOverride (the --environment flag) on top +// of the profile-derived environment (which already includes any BW_ENVIRONMENT +// overlay). +func resolveEnvironment(profileEnv string) string { + if EnvironmentOverride != "" { + return EnvironmentOverride + } + return profileEnv +} + // apiHostForEnvironment maps an environment name to its API host. // Non-production environments can be overridden with BW_API_URL. func apiHostForEnvironment(env string) string { @@ -128,9 +145,10 @@ func authenticate(accountIDOverride string) (*auth.TokenManager, string, string, return nil, "", "", err } - apiHost := apiHostForEnvironment(p.Environment) + env := resolveEnvironment(p.Environment) + apiHost := apiHostForEnvironment(env) tm := auth.NewTokenManager(p.ClientID, clientSecret, apiHost) - return tm, acctID, p.Environment, nil + return tm, acctID, env, nil } // BuildClient returns an authenticated JSON API client. @@ -193,13 +211,27 @@ func PlatformClient(accountIDOverride string) (*api.Client, string, error) { return api.NewClient(apiHostForEnvironment(env), tm), acctID, nil } +// messagingProdOnlyWarning returns a user warning when env is a non-production +// environment, because the Bandwidth Messaging API is production-only (there is +// no test host). Returns "" when no warning is needed. +func messagingProdOnlyWarning(env string) string { + if env == "test" || env == "uat" { + return "Bandwidth Messaging has no test environment — this request uses PRODUCTION regardless of --environment. Sends are real and billable." + } + return "" +} + // MessagingClient returns a client for the Bandwidth Messaging API v2. -// Messaging is production-only (see messagingHost), so the environment from -// authenticate is intentionally ignored for host selection. +// Messaging is production-only (see messagingHost) — the host never varies by +// --environment. When env is test or uat, a warning is printed to stderr because +// sends are real and billable. func MessagingClient(accountIDOverride string) (*api.Client, string, error) { - tm, acctID, _, err := authenticate(accountIDOverride) + tm, acctID, env, err := authenticate(accountIDOverride) if err != nil { return nil, "", err } + if w := messagingProdOnlyWarning(env); w != "" { + ui.Warnf("%s", w) + } return api.NewClient(messagingHost()+"/api/v2", tm), acctID, nil } diff --git a/internal/cmdutil/helpers_test.go b/internal/cmdutil/helpers_test.go index a0816d0..11c3c40 100644 --- a/internal/cmdutil/helpers_test.go +++ b/internal/cmdutil/helpers_test.go @@ -65,6 +65,35 @@ func TestMessagingHost_BW_MESSAGING_URL_TrailingSlash(t *testing.T) { } } +func TestResolveEnvironment(t *testing.T) { + t.Run("no override returns profile env", func(t *testing.T) { + EnvironmentOverride = "" + if got := resolveEnvironment("prod"); got != "prod" { + t.Errorf("got %q, want prod", got) + } + }) + t.Run("override wins over profile env", func(t *testing.T) { + EnvironmentOverride = "test" + t.Cleanup(func() { EnvironmentOverride = "" }) + if got := resolveEnvironment("prod"); got != "test" { + t.Errorf("got %q, want test", got) + } + }) +} + +func TestMessagingProdOnlyWarning(t *testing.T) { + for _, env := range []string{"test", "uat"} { + if messagingProdOnlyWarning(env) == "" { + t.Errorf("expected a warning for env %q", env) + } + } + for _, env := range []string{"", "prod", "staging"} { + if messagingProdOnlyWarning(env) != "" { + t.Errorf("expected NO warning for env %q, got one", env) + } + } +} + func TestAPIHostForEnvironment(t *testing.T) { tests := []struct { name string From b1df55036db27ffe931d4d9e335f0dbe8fd3e87a Mon Sep 17 00:00:00 2001 From: Kush Date: Fri, 29 May 2026 17:01:45 -0400 Subject: [PATCH 13/23] fix(env): show resolved --environment in the account hint --- cmd/root.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cmd/root.go b/cmd/root.go index 0d68962..ab806e1 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -204,7 +204,11 @@ func showAccountHint(cmd *cobra.Command) { // Show environment when the user operates across multiple environments // or is on a non-default one. Customers with only prod don't need the noise. + // Honor the --environment flag override so the hint matches actual routing. env := p.Environment + if cmdutil.EnvironmentOverride != "" { + env = cmdutil.EnvironmentOverride + } if env == "" { env = "prod" } From 11138e376df7bf6b9136f3859d5850a0bcd434b6 Mon Sep 17 00:00:00 2001 From: Kush Date: Fri, 29 May 2026 18:40:21 -0400 Subject: [PATCH 14/23] fix(number): order requires SiteId + ExistingTelephoneNumberOrderType wrapper The orders API rejects a bare top-level TelephoneNumberList with HTTP 500 and requires a SiteId (code 5022). Add the required --subaccount flag and emit the correct wrapped body. Verified live: order now returns OrderStatus RECEIVED. --- cmd/number/number_test.go | 18 ++++++++++++++---- cmd/number/order.go | 31 ++++++++++++++++++++----------- 2 files changed, 34 insertions(+), 15 deletions(-) diff --git a/cmd/number/number_test.go b/cmd/number/number_test.go index dc21ce5..88b1829 100644 --- a/cmd/number/number_test.go +++ b/cmd/number/number_test.go @@ -9,9 +9,18 @@ import ( ) func TestBuildOrderBody(t *testing.T) { - body := BuildOrderBody([]string{"+19195551234", "+19195551235"}) + body := BuildOrderBody("152681", []string{"+19195551234", "+19195551235"}) - tnList, ok := body["TelephoneNumberList"].(map[string]interface{}) + // The orders API requires a SiteId and the ExistingTelephoneNumberOrderType + // wrapper (a bare top-level TelephoneNumberList returns HTTP 500). + if body["SiteId"] != "152681" { + t.Errorf("SiteId = %v, want 152681", body["SiteId"]) + } + wrapper, ok := body["ExistingTelephoneNumberOrderType"].(map[string]interface{}) + if !ok { + t.Fatal("missing ExistingTelephoneNumberOrderType wrapper") + } + tnList, ok := wrapper["TelephoneNumberList"].(map[string]interface{}) if !ok { t.Fatal("TelephoneNumberList is not a map") } @@ -31,8 +40,9 @@ func TestBuildOrderBody(t *testing.T) { } func TestBuildOrderBody_SingleNumber(t *testing.T) { - body := BuildOrderBody([]string{"+19195551234"}) - tnList := body["TelephoneNumberList"].(map[string]interface{}) + body := BuildOrderBody("152681", []string{"+19195551234"}) + wrapper := body["ExistingTelephoneNumberOrderType"].(map[string]interface{}) + tnList := wrapper["TelephoneNumberList"].(map[string]interface{}) numbers := tnList["TelephoneNumber"].([]string) if len(numbers) != 1 { t.Errorf("expected 1 number, got %d", len(numbers)) diff --git a/cmd/number/order.go b/cmd/number/order.go index 1406067..29e8682 100644 --- a/cmd/number/order.go +++ b/cmd/number/order.go @@ -12,32 +12,41 @@ import ( ) var ( - orderWait bool - orderTimeout time.Duration + orderWait bool + orderTimeout time.Duration + orderSubaccount string ) func init() { + orderCmd.Flags().StringVar(&orderSubaccount, "subaccount", "", "Sub-account ID to order the number(s) into (required; see `band subaccount list`)") orderCmd.Flags().BoolVar(&orderWait, "wait", false, "Wait until the ordered number(s) appear in service") orderCmd.Flags().DurationVar(&orderTimeout, "timeout", 30*time.Second, "Maximum time to wait (default 30s)") + _ = orderCmd.MarkFlagRequired("subaccount") Cmd.AddCommand(orderCmd) } var orderCmd = &cobra.Command{ Use: "order [number...]", Short: "Order one or more phone numbers", - Long: "Orders one or more phone numbers from a search result. Use --wait to block until the numbers are active.", - Example: ` band number order +19195551234 - band number order +19195551234 +19195551235 - band number order +19195551234 --wait --timeout 30s`, + Long: "Orders one or more phone numbers (from a search result) into a sub-account. The Bandwidth orders API requires a sub-account, so --subaccount is required. Use --wait to block until the numbers are active.", + Example: ` band number order +19195551234 --subaccount 152681 + band number order +19195551234 +19195551235 --subaccount 152681 + band number order +19195551234 --subaccount 152681 --wait --timeout 30s`, Args: cobra.MinimumNArgs(1), RunE: runOrder, } -// BuildOrderBody builds the XML request body for ordering phone numbers. -func BuildOrderBody(numbers []string) map[string]interface{} { +// BuildOrderBody builds the XML request body for ordering existing (available) +// phone numbers into a sub-account. The Bandwidth orders API requires the +// ExistingTelephoneNumberOrderType wrapper and a SiteId (sub-account); omitting +// either fails (bare TelephoneNumberList → HTTP 500; missing SiteId → 5022). +func BuildOrderBody(siteID string, numbers []string) map[string]interface{} { return map[string]interface{}{ - "TelephoneNumberList": map[string]interface{}{ - "TelephoneNumber": numbers, + "SiteId": siteID, + "ExistingTelephoneNumberOrderType": map[string]interface{}{ + "TelephoneNumberList": map[string]interface{}{ + "TelephoneNumber": numbers, + }, }, } } @@ -48,7 +57,7 @@ func runOrder(cmd *cobra.Command, args []string) error { return err } - bodyData := BuildOrderBody(args) + bodyData := BuildOrderBody(orderSubaccount, args) var result interface{} if err := client.Post(fmt.Sprintf("/accounts/%s/orders", acctID), api.XMLBody{RootElement: "Order", Data: bodyData}, &result); err != nil { From bc2af18b2c72bba60a5b0e754c8ddd17afb9ca4f Mon Sep 17 00:00:00 2001 From: Kush Date: Fri, 29 May 2026 18:40:21 -0400 Subject: [PATCH 15/23] fix(number): release requires DisconnectTelephoneNumberOrderType wrapper Without the wrapper the disconnects API returns 5001 (invalid TN list). Verified live: release now returns OrderStatus RECEIVED. --- cmd/number/release.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/cmd/number/release.go b/cmd/number/release.go index 76272ee..d67c127 100644 --- a/cmd/number/release.go +++ b/cmd/number/release.go @@ -27,9 +27,14 @@ func runRelease(cmd *cobra.Command, args []string) error { return err } + // The disconnects API requires the DisconnectTelephoneNumberOrderType + // wrapper around the TelephoneNumberList; omitting it fails with 5001 + // ("Invalid input telephone number list"). bodyData := map[string]interface{}{ - "TelephoneNumberList": map[string]interface{}{ - "TelephoneNumber": []string{args[0]}, + "DisconnectTelephoneNumberOrderType": map[string]interface{}{ + "TelephoneNumberList": map[string]interface{}{ + "TelephoneNumber": []string{args[0]}, + }, }, } From 150c9c73fe6a801726c4ca75f988b8f3b4260ae7 Mon Sep 17 00:00:00 2001 From: Kush Date: Fri, 29 May 2026 18:48:26 -0400 Subject: [PATCH 16/23] fix(quickstart): correct order body, ensure sub-account+default location, retry VCP assign Reuse number.BuildOrderBody (SiteId + ExistingTelephoneNumberOrderType wrapper). Orders require a sub-account with a default SIP peer (codes 5022/5020), so the VCP path now ensures both. The just-ordered number provisions asynchronously, so the VCP assignment is retried until it succeeds (VCS-0044 until ready). Verified live: VCP quickstart now reaches status=complete. --- cmd/quickstart/quickstart.go | 93 +++++++++++++++++++++++++------ cmd/quickstart/quickstart_test.go | 32 +---------- 2 files changed, 78 insertions(+), 47 deletions(-) diff --git a/cmd/quickstart/quickstart.go b/cmd/quickstart/quickstart.go index 4ed0cff..e73767f 100644 --- a/cmd/quickstart/quickstart.go +++ b/cmd/quickstart/quickstart.go @@ -6,9 +6,11 @@ import ( "net/url" "os" "regexp" + "time" "github.com/spf13/cobra" + numbercmd "github.com/Bandwidth/cli/cmd/number" "github.com/Bandwidth/cli/internal/api" "github.com/Bandwidth/cli/internal/cmdutil" "github.com/Bandwidth/cli/internal/output" @@ -158,7 +160,13 @@ func runVCPQuickstart(cmd *cobra.Command) error { result.Status = "complete" ui.Successf("Number (existing): %s", ui.ID(existingNum)) } else { - phoneNumber, err := searchAndOrderNumber(dashClient, acctID, "") + // Orders require a sub-account (SiteId), so ensure one exists before ordering. + siteID, err := ensureSubaccount(dashClient, acctID, qsName+" Sub-account") + if err != nil { + return failWithPartial(result, err) + } + result.SiteID = siteID + phoneNumber, err := searchAndOrderNumber(dashClient, acctID, siteID) if err != nil { result.Status = "complete_no_number" ui.Warnf("%v", err) @@ -166,19 +174,33 @@ func runVCPQuickstart(cmd *cobra.Command) error { result.PhoneNumber = phoneNumber ui.Successf("Number: %s", ui.ID(phoneNumber)) - // Step 4: Assign number to VCP + // Step 4: Assign number to VCP. The just-ordered number takes a + // moment to become provisionable for voice (the order is async), so + // the bulk assign returns VCS-0044 until provisioning catches up. + // Retry until it succeeds or we time out. assignSpin := ui.NewSpinner("Assigning number to VCP...") assignSpin.Start() assignBody := map[string]interface{}{ "action": "ADD", "phoneNumbers": []string{phoneNumber}, } - var assignResp interface{} - assignErr := platClient.Post(fmt.Sprintf("/v2/accounts/%s/voiceConfigurationPackages/%s/phoneNumbers/bulk", acctID, vcpID), assignBody, &assignResp) + var lastAssignErr error + _, pollErr := cmdutil.Poll(cmdutil.PollConfig{ + Interval: 3 * time.Second, + Timeout: 90 * time.Second, + Check: func() (bool, interface{}, error) { + var assignResp interface{} + if err := platClient.Post(fmt.Sprintf("/v2/accounts/%s/voiceConfigurationPackages/%s/phoneNumbers/bulk", acctID, vcpID), assignBody, &assignResp); err != nil { + lastAssignErr = err // transient while the number provisions + return false, nil, nil + } + return true, assignResp, nil + }, + }) assignSpin.Stop() - if assignErr != nil { + if pollErr != nil { result.PhoneNumber = phoneNumber - return failWithPartial(result, fmt.Errorf("assigning number %s to VCP %s: %w", phoneNumber, vcpID, assignErr)) + return failWithPartial(result, fmt.Errorf("assigning number %s to VCP %s (number may still be provisioning): %w", phoneNumber, vcpID, lastAssignErr)) } ui.Successf("Number assigned to VCP") result.Status = "complete" @@ -399,20 +421,54 @@ func firstAssignedNumber(client *api.Client, acctID, vcpID string) (string, erro return firstE164(resp), nil } -// buildQuickstartOrderBody mirrors number.BuildOrderBody — a top-level -// TelephoneNumberList with no order-type wrapper, exactly like the proven -// `number order` command. siteID scopes the order to a sub-account on the -// legacy path; pass "" on the VCP path (no sub-account) to omit SiteId. -func buildQuickstartOrderBody(phoneNumber, siteID string) api.XMLBody { - data := map[string]interface{}{ - "TelephoneNumberList": map[string]interface{}{ - "TelephoneNumber": []string{phoneNumber}, // []string, exactly like number.BuildOrderBody - }, +// ensureSubaccount finds-or-creates a sub-account AND a default SIP peer +// (location) in it, returning the site ID. Ordering a number requires both a +// SiteId AND a default SIP peer on that site — without the peer the orders API +// fails with code 5020 ("No default SIP peer is set on the account and site"). +// Idempotent: re-running reuses the same named sub-account and location. +func ensureSubaccount(client *api.Client, acctID, name string) (string, error) { + // Sub-account (site). + siteID, err := findExistingID(client, fmt.Sprintf("/accounts/%s/sites", acctID), "Name", name, "Id", "id", "siteId") + if err != nil { + return "", err } if siteID != "" { - data["SiteId"] = siteID + ui.Successf("Sub-account (existing): %s", ui.ID(siteID)) + } else { + spin := ui.NewSpinner("Creating sub-account...") + spin.Start() + var resp interface{} + body := api.XMLBody{RootElement: "Site", Data: map[string]interface{}{"Name": name}} + err = client.Post(fmt.Sprintf("/accounts/%s/sites", acctID), body, &resp) + spin.Stop() + if err != nil { + return "", fmt.Errorf("creating sub-account: %w", err) + } + siteID = extractIDFromResponse(resp, "Id", "id", "siteId") + ui.Successf("Sub-account: %s", ui.ID(siteID)) + } + + // Default SIP peer (location) — required for ordering (avoids code 5020). + peerName := name + " Location" + existingPeer, err := findExistingID(client, fmt.Sprintf("/accounts/%s/sites/%s/sippeers", acctID, siteID), "PeerName", peerName, "PeerId", "Id", "id") + if err != nil { + return "", err + } + if existingPeer != "" { + ui.Successf("Location (existing): %s", ui.ID(existingPeer)) + } else { + spin := ui.NewSpinner("Creating default location...") + spin.Start() + var resp interface{} + body := api.XMLBody{RootElement: "SipPeer", Data: map[string]interface{}{"PeerName": peerName, "IsDefaultPeer": "true"}} + err = client.Post(fmt.Sprintf("/accounts/%s/sites/%s/sippeers", acctID, siteID), body, &resp) + spin.Stop() + if err != nil { + return "", fmt.Errorf("creating default location: %w", err) + } + ui.Successf("Location: %s", ui.ID(extractIDFromResponse(resp, "PeerId", "Id", "id"))) } - return api.XMLBody{RootElement: "Order", Data: data} + return siteID, nil } func searchAndOrderNumber(client *api.Client, acctID, siteID string) (string, error) { @@ -433,7 +489,8 @@ func searchAndOrderNumber(client *api.Client, acctID, siteID string) (string, er orderSpin := ui.NewSpinner(fmt.Sprintf("Ordering %s...", phoneNumber)) orderSpin.Start() var orderResp interface{} - orderBody := buildQuickstartOrderBody(phoneNumber, siteID) + // Reuse the shared, live-verified order body (SiteId + ExistingTelephoneNumberOrderType). + orderBody := api.XMLBody{RootElement: "Order", Data: numbercmd.BuildOrderBody(siteID, []string{phoneNumber})} orderErr := client.Post(fmt.Sprintf("/accounts/%s/orders", acctID), orderBody, &orderResp) orderSpin.Stop() if orderErr != nil { diff --git a/cmd/quickstart/quickstart_test.go b/cmd/quickstart/quickstart_test.go index 6be2530..531a6c8 100644 --- a/cmd/quickstart/quickstart_test.go +++ b/cmd/quickstart/quickstart_test.go @@ -128,35 +128,9 @@ func TestExtractPhoneNumber(t *testing.T) { } } -func TestBuildQuickstartOrderBody(t *testing.T) { - t.Run("mirrors number.BuildOrderBody shape (top-level TelephoneNumberList)", func(t *testing.T) { - body := buildQuickstartOrderBody("+19195551234", "") - if _, wrong := body.Data["ExistingTelephoneNumberOrderType"]; wrong { - t.Fatalf("must not use the ExistingTelephoneNumberOrderType wrapper: %#v", body.Data) - } - tnl, ok := body.Data["TelephoneNumberList"].(map[string]interface{}) - if !ok { - t.Fatalf("missing top-level TelephoneNumberList: %#v", body.Data) - } - // number.BuildOrderBody uses a []string even for a single TN — mirror it. - got, ok := tnl["TelephoneNumber"].([]string) - if !ok || len(got) != 1 || got[0] != "+19195551234" { - t.Fatalf("TelephoneNumber = %#v, want []string{\"+19195551234\"}", tnl["TelephoneNumber"]) - } - }) - t.Run("vcp path omits SiteId", func(t *testing.T) { - body := buildQuickstartOrderBody("+19195551234", "") - if _, ok := body.Data["SiteId"]; ok { - t.Fatalf("vcp order body must not include SiteId: %#v", body.Data) - } - }) - t.Run("legacy path scopes to the created sub-account", func(t *testing.T) { - body := buildQuickstartOrderBody("+19195551234", "site-9") - if body.Data["SiteId"] != "site-9" { - t.Fatalf("legacy order body SiteId = %v, want site-9", body.Data["SiteId"]) - } - }) -} +// Order-body construction now lives in number.BuildOrderBody (SiteId + +// ExistingTelephoneNumberOrderType wrapper, live-verified) and is covered by +// cmd/number/number_test.go's TestBuildOrderBody. func TestFindInMap(t *testing.T) { tests := []struct { From a4965f9e5bc159fa9e9500facc04c9d3f3857a00 Mon Sep 17 00:00:00 2001 From: Kush Date: Mon, 1 Jun 2026 16:29:31 -0400 Subject: [PATCH 17/23] fix(number): order --wait degrades gracefully when credential can't list TNs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If --wait polling hits a FeatureLimitError (credential lacks the Numbers role for /tns listing), the order itself already succeeded — surface it with a warning and exit 0 instead of reporting a conflict (exit 4). Verified live. --- cmd/number/order.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/cmd/number/order.go b/cmd/number/order.go index 29e8682..4ab9fb6 100644 --- a/cmd/number/order.go +++ b/cmd/number/order.go @@ -1,6 +1,7 @@ package number import ( + "errors" "fmt" "time" @@ -9,6 +10,7 @@ import ( "github.com/Bandwidth/cli/internal/api" "github.com/Bandwidth/cli/internal/cmdutil" "github.com/Bandwidth/cli/internal/output" + "github.com/Bandwidth/cli/internal/ui" ) var ( @@ -98,6 +100,16 @@ func runOrder(cmd *cobra.Command, args []string) error { }, }) if err != nil { + // The order itself already succeeded; --wait only verifies that the + // number reaches in-service. If this credential can't list numbers + // (lacks the Numbers role → FeatureLimitError), don't report that as a + // failure — surface the successful order and note we couldn't verify. + var fle *cmdutil.FeatureLimitError + if errors.As(err, &fle) { + ui.Warnf("Order placed, but in-service status can't be verified with this credential (it lacks the Numbers role for listing). Check the Bandwidth dashboard, or re-run without --wait.") + format, plain := cmdutil.OutputFlags(cmd) + return output.StdoutAuto(format, plain, result) + } return err } From dcc684c4d67d69727d2281c60d48305c72c8e9e6 Mon Sep 17 00:00:00 2001 From: Kush Date: Wed, 3 Jun 2026 09:37:58 -0400 Subject: [PATCH 18/23] fix(quickstart): scope VCP-assign retry, preserve timeout signal, correct docs Per code review: - VCP-assign retry now only retries VCS-0044 / 429 / 5xx / transport and fails fast on other 4xx (was retrying any error); add assignErrIsRetryable + test. - Preserve ErrPollTimeout (exit 5) on assign timeout instead of wrapping the last attempt error; surface last attempt + manual 'vcp assign' recovery. - number order docs now show the required --subaccount flag (README/AGENTS). - Correct the quickstart 'partial' resume docs: an ordered-but-unassigned number is not auto-reassigned on re-run. --- AGENTS.md | 10 ++++----- README.md | 6 +++--- cmd/quickstart/quickstart.go | 35 ++++++++++++++++++++++++++----- cmd/quickstart/quickstart_test.go | 27 ++++++++++++++++++++++++ 4 files changed, 65 insertions(+), 13 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 2f17af7..bff7c3f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -142,7 +142,7 @@ All read operations (gets, lists, deletes) are safe to retry. Use `--wait` to block until completion: ```bash -band number order +19195551234 --wait # blocks until number is active (30s default) +band number order +19195551234 --subaccount --wait # blocks until number is active (30s default) band call create --from ... --to ... --wait --timeout 120 # blocks until call completes band transcription create --wait # blocks until transcription ready (60s default) ``` @@ -203,12 +203,12 @@ For full flag/argument reference, use `band --help`. This section cove ### Quickstart -- **Agents should prefer the step-by-step provisioning workflows over `band quickstart`.** Quickstart creates real resources that cost money (it orders a phone number). The default (VCP) path is idempotent — re-running reuses existing resources via find-or-create and will not order a second number — and on failure it prints the resource IDs created so far (`status: partial`, see below) so a re-run can resume. The `--legacy` path is NOT idempotent (re-running it may order an additional number). Because quickstart bundles several steps behind one command, prefer the step-by-step provisioning workflows in the [Agent Workflows](#agent-workflows) section when you need per-step structured output or fine-grained control. +- **Agents should prefer the step-by-step provisioning workflows over `band quickstart`.** Quickstart creates real resources that cost money (it orders a phone number). The default (VCP) path is idempotent — re-running reuses existing resources via find-or-create and will not order a second number — and on failure it prints the resource IDs created so far (`status: partial`, see below). Re-running reuses the app/VCP/sub-account/location — but a number that was ordered and then failed to assign to the VCP is NOT auto-reassigned; finish it with `band vcp assign `. The `--legacy` path is NOT idempotent (re-running it may order an additional number). Because quickstart bundles several steps behind one command, prefer the step-by-step provisioning workflows in the [Agent Workflows](#agent-workflows) section when you need per-step structured output or fine-grained control. - **`band quickstart` output `status` values** (VCP path only — `--legacy` is not idempotent): - `complete` — all resources created and number assigned; ready to use. - `complete_no_number` — resources created but no number was available in the requested area code; re-run with `--area-code` to try a different code. - - `partial` — quickstart stopped after a failure but printed the resource IDs it created so far (app, VCP, and possibly an ordered phone number). Re-running quickstart reuses those resources via idempotency checks. If the assignment specifically failed, the ordered number is included in the partial output under `phoneNumber` so it isn't lost. + - `partial` — quickstart stopped after a failure but printed the resource IDs it created so far (app, VCP, sub-account, location, and possibly an ordered phone number). Re-running reuses the app/VCP/sub-account/location via idempotency checks. **Caveat:** if a number was ordered but its VCP assignment failed, the number is printed under `phoneNumber` but is NOT auto-reassigned on re-run (a re-run would order a *new* number) — finish the existing one with `band vcp assign `. --- @@ -330,7 +330,7 @@ band vcp create --name "Agent VCP" --app-id --if-not-exists --plain band number list --plain # 4. check existing numbers # if no numbers: band number search --area-code 919 --quantity 1 --plain -band number order --wait # 5. order number +band number order --subaccount --wait # 5. order number band vcp assign # 6. assign number to VCP band number activate --voice-inbound --wait # 7. enable inbound voice ``` @@ -347,7 +347,7 @@ band app create --name "Agent Voice" --type voice --callback-url --if-not- band number list --plain # 5. check numbers # if no numbers: band number search --area-code 919 --quantity 1 --plain -band number order --wait # 6. order number +band number order --subaccount --wait # 6. order number ``` ### Provision messaging from scratch diff --git a/README.md b/README.md index 08f3a4c..4ef14b2 100644 --- a/README.md +++ b/README.md @@ -138,7 +138,7 @@ Search for available numbers, then order one: ```sh band number search --area-code 919 --quantity 1 -band number order +19195551234 --wait +band number order +19195551234 --subaccount --wait ``` The `--wait` flag blocks until the number is active, so you don't have to poll. @@ -294,7 +294,7 @@ A fresh UP account typically has one sub-account and one location already create ```sh band number list # list your numbers band number search --area-code 919 --quantity 5 # search available numbers -band number order +19195551234 --wait # order (blocks until active) +band number order +19195551234 --subaccount --wait # order (blocks until active) band number activate +19195551234 --voice-inbound --wait # turn on inbound voice band number release +19195551234 # release a number ``` @@ -347,7 +347,7 @@ band subaccount create --name "My Subaccount" band location create --subaccount --name "My Location" band app create --name "My Voice App" --type voice --callback-url https://your-server.example.com/callbacks band number search --area-code 919 --quantity 1 -band number order +19195551234 --wait +band number order +19195551234 --subaccount --wait ``` Sub-accounts (formerly known as sites) are the top-level container. Locations (formerly known as SIP peers) sit inside sub-accounts and define where numbers get routed. The flow is: sub-account → location → application → number. diff --git a/cmd/quickstart/quickstart.go b/cmd/quickstart/quickstart.go index e73767f..b2038d8 100644 --- a/cmd/quickstart/quickstart.go +++ b/cmd/quickstart/quickstart.go @@ -2,10 +2,12 @@ package quickstart import ( "encoding/json" + "errors" "fmt" "net/url" "os" "regexp" + "strings" "time" "github.com/spf13/cobra" @@ -17,6 +19,22 @@ import ( "github.com/Bandwidth/cli/internal/ui" ) +// assignErrIsRetryable reports whether a failed VCP number-assignment is worth +// retrying. The just-ordered number provisions asynchronously, so the bulk +// assign returns VCS-0044 (HTTP 400) until it's ready — that, plus rate limits, +// 5xx, and transport errors, are transient. Auth/validation/not-found errors +// (other 4xx) are not retryable and should fail fast. +func assignErrIsRetryable(err error) bool { + var apiErr *api.APIError + if !errors.As(err, &apiErr) { + return true // transport/unknown error — treat as transient + } + if apiErr.StatusCode == 429 || apiErr.StatusCode >= 500 { + return true + } + return apiErr.StatusCode == 400 && strings.Contains(apiErr.Error(), "VCS-0044") +} + var ( qsCallbackURL string qsAreaCode string @@ -190,17 +208,24 @@ func runVCPQuickstart(cmd *cobra.Command) error { Timeout: 90 * time.Second, Check: func() (bool, interface{}, error) { var assignResp interface{} - if err := platClient.Post(fmt.Sprintf("/v2/accounts/%s/voiceConfigurationPackages/%s/phoneNumbers/bulk", acctID, vcpID), assignBody, &assignResp); err != nil { - lastAssignErr = err // transient while the number provisions - return false, nil, nil + err := platClient.Post(fmt.Sprintf("/v2/accounts/%s/voiceConfigurationPackages/%s/phoneNumbers/bulk", acctID, vcpID), assignBody, &assignResp) + if err == nil { + return true, assignResp, nil + } + lastAssignErr = err + if assignErrIsRetryable(err) { + return false, nil, nil // number still provisioning — keep polling } - return true, assignResp, nil + return false, nil, err // non-retryable (bad request/auth) — fail fast }, }) assignSpin.Stop() if pollErr != nil { result.PhoneNumber = phoneNumber - return failWithPartial(result, fmt.Errorf("assigning number %s to VCP %s (number may still be provisioning): %w", phoneNumber, vcpID, lastAssignErr)) + // pollErr is ErrPollTimeout on timeout (maps to exit 5) or the + // fail-fast error otherwise; keep it as %w and surface the last + // attempt as context. Tell the user how to finish manually. + return failWithPartial(result, fmt.Errorf("assigning number %s to VCP %s (last attempt: %v) — finish with: band vcp assign %s %s: %w", phoneNumber, vcpID, lastAssignErr, vcpID, phoneNumber, pollErr)) } ui.Successf("Number assigned to VCP") result.Status = "complete" diff --git a/cmd/quickstart/quickstart_test.go b/cmd/quickstart/quickstart_test.go index 531a6c8..bd9e683 100644 --- a/cmd/quickstart/quickstart_test.go +++ b/cmd/quickstart/quickstart_test.go @@ -1,9 +1,36 @@ package quickstart import ( + "errors" "testing" + + "github.com/Bandwidth/cli/internal/api" ) +func TestAssignErrIsRetryable(t *testing.T) { + cases := []struct { + name string + err error + want bool + }{ + {"VCS-0044 (number provisioning)", &api.APIError{StatusCode: 400, Body: `{"errors":[{"code":"VCS-0044"}]}`}, true}, + {"plain 400", &api.APIError{StatusCode: 400, Body: "bad request"}, false}, + {"401 auth", &api.APIError{StatusCode: 401, Body: ""}, false}, + {"403 forbidden", &api.APIError{StatusCode: 403, Body: ""}, false}, + {"404 not found", &api.APIError{StatusCode: 404, Body: ""}, false}, + {"429 rate limited", &api.APIError{StatusCode: 429, Body: ""}, true}, + {"500 server error", &api.APIError{StatusCode: 500, Body: ""}, true}, + {"transport error", errors.New("connection reset"), true}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + if got := assignErrIsRetryable(c.err); got != c.want { + t.Errorf("assignErrIsRetryable(%v) = %v, want %v", c.err, got, c.want) + } + }) + } +} + func TestExtractIDFromResponse(t *testing.T) { tests := []struct { name string From 70182e0a9c5093799a30d6b831a87b59322c8799 Mon Sep 17 00:00:00 2001 From: Kush Date: Wed, 3 Jun 2026 11:41:31 -0400 Subject: [PATCH 19/23] ci: bump Go to 1.26.4 to clear stdlib security advisories govulncheck flagged 4 Go stdlib CVEs (net, net/http, crypto/x509, net/textproto) reachable via version check + api client; all fixed in go1.26.3/1.26.4. Verified locally: govulncheck now reports 0 called vulns. --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 31bdd53..aec1cd3 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/Bandwidth/cli -go 1.26.2 +go 1.26.4 require ( github.com/briandowns/spinner v1.23.2 From 11a4f95a186a29dd4f493d513c82e6e16417cafe Mon Sep 17 00:00:00 2001 From: Kush Date: Wed, 3 Jun 2026 11:46:44 -0400 Subject: [PATCH 20/23] fix: address code review (messaging token realm, env validation, number lookup) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - MessagingClient now mints its OAuth token against the PROD API host, not the test realm — a test-realm token is rejected by the prod messaging endpoint (messaging is production-only). - resolveEnvironment normalizes case/whitespace and rejects unknown values instead of silently falling through to prod; --environment leak fixed by resetting EnvironmentOverride every PersistentPreRun. - firstAssignedNumber reads the explicit data[].phoneNumber field via FlattenResponse instead of regex-sniffing any numeric string (deleted firstE164/e164Re/phoneKeyRe); filter scoping verified live. - VCP-assign retry also retries 422; messaging warning reworded to be accurate for non-send commands. Verified live: messaging warning fires; VCP quickstart re-run detects the existing assigned number via the field read and skips ordering. --- cmd/quickstart/quickstart.go | 66 ++++++++++--------------------- cmd/quickstart/quickstart_test.go | 1 + cmd/root.go | 8 ++-- internal/cmdutil/helpers.go | 46 +++++++++++++++------ internal/cmdutil/helpers_test.go | 25 ++++++++++-- 5 files changed, 81 insertions(+), 65 deletions(-) diff --git a/cmd/quickstart/quickstart.go b/cmd/quickstart/quickstart.go index b2038d8..a236ba6 100644 --- a/cmd/quickstart/quickstart.go +++ b/cmd/quickstart/quickstart.go @@ -6,7 +6,6 @@ import ( "fmt" "net/url" "os" - "regexp" "strings" "time" @@ -29,7 +28,7 @@ func assignErrIsRetryable(err error) bool { if !errors.As(err, &apiErr) { return true // transport/unknown error — treat as transient } - if apiErr.StatusCode == 429 || apiErr.StatusCode >= 500 { + if apiErr.StatusCode == 429 || apiErr.StatusCode == 422 || apiErr.StatusCode >= 500 { return true } return apiErr.StatusCode == 400 && strings.Contains(apiErr.Error(), "VCS-0044") @@ -394,56 +393,31 @@ func findExistingID(client *api.Client, listPath, nameField, name string, idKeys return extractIDFromResponse(match, idKeys...), nil } -var e164Re = regexp.MustCompile(`^\+?\d{10,15}$`) - -// phoneKeyRe matches JSON keys that may hold a phone number (number, phoneNumber). -// It can also match keys like orderNumber, but those won't contain E.164 values -// in the VCP phoneNumbers/voice response, so firstE164 still picks the right one. -var phoneKeyRe = regexp.MustCompile(`(?i)(phone)?number`) - -// firstE164 walks a decoded response and returns the first value that looks -// like an E.164 phone number, preferring phone-number-looking keys before a -// field-agnostic fallback. Account IDs are <10 digits and won't match e164Re. -func firstE164(v interface{}) string { - switch x := v.(type) { - case map[string]interface{}: - for k, child := range x { // prefer phone-number-looking keys - if phoneKeyRe.MatchString(k) { - if s, ok := child.(string); ok && e164Re.MatchString(s) { - return s - } - } - } - for _, child := range x { // field-agnostic fallback - if s := firstE164(child); s != "" { - return s - } - } - case []interface{}: - for _, item := range x { - if s := firstE164(item); s != "" { - return s - } - } - case string: - if e164Re.MatchString(x) { - return x - } - } - return "" -} - -// firstAssignedNumber returns the first phone number already assigned to vcpID. -// It FAILS CLOSED: a list error is returned, not swallowed, so the caller does -// NOT order a duplicate paid number on a transient failure. Endpoint confirmed -// from cmd/vcp/numbers.go. +// firstAssignedNumber returns the first phone number already assigned to vcpID, +// reading the explicit `phoneNumber` field rather than sniffing for any numeric +// value. It FAILS CLOSED: a list error is returned, not swallowed, so the caller +// does NOT order a duplicate paid number on a transient failure. The +// voiceConfigurationPackageId filter is honored server-side (verified live), and +// the response shape is {"data":[{"phoneNumber":"+1...", ...}], ...}. func firstAssignedNumber(client *api.Client, acctID, vcpID string) (string, error) { var resp interface{} path := fmt.Sprintf("/v2/accounts/%s/phoneNumbers/voice?voiceConfigurationPackageId=%s", acctID, url.QueryEscape(vcpID)) if err := client.Get(path, &resp); err != nil { return "", fmt.Errorf("checking existing VCP numbers for %s: %w", vcpID, err) } - return firstE164(resp), nil + // FlattenResponse unwraps the {data, links, errors, page} envelope to the data array. + list, ok := output.FlattenResponse(resp).([]interface{}) + if !ok { + return "", nil + } + for _, item := range list { + if m, ok := item.(map[string]interface{}); ok { + if pn, ok := m["phoneNumber"].(string); ok && pn != "" { + return pn, nil + } + } + } + return "", nil } // ensureSubaccount finds-or-creates a sub-account AND a default SIP peer diff --git a/cmd/quickstart/quickstart_test.go b/cmd/quickstart/quickstart_test.go index bd9e683..b1e93cc 100644 --- a/cmd/quickstart/quickstart_test.go +++ b/cmd/quickstart/quickstart_test.go @@ -18,6 +18,7 @@ func TestAssignErrIsRetryable(t *testing.T) { {"401 auth", &api.APIError{StatusCode: 401, Body: ""}, false}, {"403 forbidden", &api.APIError{StatusCode: 403, Body: ""}, false}, {"404 not found", &api.APIError{StatusCode: 404, Body: ""}, false}, + {"422 unprocessable (not ready)", &api.APIError{StatusCode: 422, Body: ""}, true}, {"429 rate limited", &api.APIError{StatusCode: 429, Body: ""}, true}, {"500 server error", &api.APIError{StatusCode: 500, Body: ""}, true}, {"transport error", errors.New("connection reset"), true}, diff --git a/cmd/root.go b/cmd/root.go index ab806e1..1ca0552 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -51,10 +51,10 @@ var rootCmd = &cobra.Command{ Short: "Bandwidth CLI — manage voice, messaging, numbers, and more from the command line", Long: "The official Bandwidth CLI. Build and debug voice applications, send messages, manage phone numbers, and control calls.", PersistentPreRun: func(cmd *cobra.Command, args []string) { - // Wire the --environment flag into host/environment resolution. - if environment != "" { - cmdutil.EnvironmentOverride = environment - } + // Wire the --environment flag into host/environment resolution. Set + // unconditionally (empty when the flag is absent) so a value never + // leaks across invocations in a long-lived/multi-command process. + cmdutil.EnvironmentOverride = environment // Kick off version check in background so it doesn't slow down the command. updateResult = make(chan *versionpkg.CheckResult, 1) diff --git a/internal/cmdutil/helpers.go b/internal/cmdutil/helpers.go index 037a36c..0e68485 100644 --- a/internal/cmdutil/helpers.go +++ b/internal/cmdutil/helpers.go @@ -22,12 +22,20 @@ var EnvironmentOverride string // resolveEnvironment applies EnvironmentOverride (the --environment flag) on top // of the profile-derived environment (which already includes any BW_ENVIRONMENT -// overlay). -func resolveEnvironment(profileEnv string) string { +// overlay), normalizes case/whitespace, and validates it. An unrecognized value +// is an error rather than a silent fall-through to production. +func resolveEnvironment(profileEnv string) (string, error) { + env := profileEnv if EnvironmentOverride != "" { - return EnvironmentOverride + env = EnvironmentOverride + } + env = strings.ToLower(strings.TrimSpace(env)) + switch env { + case "", "prod", "test", "uat": + return env, nil + default: + return "", fmt.Errorf("unknown environment %q (expected one of: prod, test, uat)", env) } - return profileEnv } // apiHostForEnvironment maps an environment name to its API host. @@ -145,7 +153,10 @@ func authenticate(accountIDOverride string) (*auth.TokenManager, string, string, return nil, "", "", err } - env := resolveEnvironment(p.Environment) + env, err := resolveEnvironment(p.Environment) + if err != nil { + return nil, "", "", err + } apiHost := apiHostForEnvironment(env) tm := auth.NewTokenManager(p.ClientID, clientSecret, apiHost) return tm, acctID, env, nil @@ -213,25 +224,38 @@ func PlatformClient(accountIDOverride string) (*api.Client, string, error) { // messagingProdOnlyWarning returns a user warning when env is a non-production // environment, because the Bandwidth Messaging API is production-only (there is -// no test host). Returns "" when no warning is needed. +// no test host). Worded to be accurate for any messaging command (not just +// sends). Returns "" when no warning is needed. func messagingProdOnlyWarning(env string) string { if env == "test" || env == "uat" { - return "Bandwidth Messaging has no test environment — this request uses PRODUCTION regardless of --environment. Sends are real and billable." + return "Bandwidth Messaging has no test environment — this request hits PRODUCTION regardless of --environment (any sends are real and billable)." } return "" } // MessagingClient returns a client for the Bandwidth Messaging API v2. -// Messaging is production-only (see messagingHost) — the host never varies by -// --environment. When env is test or uat, a warning is printed to stderr because -// sends are real and billable. +// Messaging is production-only (see messagingHost): the host never varies by +// --environment, and — critically — the OAuth token is minted against the PROD +// API host too. authenticate() would mint the token against the test realm +// under --environment test, and a test-realm token is rejected by the prod +// messaging endpoint, so we build the token manager directly here. func MessagingClient(accountIDOverride string) (*api.Client, string, error) { - tm, acctID, env, err := authenticate(accountIDOverride) + cfg, p, clientSecret, err := loadConfigAndAuth() + if err != nil { + return nil, "", err + } + acctID, err := resolveAccountID(cfg, p, accountIDOverride) + if err != nil { + return nil, "", err + } + env, err := resolveEnvironment(p.Environment) if err != nil { return nil, "", err } if w := messagingProdOnlyWarning(env); w != "" { ui.Warnf("%s", w) } + // Always mint the token against prod (apiHostForEnvironment("prod")). + tm := auth.NewTokenManager(p.ClientID, clientSecret, apiHostForEnvironment("prod")) return api.NewClient(messagingHost()+"/api/v2", tm), acctID, nil } diff --git a/internal/cmdutil/helpers_test.go b/internal/cmdutil/helpers_test.go index 11c3c40..7b2c04b 100644 --- a/internal/cmdutil/helpers_test.go +++ b/internal/cmdutil/helpers_test.go @@ -68,15 +68,32 @@ func TestMessagingHost_BW_MESSAGING_URL_TrailingSlash(t *testing.T) { func TestResolveEnvironment(t *testing.T) { t.Run("no override returns profile env", func(t *testing.T) { EnvironmentOverride = "" - if got := resolveEnvironment("prod"); got != "prod" { - t.Errorf("got %q, want prod", got) + got, err := resolveEnvironment("prod") + if err != nil || got != "prod" { + t.Errorf("got %q, err %v; want prod, nil", got, err) } }) t.Run("override wins over profile env", func(t *testing.T) { EnvironmentOverride = "test" t.Cleanup(func() { EnvironmentOverride = "" }) - if got := resolveEnvironment("prod"); got != "test" { - t.Errorf("got %q, want test", got) + got, err := resolveEnvironment("prod") + if err != nil || got != "test" { + t.Errorf("got %q, err %v; want test, nil", got, err) + } + }) + t.Run("normalizes case and whitespace", func(t *testing.T) { + EnvironmentOverride = " TEST " + t.Cleanup(func() { EnvironmentOverride = "" }) + got, err := resolveEnvironment("prod") + if err != nil || got != "test" { + t.Errorf("got %q, err %v; want test, nil", got, err) + } + }) + t.Run("unknown env is an error (no silent prod fall-through)", func(t *testing.T) { + EnvironmentOverride = "staging" + t.Cleanup(func() { EnvironmentOverride = "" }) + if _, err := resolveEnvironment("prod"); err == nil { + t.Error("expected error for unknown env, got nil") } }) } From c1eba69017873dba81bad234f7d4eff4e2ad85a6 Mon Sep 17 00:00:00 2001 From: Kush Date: Wed, 3 Jun 2026 11:48:32 -0400 Subject: [PATCH 21/23] test: extract golden-test scaffolding to internal/testutil Removes the byte-identical FakeClient/NewTestRoot/CaptureStdout copies from the call and recording golden tests so an api.Requester change updates one place. --- cmd/call/golden_test.go | 58 +++----------------------------- cmd/recording/golden_test.go | 58 +++----------------------------- internal/testutil/golden.go | 64 ++++++++++++++++++++++++++++++++++++ 3 files changed, 72 insertions(+), 108 deletions(-) create mode 100644 internal/testutil/golden.go diff --git a/cmd/call/golden_test.go b/cmd/call/golden_test.go index a1624b1..f6eb9aa 100644 --- a/cmd/call/golden_test.go +++ b/cmd/call/golden_test.go @@ -3,63 +3,13 @@ package call import ( "bytes" "encoding/json" - "io" - "os" "testing" - "github.com/spf13/cobra" - "github.com/Bandwidth/cli/internal/api" "github.com/Bandwidth/cli/internal/cmdutil" + "github.com/Bandwidth/cli/internal/testutil" ) -// fakeClient implements api.Requester. Get writes a canned fixture. -type fakeClient struct { - getResult interface{} -} - -func (f *fakeClient) Get(path string, result interface{}) error { - b, _ := json.Marshal(f.getResult) - return json.Unmarshal(b, result) -} -func (f *fakeClient) Post(string, interface{}, interface{}) error { return nil } -func (f *fakeClient) Put(string, interface{}, interface{}) error { return nil } -func (f *fakeClient) Patch(string, interface{}, interface{}) error { return nil } -func (f *fakeClient) Delete(string, interface{}) error { return nil } -func (f *fakeClient) GetRaw(string) ([]byte, error) { return nil, nil } -func (f *fakeClient) PutRaw(string, []byte, string) error { return nil } - -// newTestRoot builds a minimal root with the persistent flags commands read. -func newTestRoot(child *cobra.Command) *cobra.Command { - root := &cobra.Command{Use: "band", SilenceUsage: true, SilenceErrors: true} - root.PersistentFlags().String("format", "json", "") - root.PersistentFlags().Bool("plain", false, "") - root.PersistentFlags().String("account-id", "", "") - root.PersistentFlags().String("environment", "", "") - root.AddCommand(child) - return root -} - -// captureStdout runs fn while capturing everything written to os.Stdout. -func captureStdout(t *testing.T, fn func()) string { - t.Helper() - orig := os.Stdout - r, w, err := os.Pipe() - if err != nil { - t.Fatal(err) - } - os.Stdout = w - fn() - w.Close() - os.Stdout = orig - var out []byte - out, err = io.ReadAll(r) - if err != nil { - t.Fatal(err) - } - return string(out) -} - func TestCallListPlainOutput(t *testing.T) { // Fixture is an API-shaped WRAPPER object ({"calls": [...]}), so a passing // assertion on got[0]["callId"] proves FlattenResponse stripped the wrapper. @@ -67,17 +17,17 @@ func TestCallListPlainOutput(t *testing.T) { orig := cmdutil.VoiceClient t.Cleanup(func() { cmdutil.VoiceClient = orig }) cmdutil.VoiceClient = func(string) (api.Requester, string, error) { - return &fakeClient{getResult: map[string]interface{}{ + return &testutil.FakeClient{GetResult: map[string]interface{}{ "calls": []interface{}{ map[string]interface{}{"callId": "c-1", "state": "active"}, }, }}, "acct-123", nil } - root := newTestRoot(listCmd) + root := testutil.NewTestRoot(listCmd) root.SetArgs([]string{"list", "--plain"}) - out := captureStdout(t, func() { + out := testutil.CaptureStdout(t, func() { if err := root.Execute(); err != nil { t.Fatalf("execute: %v", err) } diff --git a/cmd/recording/golden_test.go b/cmd/recording/golden_test.go index 3e62b31..ac3f0b4 100644 --- a/cmd/recording/golden_test.go +++ b/cmd/recording/golden_test.go @@ -3,79 +3,29 @@ package recording import ( "bytes" "encoding/json" - "io" - "os" "testing" - "github.com/spf13/cobra" - "github.com/Bandwidth/cli/internal/api" "github.com/Bandwidth/cli/internal/cmdutil" + "github.com/Bandwidth/cli/internal/testutil" ) -// fakeClient implements api.Requester. Get writes a canned fixture. -type fakeClient struct { - getResult interface{} -} - -func (f *fakeClient) Get(path string, result interface{}) error { - b, _ := json.Marshal(f.getResult) - return json.Unmarshal(b, result) -} -func (f *fakeClient) Post(string, interface{}, interface{}) error { return nil } -func (f *fakeClient) Put(string, interface{}, interface{}) error { return nil } -func (f *fakeClient) Patch(string, interface{}, interface{}) error { return nil } -func (f *fakeClient) Delete(string, interface{}) error { return nil } -func (f *fakeClient) GetRaw(string) ([]byte, error) { return nil, nil } -func (f *fakeClient) PutRaw(string, []byte, string) error { return nil } - -// newTestRoot builds a minimal root with the persistent flags commands read. -func newTestRoot(child *cobra.Command) *cobra.Command { - root := &cobra.Command{Use: "band", SilenceUsage: true, SilenceErrors: true} - root.PersistentFlags().String("format", "json", "") - root.PersistentFlags().Bool("plain", false, "") - root.PersistentFlags().String("account-id", "", "") - root.PersistentFlags().String("environment", "", "") - root.AddCommand(child) - return root -} - -// captureStdout runs fn while capturing everything written to os.Stdout. -func captureStdout(t *testing.T, fn func()) string { - t.Helper() - orig := os.Stdout - r, w, err := os.Pipe() - if err != nil { - t.Fatal(err) - } - os.Stdout = w - fn() - w.Close() - os.Stdout = orig - var out []byte - out, err = io.ReadAll(r) - if err != nil { - t.Fatal(err) - } - return string(out) -} - func TestRecordingListPlainOutput(t *testing.T) { // No t.Parallel(): these tests mutate the global cmdutil.VoiceClient. orig := cmdutil.VoiceClient t.Cleanup(func() { cmdutil.VoiceClient = orig }) cmdutil.VoiceClient = func(string) (api.Requester, string, error) { - return &fakeClient{getResult: map[string]interface{}{ + return &testutil.FakeClient{GetResult: map[string]interface{}{ "recordings": []interface{}{ map[string]interface{}{"recordingId": "r-1", "status": "complete"}, }, }}, "acct-123", nil } - root := newTestRoot(listCmd) + root := testutil.NewTestRoot(listCmd) root.SetArgs([]string{"list", "c-abc123", "--plain"}) // callId positional arg required - out := captureStdout(t, func() { + out := testutil.CaptureStdout(t, func() { if err := root.Execute(); err != nil { t.Fatalf("execute: %v", err) } diff --git a/internal/testutil/golden.go b/internal/testutil/golden.go new file mode 100644 index 0000000..e53fb50 --- /dev/null +++ b/internal/testutil/golden.go @@ -0,0 +1,64 @@ +// Package testutil provides shared helpers for command-level golden tests: +// a fake api.Requester, a minimal root command with the persistent flags +// commands read, and an os.Stdout capture. It lives in a regular package (not a +// _test.go file) so multiple command packages can share it; only _test.go files +// import it, so it is never linked into the production binary. +package testutil + +import ( + "encoding/json" + "io" + "os" + "testing" + + "github.com/spf13/cobra" +) + +// FakeClient implements api.Requester. Get marshals GetResult into the caller's +// result pointer (a JSON round-trip), mimicking a real API response; the other +// methods are no-ops. Set GetResult to the canned fixture for the command. +type FakeClient struct { + GetResult interface{} +} + +func (f *FakeClient) Get(path string, result interface{}) error { + b, _ := json.Marshal(f.GetResult) + return json.Unmarshal(b, result) +} +func (f *FakeClient) Post(string, interface{}, interface{}) error { return nil } +func (f *FakeClient) Put(string, interface{}, interface{}) error { return nil } +func (f *FakeClient) Patch(string, interface{}, interface{}) error { return nil } +func (f *FakeClient) Delete(string, interface{}) error { return nil } +func (f *FakeClient) GetRaw(string) ([]byte, error) { return nil, nil } +func (f *FakeClient) PutRaw(string, []byte, string) error { return nil } + +// NewTestRoot builds a minimal root command carrying the persistent flags that +// command implementations read via cmd.Root().Flag(...), with child attached. +func NewTestRoot(child *cobra.Command) *cobra.Command { + root := &cobra.Command{Use: "band", SilenceUsage: true, SilenceErrors: true} + root.PersistentFlags().String("format", "json", "") + root.PersistentFlags().Bool("plain", false, "") + root.PersistentFlags().String("account-id", "", "") + root.PersistentFlags().String("environment", "", "") + root.AddCommand(child) + return root +} + +// CaptureStdout runs fn while capturing everything written to os.Stdout. +func CaptureStdout(t *testing.T, fn func()) string { + t.Helper() + orig := os.Stdout + r, w, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + os.Stdout = w + fn() + w.Close() + os.Stdout = orig + out, err := io.ReadAll(r) + if err != nil { + t.Fatal(err) + } + return string(out) +} From dc1f661d7eae9abe77b7e0d596811020f3109e12 Mon Sep 17 00:00:00 2001 From: Kush Date: Wed, 3 Jun 2026 11:50:27 -0400 Subject: [PATCH 22/23] refactor(quickstart): share ensureVoiceApp helper across both paths The voice-app find-or-create was duplicated between the VCP and legacy paths; extract it so the app payload can't drift. (The sub-account/SIP-peer find-or- create differs between paths in peer naming and result-field population, so that unification is left as a deliberate follow-up rather than risk the legacy path.) --- cmd/quickstart/quickstart.go | 96 +++++++++++++++--------------------- 1 file changed, 41 insertions(+), 55 deletions(-) diff --git a/cmd/quickstart/quickstart.go b/cmd/quickstart/quickstart.go index a236ba6..b016530 100644 --- a/cmd/quickstart/quickstart.go +++ b/cmd/quickstart/quickstart.go @@ -105,38 +105,14 @@ func runVCPQuickstart(cmd *cobra.Command) error { result := quickstartResult{CallbackURL: qsCallbackURL, Path: "vcp"} // Step 1: Create voice application (idempotent: reuse if already exists) - appName := qsName + " App" - existingApp, err := findExistingID(dashClient, fmt.Sprintf("/accounts/%s/applications", acctID), "AppName", appName, "ApplicationId", "applicationId") + appID, err := ensureVoiceApp(dashClient, acctID, qsName+" App", qsCallbackURL) if err != nil { + // App provisioning failing often means this is a legacy account. + fmt.Fprintf(os.Stderr, "\nVoice application setup failed. If this is a legacy account, try:\n") + fmt.Fprintf(os.Stderr, " band quickstart --callback-url %s --legacy\n\n", qsCallbackURL) return failWithPartial(result, err) } - result.AppID = existingApp - if result.AppID != "" { - ui.Successf("Application (existing): %s", ui.ID(result.AppID)) - } else { - appSpin := ui.NewSpinner("Creating voice application...") - appSpin.Start() - var appResp interface{} - appBody := api.XMLBody{ - RootElement: "Application", - Data: map[string]interface{}{ - "ServiceType": "Voice-V2", - "AppName": appName, - "CallInitiatedCallbackUrl": qsCallbackURL, - }, - } - appErr := dashClient.Post(fmt.Sprintf("/accounts/%s/applications", acctID), appBody, &appResp) - appSpin.Stop() - if appErr != nil { - // If voice app creation fails with 409, suggest --legacy - fmt.Fprintf(os.Stderr, "\nVoice application creation failed. If this is a legacy account, try:\n") - fmt.Fprintf(os.Stderr, " band quickstart --callback-url %s --legacy\n\n", qsCallbackURL) - return failWithPartial(result, fmt.Errorf("creating voice application: %w", appErr)) - } - result.AppID = extractIDFromResponse(appResp, "ApplicationId", "applicationId") - ui.Successf("Application: %s", ui.ID(result.AppID)) - } - appID := result.AppID + result.AppID = appID // Step 2: Create VCP linked to the app (idempotent: reuse if already exists) vcpName := qsName + " VCP" @@ -309,35 +285,11 @@ func runLegacyQuickstart(cmd *cobra.Command) error { } // Step 3: Create voice application (idempotent: reuse if already exists) - appName := qsName + " App" - existingApp, err := findExistingID(client, fmt.Sprintf("/accounts/%s/applications", acctID), "AppName", appName, "ApplicationId", "applicationId") + appID, err := ensureVoiceApp(client, acctID, qsName+" App", qsCallbackURL) if err != nil { return failWithPartial(result, err) } - result.AppID = existingApp - if result.AppID != "" { - ui.Successf("Application (existing): %s", ui.ID(result.AppID)) - } else { - appSpin := ui.NewSpinner("Creating voice application...") - appSpin.Start() - var appResp interface{} - appBody := api.XMLBody{ - RootElement: "Application", - Data: map[string]interface{}{ - "ServiceType": "Voice-V2", - "AppName": appName, - "CallInitiatedCallbackUrl": qsCallbackURL, - }, - } - appErr := client.Post(fmt.Sprintf("/accounts/%s/applications", acctID), appBody, &appResp) - appSpin.Stop() - if appErr != nil { - return failWithPartial(result, fmt.Errorf("creating application: %w", appErr)) - } - result.AppID = extractIDFromResponse(appResp, "ApplicationId", "applicationId") - ui.Successf("Application: %s", ui.ID(result.AppID)) - } - appID := result.AppID + result.AppID = appID // Step 4: Search and order a number. // TODO: Legacy number ordering cannot be made idempotent here because there is no @@ -470,6 +422,40 @@ func ensureSubaccount(client *api.Client, acctID, name string) (string, error) { return siteID, nil } +// ensureVoiceApp find-or-creates a Voice-V2 application named appName with the +// given callback URL and returns its application ID. Idempotent: re-running +// reuses an existing app with the same name. Shared by both quickstart paths so +// the app payload can't drift between them. +func ensureVoiceApp(client *api.Client, acctID, appName, callbackURL string) (string, error) { + existing, err := findExistingID(client, fmt.Sprintf("/accounts/%s/applications", acctID), "AppName", appName, "ApplicationId", "applicationId") + if err != nil { + return "", err + } + if existing != "" { + ui.Successf("Application (existing): %s", ui.ID(existing)) + return existing, nil + } + spin := ui.NewSpinner("Creating voice application...") + spin.Start() + var resp interface{} + body := api.XMLBody{ + RootElement: "Application", + Data: map[string]interface{}{ + "ServiceType": "Voice-V2", + "AppName": appName, + "CallInitiatedCallbackUrl": callbackURL, + }, + } + err = client.Post(fmt.Sprintf("/accounts/%s/applications", acctID), body, &resp) + spin.Stop() + if err != nil { + return "", fmt.Errorf("creating voice application: %w", err) + } + id := extractIDFromResponse(resp, "ApplicationId", "applicationId") + ui.Successf("Application: %s", ui.ID(id)) + return id, nil +} + func searchAndOrderNumber(client *api.Client, acctID, siteID string) (string, error) { searchSpin := ui.NewSpinner(fmt.Sprintf("Searching for number in area code %s...", qsAreaCode)) searchSpin.Start() From cfbf0b3059f1c6f322e1a416862690a693d723d1 Mon Sep 17 00:00:00 2001 From: Kush Date: Wed, 3 Jun 2026 11:57:27 -0400 Subject: [PATCH 23/23] docs: reflect required --subaccount on number order and VCP-path sub-account provisioning - README command table: number order shows the required --subaccount. - AGENTS number-order guidance notes --subaccount is required. - README quickstart description no longer implies only --legacy uses a sub-account (the VCP path now provisions one too). --- AGENTS.md | 2 +- README.md | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index bff7c3f..8c86088 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -133,7 +133,7 @@ band app create --name "My App" --type voice --callback-url --if-not-exist band vcp create --name "My VCP" --if-not-exists ``` -For `number order`, there is no `--if-not-exists` — check `band number list --plain` first. +`number order` requires `--subaccount ` (the orders API needs a sub-account to order into; see `band subaccount list`). There is no `--if-not-exists` — check `band number list --plain` first. All read operations (gets, lists, deletes) are safe to retry. diff --git a/README.md b/README.md index 4ef14b2..237585f 100644 --- a/README.md +++ b/README.md @@ -404,7 +404,7 @@ Sub-accounts (formerly known as sites) are the top-level container. Locations (f | Command | What it does | |---------|-------------| | `band number search` | Search available numbers by area code | -| `band number order ` | Order numbers | +| `band number order --subaccount ` | Order numbers into a sub-account (`--subaccount` required) | | `band number get ` | Get voice config details (including VCP assignment) | | `band number activate ` | Activate voice/messaging services (e.g. enable inbound) | | `band number deactivate ` | Deactivate voice/messaging services | @@ -467,7 +467,7 @@ Sub-accounts (formerly known as sites) are the top-level container. Locations (f | Command | What it does | |---------|-------------| -| `band quickstart` | One-command setup: creates app, orders number, wires everything up (use `--legacy` for sub-account path) | +| `band quickstart` | One-command setup: provisions an app + VCP + sub-account/location, orders a number, and assigns it (`--legacy` uses the pre-VCP provisioning path) | | `band bxml ` | Generate BXML locally (no auth needed) | | `band version` | Print CLI version |