From d9591b6ab40dbb736d45a753612bffb8f1092404 Mon Sep 17 00:00:00 2001 From: Kush Date: Thu, 13 Aug 2026 14:41:35 -0400 Subject: [PATCH 01/21] feat(cmdutil): make ExitFlagError reachable via typed FlagError --- internal/cmdutil/exitcodes.go | 29 +++++++++++++++++++++++++++++ internal/cmdutil/exitcodes_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/internal/cmdutil/exitcodes.go b/internal/cmdutil/exitcodes.go index 7efb370..1da022e 100644 --- a/internal/cmdutil/exitcodes.go +++ b/internal/cmdutil/exitcodes.go @@ -2,6 +2,8 @@ package cmdutil import ( "errors" + "sort" + "strings" "github.com/Bandwidth/cli/internal/api" ) @@ -27,6 +29,29 @@ type SecretUnavailableError struct{ Message string } func (e *SecretUnavailableError) Error() string { return e.Message } +// FlagError reports invalid or missing command-line flags. It maps to +// ExitFlagError (6) so agents can distinguish "you called this wrong" — +// fixable without any API state — from a server-side failure. No HTTP +// request has been made when this is returned. +type FlagError struct{ Message string } + +func (e *FlagError) Error() string { return e.Message } + +// NewFlagError returns a FlagError with the given message. +func NewFlagError(msg string) error { return &FlagError{Message: msg} } + +// NewMissingFlagsError reports every missing required flag in one error, +// sorted for determinism. Cobra's MarkFlagRequired is not used on commands +// with conditional requirements, so aggregation happens here instead. +func NewMissingFlagsError(names []string) error { + sorted := append([]string(nil), names...) + sort.Strings(sorted) + for i, n := range sorted { + sorted[i] = "--" + n + } + return &FlagError{Message: "missing required flags: " + strings.Join(sorted, ", ")} +} + // ConflictError reports that the target resource exists but is not in a state // where the requested operation can succeed — a duplicate, a wrong lifecycle // state, a mismatched existing setting. It maps to ExitConflict (4). @@ -60,6 +85,10 @@ func ExitCodeForError(err error) int { if errors.Is(err, ErrPollTimeout) { return ExitTimeout } + var flagErr *FlagError + if errors.As(err, &flagErr) { + return ExitFlagError + } 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 4bcaef7..4e79c65 100644 --- a/internal/cmdutil/exitcodes_test.go +++ b/internal/cmdutil/exitcodes_test.go @@ -114,3 +114,30 @@ func TestConflictError_PreservesCauseChain(t *testing.T) { t.Errorf("unwrapped Code = %q, want 33006", got.Code) } } + +func TestFlagErrorExitCode(t *testing.T) { + err := cmdutil.NewFlagError("bad value for --evp") + if got := cmdutil.ExitCodeForError(err); got != cmdutil.ExitFlagError { + t.Errorf("ExitCodeForError = %d, want %d", got, cmdutil.ExitFlagError) + } +} + +func TestMissingFlagsErrorListsAllNames(t *testing.T) { + err := cmdutil.NewMissingFlagsError([]string{"usecase", "description", "sample1"}) + want := "missing required flags: --description, --sample1, --usecase" + if err.Error() != want { + t.Errorf("Error() = %q, want %q", err.Error(), want) + } + if got := cmdutil.ExitCodeForError(err); got != cmdutil.ExitFlagError { + t.Errorf("ExitCodeForError = %d, want %d", got, cmdutil.ExitFlagError) + } +} + +// A FlagError must win over a wrapped APIError: it is a client-side +// failure and no request was ever sent. +func TestFlagErrorTakesPrecedenceOverAPIError(t *testing.T) { + wrapped := fmt.Errorf("%w", cmdutil.NewFlagError("bad")) + if got := cmdutil.ExitCodeForError(wrapped); got != cmdutil.ExitFlagError { + t.Errorf("ExitCodeForError = %d, want %d", got, cmdutil.ExitFlagError) + } +} From d42c238913c9f19fcabd68cb8f9a6174727f4cb3 Mon Sep 17 00:00:00 2001 From: Kush Date: Thu, 13 Aug 2026 14:45:55 -0400 Subject: [PATCH 02/21] test(cmdutil): add competing error to FlagError precedence test --- internal/cmdutil/exitcodes_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/cmdutil/exitcodes_test.go b/internal/cmdutil/exitcodes_test.go index 4e79c65..5661d29 100644 --- a/internal/cmdutil/exitcodes_test.go +++ b/internal/cmdutil/exitcodes_test.go @@ -136,7 +136,10 @@ func TestMissingFlagsErrorListsAllNames(t *testing.T) { // A FlagError must win over a wrapped APIError: it is a client-side // failure and no request was ever sent. func TestFlagErrorTakesPrecedenceOverAPIError(t *testing.T) { - wrapped := fmt.Errorf("%w", cmdutil.NewFlagError("bad")) + // Build an error chain with both FlagError and APIError; FlagError must win. + // If the flagErr branch in ExitCodeForError is moved after APIError handling, + // this test fails: the 403 APIError (ExitAuth=2) would be checked first. + wrapped := fmt.Errorf("flag problem: %w (during %w)", cmdutil.NewFlagError("bad"), &api.APIError{StatusCode: 403, Body: "forbidden"}) if got := cmdutil.ExitCodeForError(wrapped); got != cmdutil.ExitFlagError { t.Errorf("ExitCodeForError = %d, want %d", got, cmdutil.ExitFlagError) } From fd94548bb26269983bc2ac0a6cb653dbc622a362 Mon Sep 17 00:00:00 2001 From: Kush Date: Thu, 13 Aug 2026 14:49:18 -0400 Subject: [PATCH 03/21] feat(api): capture response headers on APIError and add RetryAfter --- internal/api/client.go | 27 +++++++++++++++++++++++++-- internal/api/client_test.go | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 2 deletions(-) diff --git a/internal/api/client.go b/internal/api/client.go index db2abfe..c44067c 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -8,6 +8,7 @@ import ( "mime/multipart" "net/http" "net/textproto" + "strconv" "strings" "time" @@ -26,6 +27,28 @@ func userAgent() string { type APIError struct { StatusCode int Body string + // Header is the response header set. Populated on every error path so + // callers can honor Retry-After and the X-Rate-Limit-* family, which + // only ever appear on the 429 that needs them. + Header http.Header +} + +// RetryAfter reports the Retry-After delay if the server sent one. Only the +// delta-seconds form is supported — Bandwidth's v2 A2P APIs do not send the +// HTTP-date form. +func (e *APIError) RetryAfter() (time.Duration, bool) { + if e.Header == nil { + return 0, false + } + v := strings.TrimSpace(e.Header.Get("Retry-After")) + if v == "" { + return 0, false + } + secs, err := strconv.Atoi(v) + if err != nil || secs < 0 { + return 0, false + } + return time.Duration(secs) * time.Second, true } func (e *APIError) Error() string { @@ -136,7 +159,7 @@ func (c *Client) doRaw(req *http.Request) ([]byte, error) { } if resp.StatusCode < 200 || resp.StatusCode >= 300 { - return nil, &APIError{StatusCode: resp.StatusCode, Body: string(data)} + return nil, &APIError{StatusCode: resp.StatusCode, Body: string(data), Header: resp.Header.Clone()} } return data, nil } @@ -279,7 +302,7 @@ func (c *Client) PostXMLReturnLocation(path string, body XMLBody) (string, error return "", fmt.Errorf("reading response body: %w", err) } if resp.StatusCode < 200 || resp.StatusCode >= 300 { - return "", &APIError{StatusCode: resp.StatusCode, Body: string(respBody)} + return "", &APIError{StatusCode: resp.StatusCode, Body: string(respBody), Header: resp.Header.Clone()} } return resp.Header.Get("Location"), nil } diff --git a/internal/api/client_test.go b/internal/api/client_test.go index c500303..5dca980 100644 --- a/internal/api/client_test.go +++ b/internal/api/client_test.go @@ -2,10 +2,12 @@ package api import ( "encoding/json" + "errors" "net/http" "net/http/httptest" "strings" "testing" + "time" "github.com/Bandwidth/cli/internal/auth" ) @@ -466,3 +468,36 @@ func TestXMLClient_NonXMLBodyReturnsError(t *testing.T) { t.Errorf("expected error mentioning XMLBody, got: %v", err) } } + +func TestAPIErrorCapturesHeaders(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Retry-After", "7") + w.Header().Set("X-Rate-Limit-Remaining", "0") + w.WriteHeader(http.StatusTooManyRequests) + _, _ = w.Write([]byte(`{"errors":[]}`)) + })) + defer srv.Close() + + c := NewClientNoAuth(srv.URL) + var out any + err := c.Get("/x", &out) + + var apiErr *APIError + if !errors.As(err, &apiErr) { + t.Fatalf("expected *APIError, got %T", err) + } + if got := apiErr.Header.Get("X-Rate-Limit-Remaining"); got != "0" { + t.Errorf("X-Rate-Limit-Remaining = %q, want %q", got, "0") + } + d, ok := apiErr.RetryAfter() + if !ok || d != 7*time.Second { + t.Errorf("RetryAfter() = %v, %v; want 7s, true", d, ok) + } +} + +func TestRetryAfterAbsentReportsFalse(t *testing.T) { + e := &APIError{StatusCode: 500, Header: http.Header{}} + if _, ok := e.RetryAfter(); ok { + t.Error("RetryAfter() reported ok with no header") + } +} From 48b08cccaaec0da5077bd845a26c774962bd2de4 Mon Sep 17 00:00:00 2001 From: Kush Date: Thu, 13 Aug 2026 14:53:47 -0400 Subject: [PATCH 04/21] docs(api): narrow APIError.Header comment to what the codebase guarantees --- internal/api/client.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/internal/api/client.go b/internal/api/client.go index c44067c..36ae00f 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -27,9 +27,13 @@ func userAgent() string { type APIError struct { StatusCode int Body string - // Header is the response header set. Populated on every error path so - // callers can honor Retry-After and the X-Rate-Limit-* family, which - // only ever appear on the 429 that needs them. + // Header is the response header set. Populated whenever api.Client itself + // constructs the error (the doRaw and PostXMLReturnLocation paths), so + // callers can honor Retry-After and the X-Rate-Limit-* family, which only + // ever appear on the 429 that needs them. Code elsewhere that builds an + // APIError from a parsed response body rather than an *http.Response — + // internal/sip is the current example — may leave this nil. RetryAfter + // returns false in that case; treat that as "unknown," not "none sent." Header http.Header } From aa62c42e4ca2218c075199f964b7f5ecd229c375 Mon Sep 17 00:00:00 2001 From: Kush Date: Thu, 13 Aug 2026 14:58:55 -0400 Subject: [PATCH 05/21] feat(cmdutil): make Poll cancellable via optional PollConfig.Context --- internal/cmdutil/poll.go | 24 +++++++++++++++++++++++- internal/cmdutil/poll_test.go | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/internal/cmdutil/poll.go b/internal/cmdutil/poll.go index 37e7f79..f779507 100644 --- a/internal/cmdutil/poll.go +++ b/internal/cmdutil/poll.go @@ -1,6 +1,7 @@ package cmdutil import ( + "context" "errors" "fmt" "time" @@ -13,6 +14,9 @@ var ErrPollTimeout = errors.New("operation did not complete in time") // PollConfig configures a polling loop. type PollConfig struct { + // Context cancels the poll loop. Optional — nil means context.Background(). + // Existing callers omit it and keep the previous behavior exactly. + Context context.Context Interval time.Duration Timeout time.Duration // Check performs one poll attempt. It should return done=true when the @@ -25,7 +29,17 @@ type PollConfig struct { // cfg.Timeout is exceeded. On success it returns the result from Check. // On timeout it returns ErrPollTimeout. func Poll(cfg PollConfig) (interface{}, error) { + ctx := cfg.Context + if ctx == nil { + ctx = context.Background() + } deadline := time.Now().Add(cfg.Timeout) + timer := time.NewTimer(0) + if !timer.Stop() { + <-timer.C + } + defer timer.Stop() + for { done, result, err := cfg.Check() if err != nil { @@ -37,6 +51,14 @@ func Poll(cfg PollConfig) (interface{}, error) { if time.Now().After(deadline) { return nil, fmt.Errorf("timed out after %s: %w", cfg.Timeout, ErrPollTimeout) } - time.Sleep(cfg.Interval) + timer.Reset(cfg.Interval) + select { + case <-ctx.Done(): + if !timer.Stop() { + <-timer.C + } + return nil, ctx.Err() + case <-timer.C: + } } } diff --git a/internal/cmdutil/poll_test.go b/internal/cmdutil/poll_test.go index 3ac1906..bc954ad 100644 --- a/internal/cmdutil/poll_test.go +++ b/internal/cmdutil/poll_test.go @@ -1,6 +1,7 @@ package cmdutil import ( + "context" "errors" "testing" "time" @@ -48,3 +49,37 @@ func TestPollPropagatesCheckError(t *testing.T) { t.Fatalf("expected boom, got %v", err) } } + +func TestPollRespectsContextCancellation(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + calls := 0 + go func() { time.Sleep(30 * time.Millisecond); cancel() }() + + _, err := Poll(PollConfig{ + Context: ctx, + Interval: 10 * time.Millisecond, + Timeout: 10 * time.Second, + Check: func() (bool, interface{}, error) { + calls++ + return false, nil, nil + }, + }) + if !errors.Is(err, context.Canceled) { + t.Fatalf("err = %v, want context.Canceled", err) + } + if calls == 0 { + t.Error("Check was never called") + } +} + +// A nil Context must behave exactly as before — all existing callers omit it. +func TestPollNilContextStillTimesOut(t *testing.T) { + _, err := Poll(PollConfig{ + Interval: 5 * time.Millisecond, + Timeout: 20 * time.Millisecond, + Check: func() (bool, interface{}, error) { return false, nil, nil }, + }) + if !errors.Is(err, ErrPollTimeout) { + t.Fatalf("err = %v, want ErrPollTimeout", err) + } +} From 877e40858abf75ac8ffa19b3a11ef58081d87e9d Mon Sep 17 00:00:00 2001 From: Kush Date: Thu, 13 Aug 2026 15:10:37 -0400 Subject: [PATCH 06/21] fix(cmdutil): drop dead timer-drain idiom in Poll, fix vacuous test assertion Go 1.23+ made timer channels unbuffered; Stop() cancels any in-flight send, so there is never anything left in the channel to drain after Stop() returns. The old drain-after-Stop idiom was dead code on this toolchain and would deadlock if it were ever reached. Create a fresh timer per iteration instead and only Stop() it on the cancellation path as a courtesy; unreferenced timers are garbage collected. Also fix TestPollRespectsContextCancellation: the calls == 0 assertion could never fail since Check always runs at least once before context cancellation can be observed. Assert calls > 1 instead, which actually exercises loop iteration before interruption. --- internal/cmdutil/poll.go | 18 +++++++++--------- internal/cmdutil/poll_test.go | 9 +++++++-- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/internal/cmdutil/poll.go b/internal/cmdutil/poll.go index f779507..78ba7ef 100644 --- a/internal/cmdutil/poll.go +++ b/internal/cmdutil/poll.go @@ -34,11 +34,6 @@ func Poll(cfg PollConfig) (interface{}, error) { ctx = context.Background() } deadline := time.Now().Add(cfg.Timeout) - timer := time.NewTimer(0) - if !timer.Stop() { - <-timer.C - } - defer timer.Stop() for { done, result, err := cfg.Check() @@ -51,12 +46,17 @@ func Poll(cfg PollConfig) (interface{}, error) { if time.Now().After(deadline) { return nil, fmt.Errorf("timed out after %s: %w", cfg.Timeout, ErrPollTimeout) } - timer.Reset(cfg.Interval) + // A fresh timer per iteration. Go 1.23+ made timer channels + // unbuffered and Stop() cancels any in-flight send, so there is + // nothing left to drain after Stop() returns — draining an + // already-stopped timer's channel would just block forever. + // Stop() on the cancellation path is a courtesy; an abandoned + // timer is garbage collected once unreferenced, so there is no + // leak either way. + timer := time.NewTimer(cfg.Interval) select { case <-ctx.Done(): - if !timer.Stop() { - <-timer.C - } + timer.Stop() return nil, ctx.Err() case <-timer.C: } diff --git a/internal/cmdutil/poll_test.go b/internal/cmdutil/poll_test.go index bc954ad..1a79f28 100644 --- a/internal/cmdutil/poll_test.go +++ b/internal/cmdutil/poll_test.go @@ -67,8 +67,13 @@ func TestPollRespectsContextCancellation(t *testing.T) { if !errors.Is(err, context.Canceled) { t.Fatalf("err = %v, want context.Canceled", err) } - if calls == 0 { - t.Error("Check was never called") + // Poll always calls Check at least once before it can observe + // cancellation, so calls == 0 could never fail here. Assert instead + // that the loop actually iterated more than once — the interval + // (10ms) is well under the 30ms cancellation delay, so a working + // loop calls Check several times before ctx.Done() is observed. + if calls < 2 { + t.Errorf("calls = %d, want > 1 (loop should iterate before cancellation)", calls) } } From 31fb4a183ae25fa57f75fb257ecde801a0ea8830 Mon Sep 17 00:00:00 2001 From: Kush Date: Thu, 13 Aug 2026 15:13:48 -0400 Subject: [PATCH 07/21] feat(tendlc): add deepObject query encoding for list filters --- internal/tendlc/filter.go | 58 ++++++++++++++++++++++++++++++++++ internal/tendlc/filter_test.go | 58 ++++++++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+) create mode 100644 internal/tendlc/filter.go create mode 100644 internal/tendlc/filter_test.go diff --git a/internal/tendlc/filter.go b/internal/tendlc/filter.go new file mode 100644 index 0000000..d5084ee --- /dev/null +++ b/internal/tendlc/filter.go @@ -0,0 +1,58 @@ +// Package tendlc wraps Bandwidth's v2 A2P Campaign Management (10DLC +// Registration Center) API. +// +// Responses are returned as map[string]any rather than typed structs: +// production returns fields absent from the published spec, and +// encoding/json silently drops unknown fields when decoding into a struct. +// Requests are typed; responses are lossless. +package tendlc + +import ( + "net/url" + "strconv" +) + +// Filter operators. The API uses OpenAPI deepObject encoding, so a filter is +// sent as field[op]=value. Sending the bare field=value form is accepted by +// the server and then IGNORED — the result looks successful but is unfiltered. +const ( + OpEq = "eq" + OpContains = "contains" +) + +// Filter is one deepObject query filter. +type Filter struct { + Field string + Op string + Value string +} + +// EncodeQuery builds the query string for a list endpoint. Returns "" when +// there is nothing to send. Parameters are sorted so output is deterministic +// and testable. Filters with an empty Value are omitted entirely rather than +// sent blank, which the API treats as a match-nothing filter. +func EncodeQuery(limit, offset int, filters []Filter) string { + v := url.Values{} + if limit > 0 { + v.Set("limit", strconv.Itoa(limit)) + } + if offset > 0 { + v.Set("offset", strconv.Itoa(offset)) + } + for _, f := range filters { + if f.Field == "" || f.Value == "" { + continue + } + op := f.Op + if op == "" { + op = OpEq + } + v.Set(f.Field+"["+op+"]", f.Value) + } + if len(v) == 0 { + return "" + } + // url.Values.Encode sorts by key, so output is deterministic and the + // exact-string tests above are stable. + return "?" + v.Encode() +} diff --git a/internal/tendlc/filter_test.go b/internal/tendlc/filter_test.go new file mode 100644 index 0000000..c98a8b6 --- /dev/null +++ b/internal/tendlc/filter_test.go @@ -0,0 +1,58 @@ +package tendlc + +import "testing" + +func TestEncodeQueryDeepObjectForm(t *testing.T) { + tests := []struct { + name string + limit int + offset int + filters []Filter + want string + }{ + { + name: "no params", want: "", + }, + { + name: "limit and offset only", limit: 50, offset: 100, + want: "?limit=50&offset=100", + }, + { + name: "eq filter uses bracket form", + limit: 10, + filters: []Filter{{Field: "status", Op: OpEq, Value: "REGISTERED"}}, + want: "?limit=10&status%5Beq%5D=REGISTERED", + }, + { + name: "contains filter", + filters: []Filter{{Field: "brandId", Op: OpContains, Value: "B0I"}}, + want: "?brandId%5Bcontains%5D=B0I", + }, + { + name: "filters sort by field for determinism", + limit: 5, + filters: []Filter{ + {Field: "vettingStatus", Op: OpEq, Value: "APPROVED"}, + {Field: "brandId", Op: OpEq, Value: "B53K4I0"}, + }, + want: "?brandId%5Beq%5D=B53K4I0&limit=5&vettingStatus%5Beq%5D=APPROVED", + }, + { + name: "empty value is omitted, not sent blank", + filters: []Filter{{Field: "status", Op: OpEq, Value: ""}}, + want: "", + }, + { + name: "value is URL-escaped", + filters: []Filter{{Field: "campaignName", Op: OpContains, Value: "Acme Corp&Co"}}, + want: "?campaignName%5Bcontains%5D=Acme+Corp%26Co", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := EncodeQuery(tt.limit, tt.offset, tt.filters); got != tt.want { + t.Errorf("EncodeQuery() =\n %q\nwant\n %q", got, tt.want) + } + }) + } +} From 8ed4b6e4345332f12440322117a4ce6e0c72a4e7 Mon Sep 17 00:00:00 2001 From: Kush Date: Thu, 13 Aug 2026 15:17:01 -0400 Subject: [PATCH 08/21] docs(tendlc): document duplicate filter behavior and add test coverage --- internal/tendlc/filter.go | 4 ++++ internal/tendlc/filter_test.go | 16 ++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/internal/tendlc/filter.go b/internal/tendlc/filter.go index d5084ee..09a57ef 100644 --- a/internal/tendlc/filter.go +++ b/internal/tendlc/filter.go @@ -31,6 +31,10 @@ type Filter struct { // there is nothing to send. Parameters are sorted so output is deterministic // and testable. Filters with an empty Value are omitted entirely rather than // sent blank, which the API treats as a match-nothing filter. +// +// Filters are keyed by Field+Op, so a later filter with the same Field and Op +// overwrites an earlier one. Callers needing multiple constraints on one field +// (e.g. createdDate[gte] and createdDate[lte]) must use distinct Ops. func EncodeQuery(limit, offset int, filters []Filter) string { v := url.Values{} if limit > 0 { diff --git a/internal/tendlc/filter_test.go b/internal/tendlc/filter_test.go index c98a8b6..53df061 100644 --- a/internal/tendlc/filter_test.go +++ b/internal/tendlc/filter_test.go @@ -47,6 +47,22 @@ func TestEncodeQueryDeepObjectForm(t *testing.T) { filters: []Filter{{Field: "campaignName", Op: OpContains, Value: "Acme Corp&Co"}}, want: "?campaignName%5Bcontains%5D=Acme+Corp%26Co", }, + { + name: "duplicate field+op: last value wins", + filters: []Filter{ + {Field: "status", Op: OpEq, Value: "PENDING"}, + {Field: "status", Op: OpEq, Value: "APPROVED"}, + }, + want: "?status%5Beq%5D=APPROVED", + }, + { + name: "same field, different ops: both kept", + filters: []Filter{ + {Field: "createdDate", Op: "gte", Value: "2024-01-01"}, + {Field: "createdDate", Op: "lte", Value: "2024-12-31"}, + }, + want: "?createdDate%5Bgte%5D=2024-01-01&createdDate%5Blte%5D=2024-12-31", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From a8add5f2e4c8c229c7013b20777d4dff8668c634 Mon Sep 17 00:00:00 2001 From: Kush Date: Thu, 13 Aug 2026 15:19:04 -0400 Subject: [PATCH 09/21] feat(tendlc): add response envelope and pagination parsing --- internal/tendlc/envelope.go | 68 +++++++++++++++++++++++++ internal/tendlc/envelope_test.go | 86 ++++++++++++++++++++++++++++++++ 2 files changed, 154 insertions(+) create mode 100644 internal/tendlc/envelope.go create mode 100644 internal/tendlc/envelope_test.go diff --git a/internal/tendlc/envelope.go b/internal/tendlc/envelope.go new file mode 100644 index 0000000..67a8553 --- /dev/null +++ b/internal/tendlc/envelope.go @@ -0,0 +1,68 @@ +package tendlc + +import ( + "encoding/json" + "fmt" +) + +// Page is the pagination block present on every v2 A2P response. +type Page struct { + Number int `json:"pageNumber"` + Size int `json:"pageSize"` + TotalElements int `json:"totalElements"` + TotalPages int `json:"totalPages"` +} + +// Truncated reports whether more records exist beyond the ones returned. +// Driven by totalElements rather than by observing a short page, because a +// full page is not evidence of more and a short page is not evidence of none. +func (p *Page) Truncated(returned int) bool { + if p == nil { + return false + } + return returned < p.TotalElements +} + +// Envelope is the {data, page, errors, links} wrapper the API returns. +// Data is kept as any because its shape varies per endpoint: an object for +// brand detail, an array for lists and history. The published spec says +// array in both cases; production disagrees, so callers state which they +// expect and get an error rather than a silent zero value if wrong. +type Envelope struct { + Data any + Page *Page +} + +// ParseEnvelope decodes a response body into an Envelope. +func ParseEnvelope(body []byte) (*Envelope, error) { + var raw struct { + Data any `json:"data"` + Page *Page `json:"page"` + } + if err := json.Unmarshal(body, &raw); err != nil { + return nil, fmt.Errorf("decoding response envelope: %w", err) + } + return &Envelope{Data: raw.Data, Page: raw.Page}, nil +} + +// List returns data as an array. Errors if the endpoint returned an object. +func (e *Envelope) List() ([]any, error) { + if e.Data == nil { + return []any{}, nil + } + arr, ok := e.Data.([]any) + if !ok { + return nil, fmt.Errorf("expected a list response, got %T", e.Data) + } + return arr, nil +} + +// Object returns data as a single resource. Errors if the endpoint returned +// an array. +func (e *Envelope) Object() (map[string]any, error) { + obj, ok := e.Data.(map[string]any) + if !ok { + return nil, fmt.Errorf("expected a single-resource response, got %T", e.Data) + } + return obj, nil +} diff --git a/internal/tendlc/envelope_test.go b/internal/tendlc/envelope_test.go new file mode 100644 index 0000000..e27e6e4 --- /dev/null +++ b/internal/tendlc/envelope_test.go @@ -0,0 +1,86 @@ +package tendlc + +import "testing" + +func TestParseEnvelopeArrayData(t *testing.T) { + body := []byte(`{"data":[{"brandId":"B1"},{"brandId":"B2"}], + "page":{"pageNumber":0,"pageSize":2,"totalElements":10,"totalPages":5}}`) + env, err := ParseEnvelope(body) + if err != nil { + t.Fatalf("ParseEnvelope: %v", err) + } + list, err := env.List() + if err != nil { + t.Fatalf("List: %v", err) + } + if len(list) != 2 { + t.Errorf("len(list) = %d, want 2", len(list)) + } + if env.Page == nil || env.Page.TotalElements != 10 || env.Page.TotalPages != 5 { + t.Errorf("Page = %+v, want TotalElements 10 / TotalPages 5", env.Page) + } +} + +// brand get returns data as an OBJECT, contradicting the published spec. +// Verified live on 9901287. Prod wins. +func TestParseEnvelopeObjectData(t *testing.T) { + body := []byte(`{"data":{"brandId":"B0IRNU4","universalEin":"US_562242657"}}`) + env, err := ParseEnvelope(body) + if err != nil { + t.Fatalf("ParseEnvelope: %v", err) + } + obj, err := env.Object() + if err != nil { + t.Fatalf("Object: %v", err) + } + if obj["brandId"] != "B0IRNU4" { + t.Errorf("brandId = %v, want B0IRNU4", obj["brandId"]) + } + // Undocumented field must survive — responses are lossless. + if obj["universalEin"] != "US_562242657" { + t.Errorf("universalEin was dropped: %v", obj["universalEin"]) + } +} + +func TestEnvelopeShapeMismatchIsAnError(t *testing.T) { + env, _ := ParseEnvelope([]byte(`{"data":{"brandId":"B1"}}`)) + if _, err := env.List(); err == nil { + t.Error("List() on object data should error, not silently return empty") + } + env2, _ := ParseEnvelope([]byte(`{"data":[]}`)) + if _, err := env2.Object(); err == nil { + t.Error("Object() on array data should error") + } +} + +func TestPageTruncated(t *testing.T) { + tests := []struct { + name string + page *Page + returned int + want bool + }{ + {"more pages remain", &Page{TotalElements: 10, TotalPages: 5, Size: 2}, 2, true}, + {"everything returned", &Page{TotalElements: 2, TotalPages: 1, Size: 50}, 2, false}, + {"empty result", &Page{TotalElements: 0, TotalPages: 0, Size: 50}, 0, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.page.Truncated(tt.returned); got != tt.want { + t.Errorf("Truncated(%d) = %v, want %v", tt.returned, got, tt.want) + } + }) + } +} + +// Missing page metadata must be detectable so callers can fail closed +// rather than silently emitting a partial list. +func TestParseEnvelopeMissingPageIsNil(t *testing.T) { + env, err := ParseEnvelope([]byte(`{"data":[]}`)) + if err != nil { + t.Fatalf("ParseEnvelope: %v", err) + } + if env.Page != nil { + t.Errorf("Page = %+v, want nil when absent", env.Page) + } +} From b4856f1f751fcf9681fad843745254c4ce2883ba Mon Sep 17 00:00:00 2001 From: Kush Date: Thu, 13 Aug 2026 15:38:49 -0400 Subject: [PATCH 10/21] fix(tendlc): fail closed on nil envelope data, clarify Truncated contract List() no longer treats null/absent data as an empty result: production sends data:[] for genuinely empty lists, so nil is a malformed response, not zero results, and must error like any other shape mismatch. Truncated's parameter is renamed to returnedSoFar and documented as the cumulative count across all pages walked, not the current page's length, to prevent downstream callers from misreading it on the last page of a paginated walk. Shape-mismatch errors now describe the JSON shape (array/object/null) instead of printing a Go type name. --- internal/tendlc/envelope.go | 41 ++++++++++++++++++++++++-------- internal/tendlc/envelope_test.go | 38 ++++++++++++++++++++++++----- 2 files changed, 63 insertions(+), 16 deletions(-) diff --git a/internal/tendlc/envelope.go b/internal/tendlc/envelope.go index 67a8553..7776082 100644 --- a/internal/tendlc/envelope.go +++ b/internal/tendlc/envelope.go @@ -13,14 +13,20 @@ type Page struct { TotalPages int `json:"totalPages"` } -// Truncated reports whether more records exist beyond the ones returned. +// Truncated reports whether more records exist beyond the ones retrieved so +// far. returnedSoFar must be the CUMULATIVE count of records collected across +// every page fetched up to this point — not the length of the current page. +// Passing a per-page count gives a correct answer on the first page and a +// wrong one on every subsequent page: e.g. on the last of 5 pages of 2 +// records each, a per-page count of 2 against TotalElements 10 would report +// "truncated" even though nothing remains. // Driven by totalElements rather than by observing a short page, because a // full page is not evidence of more and a short page is not evidence of none. -func (p *Page) Truncated(returned int) bool { +func (p *Page) Truncated(returnedSoFar int) bool { if p == nil { return false } - return returned < p.TotalElements + return returnedSoFar < p.TotalElements } // Envelope is the {data, page, errors, links} wrapper the API returns. @@ -45,24 +51,39 @@ func ParseEnvelope(body []byte) (*Envelope, error) { return &Envelope{Data: raw.Data, Page: raw.Page}, nil } -// List returns data as an array. Errors if the endpoint returned an object. +// List returns data as an array. Errors if the endpoint returned an object, +// or if data was null or absent — genuinely empty lists come back from the +// API as "data":[], so a nil data field is an anomaly, not an empty result, +// and must not be silently treated as one. func (e *Envelope) List() ([]any, error) { - if e.Data == nil { - return []any{}, nil - } arr, ok := e.Data.([]any) if !ok { - return nil, fmt.Errorf("expected a list response, got %T", e.Data) + return nil, fmt.Errorf("response data must be an array, got %s", dataShape(e.Data)) } return arr, nil } // Object returns data as a single resource. Errors if the endpoint returned -// an array. +// an array, or if data was null or absent. func (e *Envelope) Object() (map[string]any, error) { obj, ok := e.Data.(map[string]any) if !ok { - return nil, fmt.Errorf("expected a single-resource response, got %T", e.Data) + return nil, fmt.Errorf("response data must be an object, got %s", dataShape(e.Data)) } return obj, nil } + +// dataShape describes the JSON shape of data in terms a CLI user can act on, +// rather than a Go type name. +func dataShape(data any) string { + switch data.(type) { + case nil: + return "null" + case []any: + return "an array" + case map[string]any: + return "an object" + default: + return fmt.Sprintf("%T", data) + } +} diff --git a/internal/tendlc/envelope_test.go b/internal/tendlc/envelope_test.go index e27e6e4..b314c25 100644 --- a/internal/tendlc/envelope_test.go +++ b/internal/tendlc/envelope_test.go @@ -53,21 +53,47 @@ func TestEnvelopeShapeMismatchIsAnError(t *testing.T) { } } +// Data that is null, or a data key that is entirely absent, must fail +// closed rather than being read as "no results" — genuinely empty lists +// come back from the API as "data":[], so nil is anomalous, not normal. +func TestEnvelopeNilDataIsAnError(t *testing.T) { + env, err := ParseEnvelope([]byte(`{"data":null}`)) + if err != nil { + t.Fatalf("ParseEnvelope: %v", err) + } + if _, err := env.List(); err == nil { + t.Error("List() on explicit null data should error, not report an empty list") + } + + env2, err := ParseEnvelope([]byte(`{}`)) + if err != nil { + t.Fatalf("ParseEnvelope: %v", err) + } + if _, err := env2.List(); err == nil { + t.Error("List() on a body with no data key should error, not report an empty list") + } +} + func TestPageTruncated(t *testing.T) { tests := []struct { - name string - page *Page - returned int - want bool + name string + page *Page + returnedSoFar int + want bool }{ {"more pages remain", &Page{TotalElements: 10, TotalPages: 5, Size: 2}, 2, true}, {"everything returned", &Page{TotalElements: 2, TotalPages: 1, Size: 50}, 2, false}, {"empty result", &Page{TotalElements: 0, TotalPages: 0, Size: 50}, 0, false}, + // Cumulative count across all pages walked equals the total: nothing + // left, even though a naive per-page count would have said otherwise + // on earlier pages. + {"cumulative count reaches total on last page", &Page{TotalElements: 10, TotalPages: 5, Size: 2}, 10, false}, + {"nil page is never truncated", nil, 2, false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := tt.page.Truncated(tt.returned); got != tt.want { - t.Errorf("Truncated(%d) = %v, want %v", tt.returned, got, tt.want) + if got := tt.page.Truncated(tt.returnedSoFar); got != tt.want { + t.Errorf("Truncated(%d) = %v, want %v", tt.returnedSoFar, got, tt.want) } }) } From 33bb5ecf7b3ae4b7969c8826383069f39e8975af Mon Sep 17 00:00:00 2001 From: Kush Date: Thu, 13 Aug 2026 15:41:54 -0400 Subject: [PATCH 11/21] feat(tendlc): add service with brand and campaign read methods --- internal/tendlc/service.go | 62 ++++++++++++++ internal/tendlc/service_test.go | 146 ++++++++++++++++++++++++++++++++ 2 files changed, 208 insertions(+) create mode 100644 internal/tendlc/service.go create mode 100644 internal/tendlc/service_test.go diff --git a/internal/tendlc/service.go b/internal/tendlc/service.go new file mode 100644 index 0000000..e543575 --- /dev/null +++ b/internal/tendlc/service.go @@ -0,0 +1,62 @@ +package tendlc + +import ( + "fmt" + "net/url" + + "github.com/Bandwidth/cli/internal/api" +) + +// Service wraps the v2 A2P Campaign Management endpoints for one account. +type Service struct { + client *api.Client + accountID string +} + +// NewService returns a Service bound to an account. c must be a JSON client +// (see cmdutil.PlatformClient). +func NewService(c *api.Client, accountID string) *Service { + return &Service{client: c, accountID: accountID} +} + +func (s *Service) base() string { + return "/api/v2/accounts/" + url.PathEscape(s.accountID) + "/tendlc" +} + +// get issues a GET and parses the standard {data, page} envelope. +func (s *Service) get(path string) (*Envelope, error) { + raw, err := s.client.GetRaw(path) + if err != nil { + return nil, err + } + return ParseEnvelope(raw) +} + +// ListBrands returns the brands on the account. +func (s *Service) ListBrands(limit, offset int, filters []Filter) (*Envelope, error) { + return s.get(s.base() + "/brands" + EncodeQuery(limit, offset, filters)) +} + +// GetBrand returns one brand. The response wraps data as an object, not a +// single-element array — verified against production. +func (s *Service) GetBrand(brandID string) (*Envelope, error) { + if brandID == "" { + return nil, fmt.Errorf("brand ID is required") + } + return s.get(s.base() + "/brands/" + url.PathEscape(brandID)) +} + +// ListCampaigns returns the campaigns on the account. The list projection +// omits fields the campaign schema defines (imported, cspId, samples, +// messageFlow) — use GetCampaign when those are needed. +func (s *Service) ListCampaigns(limit, offset int, filters []Filter) (*Envelope, error) { + return s.get(s.base() + "/campaigns" + EncodeQuery(limit, offset, filters)) +} + +// GetCampaign returns one campaign, including the fields the list omits. +func (s *Service) GetCampaign(campaignID string) (*Envelope, error) { + if campaignID == "" { + return nil, fmt.Errorf("campaign ID is required") + } + return s.get(s.base() + "/campaigns/" + url.PathEscape(campaignID)) +} diff --git a/internal/tendlc/service_test.go b/internal/tendlc/service_test.go new file mode 100644 index 0000000..768f882 --- /dev/null +++ b/internal/tendlc/service_test.go @@ -0,0 +1,146 @@ +package tendlc + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/Bandwidth/cli/internal/api" +) + +func newTestService(t *testing.T, h http.HandlerFunc) (*Service, func()) { + t.Helper() + srv := httptest.NewServer(h) + return NewService(api.NewClientNoAuth(srv.URL), "9901287"), srv.Close +} + +func TestListBrandsBuildsPathAndQuery(t *testing.T) { + var gotPath, gotQuery string + svc, done := newTestService(t, func(w http.ResponseWriter, r *http.Request) { + gotPath, gotQuery = r.URL.Path, r.URL.RawQuery + _, _ = w.Write([]byte(`{"data":[],"page":{"totalElements":0}}`)) + }) + defer done() + + _, err := svc.ListBrands(25, 0, []Filter{{Field: "brandType", Op: OpEq, Value: "PUBLIC_PROFIT"}}) + if err != nil { + t.Fatalf("ListBrands: %v", err) + } + if want := "/api/v2/accounts/9901287/tendlc/brands"; gotPath != want { + t.Errorf("path = %q, want %q", gotPath, want) + } + if want := "brandType%5Beq%5D=PUBLIC_PROFIT&limit=25"; gotQuery != want { + t.Errorf("query = %q, want %q", gotQuery, want) + } +} + +func TestGetBrandReturnsObjectEnvelope(t *testing.T) { + svc, done := newTestService(t, func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`{"data":{"brandId":"B0IRNU4"}}`)) + }) + defer done() + + env, err := svc.GetBrand("B0IRNU4") + if err != nil { + t.Fatalf("GetBrand: %v", err) + } + obj, err := env.Object() + if err != nil { + t.Fatalf("Object: %v", err) + } + if obj["brandId"] != "B0IRNU4" { + t.Errorf("brandId = %v", obj["brandId"]) + } +} + +func TestGetBrandEscapesID(t *testing.T) { + var gotPath string + svc, done := newTestService(t, func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.EscapedPath() + _, _ = w.Write([]byte(`{"data":{}}`)) + }) + defer done() + + _, _ = svc.GetBrand("B/../evil") + if want := "/api/v2/accounts/9901287/tendlc/brands/B%2F..%2Fevil"; gotPath != want { + t.Errorf("escaped path = %q, want %q", gotPath, want) + } +} + +func TestServicePropagatesAPIError(t *testing.T) { + svc, done := newTestService(t, func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusForbidden) + _, _ = w.Write([]byte(`{"errors":[{"description":"does not have access rights"}]}`)) + }) + defer done() + + if _, err := svc.ListBrands(0, 0, nil); err == nil { + t.Fatal("expected an error for 403") + } +} + +func TestListCampaignsBuildsPathAndQuery(t *testing.T) { + var gotPath, gotQuery string + svc, done := newTestService(t, func(w http.ResponseWriter, r *http.Request) { + gotPath, gotQuery = r.URL.Path, r.URL.RawQuery + _, _ = w.Write([]byte(`{"data":[],"page":{"totalElements":0}}`)) + }) + defer done() + + _, err := svc.ListCampaigns(10, 5, nil) + if err != nil { + t.Fatalf("ListCampaigns: %v", err) + } + if want := "/api/v2/accounts/9901287/tendlc/campaigns"; gotPath != want { + t.Errorf("path = %q, want %q", gotPath, want) + } + if want := "limit=10&offset=5"; gotQuery != want { + t.Errorf("query = %q, want %q", gotQuery, want) + } +} + +func TestGetCampaignEscapesID(t *testing.T) { + var gotPath string + svc, done := newTestService(t, func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.EscapedPath() + _, _ = w.Write([]byte(`{"data":{"campaignId":"C123"}}`)) + }) + defer done() + + env, err := svc.GetCampaign("C/../evil") + if err != nil { + t.Fatalf("GetCampaign: %v", err) + } + if want := "/api/v2/accounts/9901287/tendlc/campaigns/C%2F..%2Fevil"; gotPath != want { + t.Errorf("escaped path = %q, want %q", gotPath, want) + } + obj, err := env.Object() + if err != nil { + t.Fatalf("Object: %v", err) + } + if obj["campaignId"] != "C123" { + t.Errorf("campaignId = %v", obj["campaignId"]) + } +} + +func TestGetBrandEmptyIDErrors(t *testing.T) { + svc, done := newTestService(t, func(w http.ResponseWriter, r *http.Request) { + t.Fatal("server should not be called for an empty brand ID") + }) + defer done() + + if _, err := svc.GetBrand(""); err == nil { + t.Fatal("expected an error for an empty brand ID") + } +} + +func TestGetCampaignEmptyIDErrors(t *testing.T) { + svc, done := newTestService(t, func(w http.ResponseWriter, r *http.Request) { + t.Fatal("server should not be called for an empty campaign ID") + }) + defer done() + + if _, err := svc.GetCampaign(""); err == nil { + t.Fatal("expected an error for an empty campaign ID") + } +} From 9850c02a659aa00a8e47588c1c4893188225215a Mon Sep 17 00:00:00 2001 From: Kush Date: Thu, 13 Aug 2026 15:45:36 -0400 Subject: [PATCH 12/21] test(tendlc): lock error type/status contract and cover account-ID escaping --- internal/tendlc/service_test.go | 41 +++++++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/internal/tendlc/service_test.go b/internal/tendlc/service_test.go index 768f882..a15686d 100644 --- a/internal/tendlc/service_test.go +++ b/internal/tendlc/service_test.go @@ -1,6 +1,7 @@ package tendlc import ( + "errors" "net/http" "net/http/httptest" "testing" @@ -9,9 +10,14 @@ import ( ) func newTestService(t *testing.T, h http.HandlerFunc) (*Service, func()) { + t.Helper() + return newTestServiceForAccount(t, "9901287", h) +} + +func newTestServiceForAccount(t *testing.T, accountID string, h http.HandlerFunc) (*Service, func()) { t.Helper() srv := httptest.NewServer(h) - return NewService(api.NewClientNoAuth(srv.URL), "9901287"), srv.Close + return NewService(api.NewClientNoAuth(srv.URL), accountID), srv.Close } func TestListBrandsBuildsPathAndQuery(t *testing.T) { @@ -67,6 +73,23 @@ func TestGetBrandEscapesID(t *testing.T) { } } +func TestListBrandsEscapesAccountID(t *testing.T) { + var gotPath string + svc, done := newTestServiceForAccount(t, "99/../../etc", func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.EscapedPath() + _, _ = w.Write([]byte(`{"data":[]}`)) + }) + defer done() + + _, err := svc.ListBrands(0, 0, nil) + if err != nil { + t.Fatalf("ListBrands: %v", err) + } + if want := "/api/v2/accounts/99%2F..%2F..%2Fetc/tendlc/brands"; gotPath != want { + t.Errorf("escaped path = %q, want %q", gotPath, want) + } +} + func TestServicePropagatesAPIError(t *testing.T) { svc, done := newTestService(t, func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusForbidden) @@ -74,9 +97,23 @@ func TestServicePropagatesAPIError(t *testing.T) { }) defer done() - if _, err := svc.ListBrands(0, 0, nil); err == nil { + _, err := svc.ListBrands(0, 0, nil) + if err == nil { t.Fatal("expected an error for 403") } + + // band tendlc status (Task 9) distinguishes "no Registration Center + // access" (403, a definite answer) from a transport failure by doing + // errors.As(err, &apiErr) and branching on apiErr.StatusCode. That + // contract only holds if *api.APIError survives this layer unwrapped + // and untyped-away — assert both, not just "an error happened". + var apiErr *api.APIError + if !errors.As(err, &apiErr) { + t.Fatalf("error type = %T, want *api.APIError (or something wrapping it)", err) + } + if apiErr.StatusCode != http.StatusForbidden { + t.Errorf("StatusCode = %d, want %d", apiErr.StatusCode, http.StatusForbidden) + } } func TestListCampaignsBuildsPathAndQuery(t *testing.T) { From 42ad330f5edacf23a906452c4fdd008f378e3859 Mon Sep 17 00:00:00 2001 From: Kush Date: Thu, 13 Aug 2026 15:48:26 -0400 Subject: [PATCH 13/21] feat(customerprofile): add service with list and get --- internal/customerprofile/service.go | 53 ++++++++ internal/customerprofile/service_test.go | 150 +++++++++++++++++++++++ 2 files changed, 203 insertions(+) create mode 100644 internal/customerprofile/service.go create mode 100644 internal/customerprofile/service_test.go diff --git a/internal/customerprofile/service.go b/internal/customerprofile/service.go new file mode 100644 index 0000000..b9409ef --- /dev/null +++ b/internal/customerprofile/service.go @@ -0,0 +1,53 @@ +// Package customerprofile wraps Bandwidth's Numbers v2 customer-profile API. +// +// Customer profiles are a prerequisite for 10DLC brand registration, and a +// profile backs EXACTLY ONE brand — reusing one fails with "cannot be +// assigned to another brand". This 1:1 constraint is undocumented in the +// published specs and was found by probing production. +package customerprofile + +import ( + "fmt" + "net/url" + + "github.com/Bandwidth/cli/internal/api" + "github.com/Bandwidth/cli/internal/tendlc" +) + +// Service wraps the customer-profile endpoints for one account. +type Service struct { + client *api.Client + accountID string +} + +// NewService returns a Service bound to an account. +func NewService(c *api.Client, accountID string) *Service { + return &Service{client: c, accountID: accountID} +} + +func (s *Service) base() string { + return "/api/v2/accounts/" + url.PathEscape(s.accountID) + "/customerProfiles" +} + +func (s *Service) get(path string) (*tendlc.Envelope, error) { + raw, err := s.client.GetRaw(path) + if err != nil { + return nil, err + } + return tendlc.ParseEnvelope(raw) +} + +// List returns customer profiles on the account. +func (s *Service) List(limit, offset int, filters []tendlc.Filter) (*tendlc.Envelope, error) { + return s.get(s.base() + tendlc.EncodeQuery(limit, offset, filters)) +} + +// Get returns one customer profile. Soft-deleted profiles are still +// returned individually, with a "deleted" flag set — check it before +// creating any association. +func (s *Service) Get(profileID string) (*tendlc.Envelope, error) { + if profileID == "" { + return nil, fmt.Errorf("customer profile ID is required") + } + return s.get(s.base() + "/" + url.PathEscape(profileID)) +} diff --git a/internal/customerprofile/service_test.go b/internal/customerprofile/service_test.go new file mode 100644 index 0000000..e9d480a --- /dev/null +++ b/internal/customerprofile/service_test.go @@ -0,0 +1,150 @@ +package customerprofile + +import ( + "errors" + "net/http" + "net/http/httptest" + "testing" + + "github.com/Bandwidth/cli/internal/api" + "github.com/Bandwidth/cli/internal/tendlc" +) + +func newTestService(t *testing.T, h http.HandlerFunc) (*Service, func()) { + t.Helper() + return newTestServiceForAccount(t, "9901287", h) +} + +func newTestServiceForAccount(t *testing.T, accountID string, h http.HandlerFunc) (*Service, func()) { + t.Helper() + srv := httptest.NewServer(h) + return NewService(api.NewClientNoAuth(srv.URL), accountID), srv.Close +} + +func TestListBuildsPathAndQuery(t *testing.T) { + var gotPath, gotQuery string + svc, done := newTestService(t, func(w http.ResponseWriter, r *http.Request) { + gotPath, gotQuery = r.URL.Path, r.URL.RawQuery + _, _ = w.Write([]byte(`{"data":[],"page":{"totalElements":0}}`)) + }) + defer done() + + _, err := svc.List(10, 0, nil) + if err != nil { + t.Fatalf("List: %v", err) + } + if want := "/api/v2/accounts/9901287/customerProfiles"; gotPath != want { + t.Errorf("path = %q, want %q", gotPath, want) + } + if want := "limit=10"; gotQuery != want { + t.Errorf("query = %q, want %q", gotQuery, want) + } +} + +func TestListWithFilterBuildsQuery(t *testing.T) { + var gotQuery string + svc, done := newTestService(t, func(w http.ResponseWriter, r *http.Request) { + gotQuery = r.URL.RawQuery + _, _ = w.Write([]byte(`{"data":[],"page":{"totalElements":0}}`)) + }) + defer done() + + _, err := svc.List(25, 5, []tendlc.Filter{{Field: "brandId", Op: tendlc.OpEq, Value: "B0IRNU4"}}) + if err != nil { + t.Fatalf("List: %v", err) + } + if want := "brandId%5Beq%5D=B0IRNU4&limit=25&offset=5"; gotQuery != want { + t.Errorf("query = %q, want %q", gotQuery, want) + } +} + +func TestListEscapesAccountID(t *testing.T) { + var gotPath string + svc, done := newTestServiceForAccount(t, "99/../../etc", func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.EscapedPath() + _, _ = w.Write([]byte(`{"data":[]}`)) + }) + defer done() + + _, err := svc.List(0, 0, nil) + if err != nil { + t.Fatalf("List: %v", err) + } + if want := "/api/v2/accounts/99%2F..%2F..%2Fetc/customerProfiles"; gotPath != want { + t.Errorf("escaped path = %q, want %q", gotPath, want) + } +} + +func TestGetReturnsObjectEnvelope(t *testing.T) { + svc, done := newTestService(t, func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`{"data":{"customerProfileId":"CP123"}}`)) + }) + defer done() + + env, err := svc.Get("CP123") + if err != nil { + t.Fatalf("Get: %v", err) + } + obj, err := env.Object() + if err != nil { + t.Fatalf("Object: %v", err) + } + if obj["customerProfileId"] != "CP123" { + t.Errorf("customerProfileId = %v", obj["customerProfileId"]) + } +} + +func TestGetEscapesID(t *testing.T) { + var gotPath string + svc, done := newTestService(t, func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.EscapedPath() + _, _ = w.Write([]byte(`{"data":{}}`)) + }) + defer done() + + _, _ = svc.Get("CP/../evil") + if want := "/api/v2/accounts/9901287/customerProfiles/CP%2F..%2Fevil"; gotPath != want { + t.Errorf("escaped path = %q, want %q", gotPath, want) + } +} + +// TestGetEmptyIDErrorsBeforeRequest asserts not just that Get("") errors, but +// that it does so WITHOUT making a request — the handler calls t.Fatal if +// hit, so this fails if the empty-ID guard is ever removed or short-circuited +// after the request is issued. +func TestGetEmptyIDErrorsBeforeRequest(t *testing.T) { + svc, done := newTestService(t, func(w http.ResponseWriter, r *http.Request) { + t.Fatal("server should not be called for an empty profile ID") + }) + defer done() + + if _, err := svc.Get(""); err == nil { + t.Fatal("expected an error for an empty profile ID") + } +} + +func TestServicePropagatesAPIError(t *testing.T) { + svc, done := newTestService(t, func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusForbidden) + _, _ = w.Write([]byte(`{"errors":[{"description":"does not have access rights"}]}`)) + }) + defer done() + + _, err := svc.List(0, 0, nil) + if err == nil { + t.Fatal("expected an error for 403") + } + + // Callers (e.g. band customer-profile status) need to branch on the HTTP + // status code, which requires *api.APIError to survive this layer + // unwrapped rather than being flattened into a plain error string. Assert + // the type and the status, not just "an error happened" — this fails if + // the service wraps with fmt.Errorf("%v", err) instead of "%w". + var apiErr *api.APIError + if !errors.As(err, &apiErr) { + t.Fatalf("error type = %T, want *api.APIError (or something wrapping it)", err) + } + if apiErr.StatusCode != http.StatusForbidden { + t.Errorf("StatusCode = %d, want %d", apiErr.StatusCode, http.StatusForbidden) + } +} From 2afd4a9cbbbbfa4dd4d03c72c0b99f347911a216 Mon Sep 17 00:00:00 2001 From: Kush Date: Thu, 13 Aug 2026 16:05:52 -0400 Subject: [PATCH 14/21] refactor(api): move response envelope and query encoding out of tendlc --- internal/api/client.go | 2 ++ internal/{tendlc => api}/envelope.go | 7 ++++++- internal/{tendlc => api}/envelope_test.go | 2 +- internal/{tendlc => api}/filter.go | 9 +-------- internal/{tendlc => api}/filter_test.go | 2 +- internal/customerprofile/service.go | 11 +++++------ internal/customerprofile/service_test.go | 3 +-- internal/tendlc/service.go | 18 ++++++++++-------- internal/tendlc/service_test.go | 2 +- 9 files changed, 28 insertions(+), 28 deletions(-) rename internal/{tendlc => api}/envelope.go (91%) rename internal/{tendlc => api}/envelope_test.go (99%) rename internal/{tendlc => api}/filter.go (79%) rename internal/{tendlc => api}/filter_test.go (99%) diff --git a/internal/api/client.go b/internal/api/client.go index 36ae00f..3fd8e99 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -1,3 +1,5 @@ +// Package api provides the shared HTTP client, response-envelope parsing, +// and query encoding used by Bandwidth's platform API packages. package api import ( diff --git a/internal/tendlc/envelope.go b/internal/api/envelope.go similarity index 91% rename from internal/tendlc/envelope.go rename to internal/api/envelope.go index 7776082..b8bddff 100644 --- a/internal/tendlc/envelope.go +++ b/internal/api/envelope.go @@ -1,4 +1,4 @@ -package tendlc +package api import ( "encoding/json" @@ -34,6 +34,11 @@ func (p *Page) Truncated(returnedSoFar int) bool { // brand detail, an array for lists and history. The published spec says // array in both cases; production disagrees, so callers state which they // expect and get an error rather than a silent zero value if wrong. +// +// Responses are returned as map[string]any rather than typed structs: +// production returns fields absent from the published spec, and +// encoding/json silently drops unknown fields when decoding into a struct. +// Requests are typed; responses are lossless. type Envelope struct { Data any Page *Page diff --git a/internal/tendlc/envelope_test.go b/internal/api/envelope_test.go similarity index 99% rename from internal/tendlc/envelope_test.go rename to internal/api/envelope_test.go index b314c25..04b1e49 100644 --- a/internal/tendlc/envelope_test.go +++ b/internal/api/envelope_test.go @@ -1,4 +1,4 @@ -package tendlc +package api import "testing" diff --git a/internal/tendlc/filter.go b/internal/api/filter.go similarity index 79% rename from internal/tendlc/filter.go rename to internal/api/filter.go index 09a57ef..e3847d2 100644 --- a/internal/tendlc/filter.go +++ b/internal/api/filter.go @@ -1,11 +1,4 @@ -// Package tendlc wraps Bandwidth's v2 A2P Campaign Management (10DLC -// Registration Center) API. -// -// Responses are returned as map[string]any rather than typed structs: -// production returns fields absent from the published spec, and -// encoding/json silently drops unknown fields when decoding into a struct. -// Requests are typed; responses are lossless. -package tendlc +package api import ( "net/url" diff --git a/internal/tendlc/filter_test.go b/internal/api/filter_test.go similarity index 99% rename from internal/tendlc/filter_test.go rename to internal/api/filter_test.go index 53df061..4e5296d 100644 --- a/internal/tendlc/filter_test.go +++ b/internal/api/filter_test.go @@ -1,4 +1,4 @@ -package tendlc +package api import "testing" diff --git a/internal/customerprofile/service.go b/internal/customerprofile/service.go index b9409ef..4dd2125 100644 --- a/internal/customerprofile/service.go +++ b/internal/customerprofile/service.go @@ -11,7 +11,6 @@ import ( "net/url" "github.com/Bandwidth/cli/internal/api" - "github.com/Bandwidth/cli/internal/tendlc" ) // Service wraps the customer-profile endpoints for one account. @@ -29,23 +28,23 @@ func (s *Service) base() string { return "/api/v2/accounts/" + url.PathEscape(s.accountID) + "/customerProfiles" } -func (s *Service) get(path string) (*tendlc.Envelope, error) { +func (s *Service) get(path string) (*api.Envelope, error) { raw, err := s.client.GetRaw(path) if err != nil { return nil, err } - return tendlc.ParseEnvelope(raw) + return api.ParseEnvelope(raw) } // List returns customer profiles on the account. -func (s *Service) List(limit, offset int, filters []tendlc.Filter) (*tendlc.Envelope, error) { - return s.get(s.base() + tendlc.EncodeQuery(limit, offset, filters)) +func (s *Service) List(limit, offset int, filters []api.Filter) (*api.Envelope, error) { + return s.get(s.base() + api.EncodeQuery(limit, offset, filters)) } // Get returns one customer profile. Soft-deleted profiles are still // returned individually, with a "deleted" flag set — check it before // creating any association. -func (s *Service) Get(profileID string) (*tendlc.Envelope, error) { +func (s *Service) Get(profileID string) (*api.Envelope, error) { if profileID == "" { return nil, fmt.Errorf("customer profile ID is required") } diff --git a/internal/customerprofile/service_test.go b/internal/customerprofile/service_test.go index e9d480a..82e83b1 100644 --- a/internal/customerprofile/service_test.go +++ b/internal/customerprofile/service_test.go @@ -7,7 +7,6 @@ import ( "testing" "github.com/Bandwidth/cli/internal/api" - "github.com/Bandwidth/cli/internal/tendlc" ) func newTestService(t *testing.T, h http.HandlerFunc) (*Service, func()) { @@ -49,7 +48,7 @@ func TestListWithFilterBuildsQuery(t *testing.T) { }) defer done() - _, err := svc.List(25, 5, []tendlc.Filter{{Field: "brandId", Op: tendlc.OpEq, Value: "B0IRNU4"}}) + _, err := svc.List(25, 5, []api.Filter{{Field: "brandId", Op: api.OpEq, Value: "B0IRNU4"}}) if err != nil { t.Fatalf("List: %v", err) } diff --git a/internal/tendlc/service.go b/internal/tendlc/service.go index e543575..49fc2d9 100644 --- a/internal/tendlc/service.go +++ b/internal/tendlc/service.go @@ -1,3 +1,5 @@ +// Package tendlc wraps Bandwidth's v2 A2P Campaign Management (10DLC +// Registration Center) API. package tendlc import ( @@ -24,22 +26,22 @@ func (s *Service) base() string { } // get issues a GET and parses the standard {data, page} envelope. -func (s *Service) get(path string) (*Envelope, error) { +func (s *Service) get(path string) (*api.Envelope, error) { raw, err := s.client.GetRaw(path) if err != nil { return nil, err } - return ParseEnvelope(raw) + return api.ParseEnvelope(raw) } // ListBrands returns the brands on the account. -func (s *Service) ListBrands(limit, offset int, filters []Filter) (*Envelope, error) { - return s.get(s.base() + "/brands" + EncodeQuery(limit, offset, filters)) +func (s *Service) ListBrands(limit, offset int, filters []api.Filter) (*api.Envelope, error) { + return s.get(s.base() + "/brands" + api.EncodeQuery(limit, offset, filters)) } // GetBrand returns one brand. The response wraps data as an object, not a // single-element array — verified against production. -func (s *Service) GetBrand(brandID string) (*Envelope, error) { +func (s *Service) GetBrand(brandID string) (*api.Envelope, error) { if brandID == "" { return nil, fmt.Errorf("brand ID is required") } @@ -49,12 +51,12 @@ func (s *Service) GetBrand(brandID string) (*Envelope, error) { // ListCampaigns returns the campaigns on the account. The list projection // omits fields the campaign schema defines (imported, cspId, samples, // messageFlow) — use GetCampaign when those are needed. -func (s *Service) ListCampaigns(limit, offset int, filters []Filter) (*Envelope, error) { - return s.get(s.base() + "/campaigns" + EncodeQuery(limit, offset, filters)) +func (s *Service) ListCampaigns(limit, offset int, filters []api.Filter) (*api.Envelope, error) { + return s.get(s.base() + "/campaigns" + api.EncodeQuery(limit, offset, filters)) } // GetCampaign returns one campaign, including the fields the list omits. -func (s *Service) GetCampaign(campaignID string) (*Envelope, error) { +func (s *Service) GetCampaign(campaignID string) (*api.Envelope, error) { if campaignID == "" { return nil, fmt.Errorf("campaign ID is required") } diff --git a/internal/tendlc/service_test.go b/internal/tendlc/service_test.go index a15686d..16b5c92 100644 --- a/internal/tendlc/service_test.go +++ b/internal/tendlc/service_test.go @@ -28,7 +28,7 @@ func TestListBrandsBuildsPathAndQuery(t *testing.T) { }) defer done() - _, err := svc.ListBrands(25, 0, []Filter{{Field: "brandType", Op: OpEq, Value: "PUBLIC_PROFIT"}}) + _, err := svc.ListBrands(25, 0, []api.Filter{{Field: "brandType", Op: api.OpEq, Value: "PUBLIC_PROFIT"}}) if err != nil { t.Fatalf("ListBrands: %v", err) } From 7b1adecf61c867eb143302f37bb9d9411a661bd5 Mon Sep 17 00:00:00 2001 From: Kush Date: Thu, 13 Aug 2026 16:11:05 -0400 Subject: [PATCH 15/21] feat(auth): add tendlc tri-state and customer_profiles capabilities --- cmd/auth/auth_test.go | 31 +++++++++++++++++++++++++++++++ cmd/auth/status.go | 39 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 68 insertions(+), 2 deletions(-) diff --git a/cmd/auth/auth_test.go b/cmd/auth/auth_test.go index 962503e..96bebd3 100644 --- a/cmd/auth/auth_test.go +++ b/cmd/auth/auth_test.go @@ -211,6 +211,37 @@ func TestHasRole(t *testing.T) { } } +func TestTenDLCCapability(t *testing.T) { + got := tendlcCapability(true) + if got["status"] != "unknown" || got["reason"] != "role_present_not_probed" { + t.Errorf("tendlcCapability(true) = %v, want unknown/role_present_not_probed", got) + } + got = tendlcCapability(false) + if got["status"] != "unavailable" || got["reason"] != "role_absent" { + t.Errorf("tendlcCapability(false) = %v, want unavailable/role_absent", got) + } +} + +// Customer Profiles Access is a distinct role from campaign_management — +// confirmed on a live credential that carries both. +func TestCustomerProfilesCapabilityIsSeparate(t *testing.T) { + caps := Capabilities([]string{"campaign_management"}) + if caps["customer_profiles"] { + t.Error("customer_profiles should be false without the Customer Profiles Access role") + } + if !caps["campaign_management"] { + t.Error("campaign_management should be true") + } + + caps = Capabilities([]string{"Customer Profiles Access"}) + if !caps["customer_profiles"] { + t.Error("customer_profiles should be true with the role") + } + if caps["campaign_management"] { + t.Error("campaign_management should be false without its own role") + } +} + // TestRunSwitch_PersistsTargetIntoActiveProfile guards against the bug where // switch only updated the legacy top-level cfg.AccountID, leaving the active // profile's AccountID stale — so subsequent commands continued targeting the diff --git a/cmd/auth/status.go b/cmd/auth/status.go index a1c5be0..050408a 100644 --- a/cmd/auth/status.go +++ b/cmd/auth/status.go @@ -42,8 +42,11 @@ type statusJSON struct { // SIP reports SIP provisioning availability as a tri-state object // ({"status":..., "reason":...}) rather than a bool inside Capabilities — // see sipCapability. - SIP map[string]string `json:"sip,omitempty"` - Error string `json:"error,omitempty"` + SIP map[string]string `json:"sip,omitempty"` + // TenDLC reports Registration Center availability as a tri-state, for the + // same reason as SIP — see tendlcCapability. + TenDLC map[string]string `json:"tendlc,omitempty"` + Error string `json:"error,omitempty"` } func runStatus(cmd *cobra.Command, args []string) error { @@ -93,6 +96,7 @@ func runStatus(cmd *cobra.Command, args []string) error { Roles: p.Roles, Capabilities: Capabilities(p.Roles), SIP: sipCapability(hasRole(p.Roles, "sip credentials")), + TenDLC: tendlcCapability(hasRole(p.Roles, "campaign_management")), } if keychainErr != nil { out.Error = "credentials not found in keychain" @@ -128,6 +132,7 @@ func runStatus(cmd *cobra.Command, args []string) error { fmt.Printf("Capable of: %s\n", capabilitySummary(Capabilities(p.Roles))) } fmt.Printf("SIP: %s\n", sipSummary(sipCapability(hasRole(p.Roles, "sip credentials")))) + fmt.Printf("10DLC: %s\n", tendlcSummary(tendlcCapability(hasRole(p.Roles, "campaign_management")))) if env != "prod" || cfg.HasMultipleEnvironments() { fmt.Printf("Environment: %s\n", env) } @@ -156,6 +161,7 @@ func Capabilities(roles []string) map[string]bool { "numbers": false, "vcp": false, "campaign_management": false, + "customer_profiles": false, "tfv": false, } for _, r := range roles { @@ -178,6 +184,9 @@ func Capabilities(roles []string) map[string]bool { if strings.Contains(rl, "campaign") { caps["campaign_management"] = true } + if strings.Contains(rl, "customer profiles") { + caps["customer_profiles"] = true + } if strings.Contains(rl, "tfv") || strings.Contains(rl, "toll-free") || strings.Contains(rl, "tollfree") { caps["tfv"] = true } @@ -224,6 +233,32 @@ func sipSummary(sip map[string]string) string { } } +// tendlcCapability reports 10DLC Registration Center availability as a +// tri-state. Access needs both the Campaign Management role and the +// account-level Registration Center feature; only the role is knowable +// offline, so a boolean would over-promise. Mirrors sipCapability. +// +// Note this is deliberately separate from the campaign_management boolean, +// which keeps its existing meaning: "the credential holds the role." +func tendlcCapability(hasRole bool) map[string]string { + if !hasRole { + return map[string]string{"status": "unavailable", "reason": "role_absent"} + } + return map[string]string{"status": "unknown", "reason": "role_present_not_probed"} +} + +// tendlcSummary renders the offline tri-state for human-readable output. +func tendlcSummary(t map[string]string) string { + switch t["reason"] { + case "role_absent": + return ui.Muted("not available (missing Campaign Management role)") + case "role_present_not_probed": + return ui.Muted("unknown — run 'band tendlc status' to check") + default: + return t["status"] + } +} + // capabilitySummary renders a capability map as a "have / not" line // for the human-readable auth status output on Build accounts. func capabilitySummary(caps map[string]bool) string { From c0550544c4182b931c3d6a1e473072e03c1e07e7 Mon Sep 17 00:00:00 2001 From: Kush Date: Thu, 13 Aug 2026 16:13:55 -0400 Subject: [PATCH 16/21] test(auth): cover tendlc/customer_profiles wiring against live role strings --- cmd/auth/auth_test.go | 75 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/cmd/auth/auth_test.go b/cmd/auth/auth_test.go index 96bebd3..74a0308 100644 --- a/cmd/auth/auth_test.go +++ b/cmd/auth/auth_test.go @@ -242,6 +242,81 @@ func TestCustomerProfilesCapabilityIsSeparate(t *testing.T) { } } +// liveAccount9901287Roles is the (trimmed) role list observed on a live +// credential for account 9901287 — captured directly rather than invented, so +// tests exercise the tri-state wiring against a realistic role string, not +// just an idealized one. In particular it confirms the role is genuinely +// snake_case "campaign_management" (not the "Campaign Management" display +// string used elsewhere for error messages), and it includes a decoy — +// "specialized customer external tns" — that contains the word "customer" but +// is not a Customer Profiles Access role. +var liveAccount9901287Roles = []string{ + "Alerting Insights", "Analytics", "Billing Reports", + "campaign_management", "Configuration", "Customer Profiles Access", + "Disconnect", "E911 Management", "specialized customer external tns", + "HTTP Application Management", "HttpVoice", "Line Features", + "Ordering", "Porting", "Reporting", "Short Code Access", +} + +// removeRole returns a copy of roles with every occurrence of target removed. +func removeRole(roles []string, target string) []string { + out := make([]string, 0, len(roles)) + for _, r := range roles { + if r != target { + out = append(out, r) + } + } + return out +} + +// TestTenDLCWiringAgainstLiveRoles exercises hasRole, tendlcCapability, and +// Capabilities together against the real role strings from +// liveAccount9901287Roles, rather than testing tendlcCapability(bool) in +// isolation. TestTenDLCCapability and TestCustomerProfilesCapabilityIsSeparate +// already cover the functions' own logic; this test guards the wiring itself +// — a future change to hasRole's matching (e.g. adding normalization that +// treats "_" and " " differently, or an accidental typo in the substring +// passed at the call site) could silently make campaign_management always +// evaluate to role_absent while every existing test still passes. +func TestTenDLCWiringAgainstLiveRoles(t *testing.T) { + if !hasRole(liveAccount9901287Roles, "campaign_management") { + t.Fatal("hasRole should find campaign_management in the live role list") + } + got := tendlcCapability(hasRole(liveAccount9901287Roles, "campaign_management")) + if got["status"] != "unknown" || got["reason"] != "role_present_not_probed" { + t.Errorf("tendlcCapability with live roles = %v, want unknown/role_present_not_probed", got) + } + if !Capabilities(liveAccount9901287Roles)["customer_profiles"] { + t.Error("customer_profiles should be true with live roles (Customer Profiles Access is present)") + } + + withoutCampaign := removeRole(liveAccount9901287Roles, "campaign_management") + if hasRole(withoutCampaign, "campaign_management") { + t.Fatal("hasRole should not find campaign_management once it's removed from the role list") + } + got = tendlcCapability(hasRole(withoutCampaign, "campaign_management")) + if got["status"] != "unavailable" || got["reason"] != "role_absent" { + t.Errorf("tendlcCapability without campaign_management = %v, want unavailable/role_absent", got) + } + + withoutCustomerProfiles := removeRole(liveAccount9901287Roles, "Customer Profiles Access") + if Capabilities(withoutCustomerProfiles)["customer_profiles"] { + t.Error("customer_profiles should be false once Customer Profiles Access is removed from the role list") + } +} + +// TestCustomerProfilesMatcherNotOverBroad guards against loosening the +// customer_profiles matcher from "customer profiles" to just "customer". +// "specialized customer external tns" is a real role from +// liveAccount9901287Roles that contains "customer" but has nothing to do with +// Customer Profiles Access — it must not flip customer_profiles to true. +func TestCustomerProfilesMatcherNotOverBroad(t *testing.T) { + caps := Capabilities([]string{"specialized customer external tns"}) + if caps["customer_profiles"] { + t.Error("customer_profiles should be false for 'specialized customer external tns' — it contains 'customer' but is not a Customer Profiles Access role") + } +} + // TestRunSwitch_PersistsTargetIntoActiveProfile guards against the bug where // switch only updated the legacy top-level cfg.AccountID, leaving the active // profile's AccountID stale — so subsequent commands continued targeting the From 78df52fe6be561a80378f3806b1098617cea41cc Mon Sep 17 00:00:00 2001 From: Kush Date: Thu, 13 Aug 2026 16:18:52 -0400 Subject: [PATCH 17/21] feat(tendlc): add status command probing Registration Center access --- cmd/tendlc/status.go | 91 +++++++++++++++++++++++++++++++++++++++ cmd/tendlc/tendlc_test.go | 54 +++++++++++++++++++++++ 2 files changed, 145 insertions(+) create mode 100644 cmd/tendlc/status.go diff --git a/cmd/tendlc/status.go b/cmd/tendlc/status.go new file mode 100644 index 0000000..e1ed08f --- /dev/null +++ b/cmd/tendlc/status.go @@ -0,0 +1,91 @@ +package tendlc + +import ( + "errors" + "strings" + + "github.com/spf13/cobra" + + "github.com/Bandwidth/cli/internal/api" + "github.com/Bandwidth/cli/internal/cmdutil" + "github.com/Bandwidth/cli/internal/output" + tendlcsvc "github.com/Bandwidth/cli/internal/tendlc" +) + +func init() { Cmd.AddCommand(statusCmd) } + +// modeUnknown is returned on every code path. Account mode (direct vs +// import) is a property of how the account is configured with Bandwidth, not +// a runtime-discoverable fact: brand.imported is true on direct AND import +// accounts, and campaign.imported requires an existing campaign plus a +// detail call. Reporting the field as explicitly unknown beats omitting it, +// which invites callers to assume a default. +func modeUnknown() map[string]string { + return map[string]string{"status": "unknown", "reason": "not_discoverable"} +} + +// statusResult maps a probe outcome onto the stable --plain shape. +func statusResult(statusCode int, body string) map[string]any { + res := map[string]any{"mode": modeUnknown()} + switch { + case statusCode >= 200 && statusCode < 300: + res["access"], res["reason"] = "available", "probe_succeeded" + case statusCode == 403 && strings.Contains(body, "not enabled for the Registration Center"): + res["access"], res["reason"] = "unavailable", "registration_center_not_enabled" + case statusCode == 403 && strings.Contains(body, "is not enabled on account"): + res["access"], res["reason"] = "unavailable", "campaign_management_not_enabled" + case statusCode == 403 && strings.Contains(body, "does not have access rights"): + res["access"], res["reason"] = "unavailable", "role_absent" + case statusCode == 403: + // A 403 we don't recognize is still a definite negative — the probe + // answered the question. Do not guess a specific cause. + res["access"], res["reason"] = "unavailable", "access_denied" + default: + res["access"], res["reason"] = "unknown", "probe_failed" + } + return res +} + +var statusCmd = &cobra.Command{ + Use: "status", + Short: "Check whether this account can use the 10DLC Registration Center", + Long: `Probes the Registration Center API to resolve the "unknown" capability reported +by 'band auth status'. Access requires both the Campaign Management role and the +account-level Registration Center feature; only the probe can confirm the latter. + +Reports access only. Account mode — whether you register campaigns directly or +import them from TCR — is not probed and cannot be discovered: an account is one +or the other, and that is a property of your Bandwidth setup. If you don't know +which yours is, ask your Bandwidth account contact rather than guessing.`, + Example: ` band tendlc status --plain`, + RunE: func(cmd *cobra.Command, args []string) error { + client, acctID, err := cmdutil.PlatformClient(cmdutil.AccountIDFlag(cmd)) + if err != nil { + return err + } + format, plain := cmdutil.OutputFlags(cmd) + svc := tendlcsvc.NewService(client, acctID) + + _, probeErr := svc.ListBrands(1, 0, nil) + if probeErr == nil { + return output.StdoutAuto(format, plain, statusResult(200, "")) + } + + var apiErr *api.APIError + if errors.As(probeErr, &apiErr) { + res := statusResult(apiErr.StatusCode, apiErr.Body) + // A 403 is a probe that succeeded in answering the question: + // this account cannot use the Registration Center. Exit 0 — the + // command did its job. Anything else is a failure to answer, so + // emit the "unknown" result for callers parsing stdout, then + // fall through to the normal non-zero error path. + if apiErr.StatusCode == 403 { + return output.StdoutAuto(format, plain, res) + } + if emitErr := output.StdoutAuto(format, plain, res); emitErr != nil { + return emitErr + } + } + return roleGateError(probeErr, "Campaign Management") + }, +} diff --git a/cmd/tendlc/tendlc_test.go b/cmd/tendlc/tendlc_test.go index da7ebf4..505dd19 100644 --- a/cmd/tendlc/tendlc_test.go +++ b/cmd/tendlc/tendlc_test.go @@ -220,6 +220,60 @@ func TestFilterNumbers(t *testing.T) { }) } +func TestStatusCommandRegistered(t *testing.T) { + c, _, err := Cmd.Find([]string{"status"}) + if err != nil || c.Name() != "status" { + t.Fatalf("Find(status) = %v, err %v; want the status command", c, err) + } +} + +func TestStatusResultShape(t *testing.T) { + tests := []struct { + name string + statusCode int + body string + wantAccess string + wantReason string + }{ + {"success", 200, `{"data":[]}`, "available", "probe_succeeded"}, + {"role missing", 403, `{"errors":[{"description":"does not have access rights"}]}`, + "unavailable", "role_absent"}, + {"registration center off", 403, `{"errors":[{"description":"not enabled for the Registration Center"}]}`, + "unavailable", "registration_center_not_enabled"}, + {"campaign mgmt off", 403, `{"errors":[{"description":"10DLC is not enabled on account"}]}`, + "unavailable", "campaign_management_not_enabled"}, + {"unrecognized 403", 403, `{"errors":[{"description":"something new"}]}`, + "unavailable", "access_denied"}, + {"server error", 503, `{}`, "unknown", "probe_failed"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := statusResult(tt.statusCode, tt.body) + if got["access"] != tt.wantAccess { + t.Errorf("access = %v, want %v", got["access"], tt.wantAccess) + } + if got["reason"] != tt.wantReason { + t.Errorf("reason = %v, want %v", got["reason"], tt.wantReason) + } + }) + } +} + +// mode is always present and always unknown — an omitted field invites +// callers to guess a default. +func TestStatusAlwaysReportsModeUnknown(t *testing.T) { + for _, code := range []int{200, 403, 503} { + got := statusResult(code, `{}`) + mode, ok := got["mode"].(map[string]string) + if !ok { + t.Fatalf("code %d: mode missing or wrong type: %#v", code, got["mode"]) + } + if mode["status"] != "unknown" || mode["reason"] != "not_discoverable" { + t.Errorf("code %d: mode = %v, want unknown/not_discoverable", code, mode) + } + } +} + func contains(s, sub string) bool { return len(s) >= len(sub) && (s == sub || len(s) > 0 && containsSubstring(s, sub)) } From 1a2f20d742575e81445350c2d5f25bd498afdacf Mon Sep 17 00:00:00 2001 From: Kush Date: Thu, 13 Aug 2026 16:26:51 -0400 Subject: [PATCH 18/21] fix(tendlc): always emit a stable status result on probe failure Transport errors (connection refused, DNS, TLS, timeout) never unwrap to *api.APIError, so the status command fell through to the error path without emitting anything on stdout. Every probeErr path now defaults to the unknown/probe_failed result before checking for a more specific outcome, so callers parsing stdout always get stable JSON regardless of how the probe failed. Adds command-level tests exercising RunE against stub servers, covering the success path, both 403 outcomes, a 5xx, and the transport-failure regression. --- cmd/tendlc/status.go | 46 +++++++-- cmd/tendlc/status_test.go | 198 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 235 insertions(+), 9 deletions(-) create mode 100644 cmd/tendlc/status_test.go diff --git a/cmd/tendlc/status.go b/cmd/tendlc/status.go index e1ed08f..4cd7cdb 100644 --- a/cmd/tendlc/status.go +++ b/cmd/tendlc/status.go @@ -14,6 +14,18 @@ import ( func init() { Cmd.AddCommand(statusCmd) } +// service builds a tendlc Service for the active account. It is a package +// var, not a plain func, so tests can substitute a service pointed at a stub +// server — the same seam pattern as cmd/sip's `service` and +// cmdutil.VoiceClient. +var service = func(cmd *cobra.Command) (*tendlcsvc.Service, error) { + client, acctID, err := cmdutil.PlatformClient(cmdutil.AccountIDFlag(cmd)) + if err != nil { + return nil, err + } + return tendlcsvc.NewService(client, acctID), nil +} + // modeUnknown is returned on every code path. Account mode (direct vs // import) is a property of how the account is configured with Bandwidth, not // a runtime-discoverable fact: brand.imported is true on direct AND import @@ -25,6 +37,14 @@ func modeUnknown() map[string]string { } // statusResult maps a probe outcome onto the stable --plain shape. +// +// The 403 branches are checked in a deliberate order — Registration Center, +// then campaign management, then role — because a body could in principle +// contain more than one of these substrings and the first match wins. This +// order reports the most fundamental blocker first: an account-level feature +// gap (Registration Center or campaign management not enabled) is a bigger +// blocker than a missing role on the credential, so it's surfaced ahead of +// role_absent. func statusResult(statusCode int, body string) map[string]any { res := map[string]any{"mode": modeUnknown()} switch { @@ -59,32 +79,40 @@ or the other, and that is a property of your Bandwidth setup. If you don't know which yours is, ask your Bandwidth account contact rather than guessing.`, Example: ` band tendlc status --plain`, RunE: func(cmd *cobra.Command, args []string) error { - client, acctID, err := cmdutil.PlatformClient(cmdutil.AccountIDFlag(cmd)) + svc, err := service(cmd) if err != nil { return err } format, plain := cmdutil.OutputFlags(cmd) - svc := tendlcsvc.NewService(client, acctID) _, probeErr := svc.ListBrands(1, 0, nil) if probeErr == nil { return output.StdoutAuto(format, plain, statusResult(200, "")) } + // Default to unknown/probe_failed so that EVERY failure path — a + // recognized 403, an unrecognized 403, a 429/5xx, or a bare transport + // error (DNS, connection refused, TLS, timeout, context cancellation) + // — emits a stable JSON document on stdout. A caller parsing stdout + // must never see an empty body just because the failure didn't happen + // to arrive wrapped in *api.APIError. + res := statusResult(0, "") var apiErr *api.APIError if errors.As(probeErr, &apiErr) { - res := statusResult(apiErr.StatusCode, apiErr.Body) + res = statusResult(apiErr.StatusCode, apiErr.Body) // A 403 is a probe that succeeded in answering the question: // this account cannot use the Registration Center. Exit 0 — the - // command did its job. Anything else is a failure to answer, so - // emit the "unknown" result for callers parsing stdout, then - // fall through to the normal non-zero error path. + // command did its job. if apiErr.StatusCode == 403 { return output.StdoutAuto(format, plain, res) } - if emitErr := output.StdoutAuto(format, plain, res); emitErr != nil { - return emitErr - } + } + // Anything else — 429, 5xx, or a transport error that never made it + // to an HTTP response — is a failure to answer, not an answer. Emit + // the result for callers parsing stdout, then fall through to the + // normal non-zero error path. + if emitErr := output.StdoutAuto(format, plain, res); emitErr != nil { + return emitErr } return roleGateError(probeErr, "Campaign Management") }, diff --git a/cmd/tendlc/status_test.go b/cmd/tendlc/status_test.go new file mode 100644 index 0000000..8ea1c9f --- /dev/null +++ b/cmd/tendlc/status_test.go @@ -0,0 +1,198 @@ +package tendlc + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/spf13/cobra" + + "github.com/Bandwidth/cli/internal/api" + tendlcsvc "github.com/Bandwidth/cli/internal/tendlc" + "github.com/Bandwidth/cli/internal/testutil" +) + +// withStubStatusService swaps the package-level `service` var for the +// duration of a test so `band tendlc status` hits a stub server instead of +// performing real auth — the same seam pattern cmd/sip's `service` var uses. +func withStubStatusService(t *testing.T, baseURL string) { + t.Helper() + orig := service + t.Cleanup(func() { service = orig }) + service = func(cmd *cobra.Command) (*tendlcsvc.Service, error) { + return tendlcsvc.NewService(api.NewClientNoAuth(baseURL), "9901287"), nil + } +} + +// statusStubServerWithCode returns a stub that answers GET .../brands (the +// cheap probe `band tendlc status` issues via ListBrands) with the given +// status code and body. +func statusStubServerWithCode(t *testing.T, code int, body string) *httptest.Server { + t.Helper() + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet || !strings.HasSuffix(r.URL.Path, "/brands") { + w.WriteHeader(404) + return + } + w.WriteHeader(code) + _, _ = w.Write([]byte(body)) + })) +} + +// runStatus executes `band tendlc status --plain` against whatever `service` +// currently resolves to, returning stdout and the command's error together — +// the pairing that matters for this command's exit-code contract. +func runStatus(t *testing.T) (string, error) { + t.Helper() + root := testutil.NewTestRoot(statusCmd) + root.SetArgs([]string{"status", "--plain"}) + + var err error + out := testutil.CaptureStdout(t, func() { + err = root.Execute() + }) + return out, err +} + +func decodeStatusOutput(t *testing.T, out string) map[string]interface{} { + t.Helper() + var got map[string]interface{} + if err := json.Unmarshal([]byte(out), &got); err != nil { + t.Fatalf("output is not JSON: %q (%v)", out, err) + } + return got +} + +func assertModeUnknown(t *testing.T, got map[string]interface{}) { + t.Helper() + mode, ok := got["mode"].(map[string]interface{}) + if !ok { + t.Fatalf("mode missing or wrong type: %#v", got["mode"]) + } + if mode["status"] != "unknown" || mode["reason"] != "not_discoverable" { + t.Errorf("mode = %v, want unknown/not_discoverable", mode) + } +} + +// TestTendlcStatus_ProbeSucceeded covers the 200 path end to end: RunE must +// return nil (exit 0) and stdout must carry access=available/probe_succeeded. +func TestTendlcStatus_ProbeSucceeded(t *testing.T) { + srv := statusStubServerWithCode(t, 200, `{"data":[]}`) + defer srv.Close() + withStubStatusService(t, srv.URL) + + out, err := runStatus(t) + if err != nil { + t.Fatalf("Execute() error = %v, want nil", err) + } + + got := decodeStatusOutput(t, out) + if got["access"] != "available" { + t.Errorf("access = %v, want %q", got["access"], "available") + } + if got["reason"] != "probe_succeeded" { + t.Errorf("reason = %v, want %q", got["reason"], "probe_succeeded") + } + assertModeUnknown(t, got) +} + +// TestTendlcStatus_RecognizedForbidden covers a documented 403: this is the +// rule most at risk of regression — a successful probe reporting a negative +// fact must still exit 0, not surface as a command failure. +func TestTendlcStatus_RecognizedForbidden(t *testing.T) { + srv := statusStubServerWithCode(t, 403, + `{"errors":[{"description":"client does not have access rights to the content"}]}`) + defer srv.Close() + withStubStatusService(t, srv.URL) + + out, err := runStatus(t) + if err != nil { + t.Fatalf("Execute() error = %v, want nil (a 403 probe result is not a command failure)", err) + } + + got := decodeStatusOutput(t, out) + if got["access"] != "unavailable" { + t.Errorf("access = %v, want %q", got["access"], "unavailable") + } + if got["reason"] != "role_absent" { + t.Errorf("reason = %v, want %q", got["reason"], "role_absent") + } + assertModeUnknown(t, got) +} + +// TestTendlcStatus_UnrecognizedForbidden covers a 403 body that doesn't match +// any of the three documented substrings: still a successful probe (exit 0), +// mapped to the access_denied fallback reason. +func TestTendlcStatus_UnrecognizedForbidden(t *testing.T) { + srv := statusStubServerWithCode(t, 403, `{"errors":[{"description":"something new"}]}`) + defer srv.Close() + withStubStatusService(t, srv.URL) + + out, err := runStatus(t) + if err != nil { + t.Fatalf("Execute() error = %v, want nil (a 403 probe result is not a command failure)", err) + } + + got := decodeStatusOutput(t, out) + if got["access"] != "unavailable" { + t.Errorf("access = %v, want %q", got["access"], "unavailable") + } + if got["reason"] != "access_denied" { + t.Errorf("reason = %v, want %q", got["reason"], "access_denied") + } + assertModeUnknown(t, got) +} + +// TestTendlcStatus_ServerError covers the probe_failed path for a 5xx: the +// probe's job IS to report Registration Center availability, so even though +// this exits non-zero, an agent branching on stable JSON fields still needs +// unknown/probe_failed on stdout rather than an empty body. +func TestTendlcStatus_ServerError(t *testing.T) { + srv := statusStubServerWithCode(t, 503, `{}`) + defer srv.Close() + withStubStatusService(t, srv.URL) + + out, err := runStatus(t) + if err == nil { + t.Fatal("Execute() error = nil, want a non-nil error for HTTP 503") + } + + got := decodeStatusOutput(t, out) + if got["access"] != "unknown" { + t.Errorf("access = %v, want %q", got["access"], "unknown") + } + if got["reason"] != "probe_failed" { + t.Errorf("reason = %v, want %q", got["reason"], "probe_failed") + } + assertModeUnknown(t, got) +} + +// TestTendlcStatus_TransportFailure is the regression lock for the bug where +// a bare transport error (never wrapped in *api.APIError) produced EMPTY +// stdout: RunE fell straight through to roleGateError without emitting +// anything. Closing the server before the request forces a connection +// refused, which api.Client wraps as a plain error, not *api.APIError. +func TestTendlcStatus_TransportFailure(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) + srv.Close() // closed before use: any request now fails at the transport layer + withStubStatusService(t, srv.URL) + + out, err := runStatus(t) + if err == nil { + t.Fatal("Execute() error = nil, want a non-nil error for a transport failure") + } + if strings.TrimSpace(out) == "" { + t.Fatal("stdout is empty; want a stable unknown/probe_failed JSON document even on transport failure") + } + + got := decodeStatusOutput(t, out) + if got["access"] != "unknown" { + t.Errorf("access = %v, want %q", got["access"], "unknown") + } + if got["reason"] != "probe_failed" { + t.Errorf("reason = %v, want %q", got["reason"], "probe_failed") + } + assertModeUnknown(t, got) +} From a246c8ce8c9246cafb419b518acca89c80fb42b9 Mon Sep 17 00:00:00 2001 From: Kush Date: Thu, 13 Aug 2026 16:32:12 -0400 Subject: [PATCH 19/21] docs(agents): document the 10DLC capability tri-state and status probe --- AGENTS.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index f6b13c3..f84db7b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -85,6 +85,28 @@ This issues one cheap `GET /realms` call. A `200` reports `available`/`probe_suc Important: `band sip status` **does not persist** its result anywhere. Run it again any time you need a fresh answer, and don't expect `band auth status` to start reporting anything other than `unknown` for a role-holding credential — that command stays fully offline by design. +#### 10DLC capability (tri-state, not boolean) + +`band auth status --plain` reports `tendlc` as `{"status":..., "reason":...}` rather than a +boolean, for the same reason as SIP: access needs both the Campaign Management role and the +account-level Registration Center feature, and only the role is knowable offline. + +- `unavailable` / `role_absent` — the credential lacks the Campaign Management role. +- `unknown` / `role_present_not_probed` — run `band tendlc status` to resolve. + +`campaign_management` remains a plain boolean meaning "the credential holds the role", and +`customer_profiles` is a separate capability — the Customer Profiles Access role is distinct. + +```bash +band tendlc status --plain +# → {"access":"available","mode":{"reason":"not_discoverable","status":"unknown"},"reason":"probe_succeeded"} +``` + +**`mode` is always `unknown`, by design.** An account either registers campaigns *directly* or +*imports* them from TCR — never both — and that is a property of the account's Bandwidth setup, +not something the API exposes. Do not infer it, and do not try one path to see what happens. +If direct-vs-import was not given to you in the task, stop and ask the operator. + ### Account Hint When multiple accounts or profiles are active, commands write a hint to stderr so you know which account is being targeted: From 810e198463e492a893527cdfb1a8b585c2f3a27b Mon Sep 17 00:00:00 2001 From: Kush Date: Thu, 13 Aug 2026 16:41:52 -0400 Subject: [PATCH 20/21] fix(tendlc): document status reason codes, guard empty flag error, correct stale comments --- AGENTS.md | 15 +++++++++++++++ cmd/tendlc/status.go | 10 ++++++++++ cmd/tendlc/status_test.go | 27 +++++++++++++++++++++++++++ internal/cmdutil/exitcodes.go | 11 +++++++++++ internal/cmdutil/exitcodes_test.go | 13 +++++++++++++ internal/customerprofile/service.go | 3 ++- 6 files changed, 78 insertions(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index f84db7b..eee6285 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -102,6 +102,21 @@ band tendlc status --plain # → {"access":"available","mode":{"reason":"not_discoverable","status":"unknown"},"reason":"probe_succeeded"} ``` +`band tendlc status` can emit any of these `reason` values. Every 403-derived result +exits **0** — the probe answered the question, even when the answer is negative — so +there is no stderr message to fall back on; this table is the only authoritative place +to learn what to do next. `probe_failed` is the sole exception: the probe couldn't +answer at all, so it exits non-zero and is the only reason worth retrying. + +| `reason` | `access` | Exit code | Next action | +|---|---|---|---| +| `probe_succeeded` | `available` | 0 | Registration Center access confirmed. Proceed with `band tendlc` commands. | +| `registration_center_not_enabled` | `unavailable` | 0 | Account doesn't have the Registration Center feature. Escalate to your Bandwidth account manager to enable it — do not retry. | +| `campaign_management_not_enabled` | `unavailable` | 0 | Campaign Management/messaging is not enabled on the account. Escalate to your Bandwidth account manager — do not retry. | +| `role_absent` | `unavailable` | 0 | The credential lacks the Campaign Management role. Have an account manager assign the role — do not retry; a retry hits the same 403. | +| `access_denied` | `unavailable` | 0 | A 403 that didn't match any recognized cause. Escalate to your Bandwidth account manager — do not retry. | +| `probe_failed` | `unknown` | non-zero | The probe itself failed (rate limited, 5xx, or a transport error) — it could not answer the question. This is the only reason where retrying makes sense. | + **`mode` is always `unknown`, by design.** An account either registers campaigns *directly* or *imports* them from TCR — never both — and that is a property of the account's Bandwidth setup, not something the API exposes. Do not infer it, and do not try one path to see what happens. diff --git a/cmd/tendlc/status.go b/cmd/tendlc/status.go index 4cd7cdb..db94c69 100644 --- a/cmd/tendlc/status.go +++ b/cmd/tendlc/status.go @@ -78,6 +78,16 @@ import them from TCR — is not probed and cannot be discovered: an account is o or the other, and that is a property of your Bandwidth setup. If you don't know which yours is, ask your Bandwidth account contact rather than guessing.`, Example: ` band tendlc status --plain`, + // Contract: probe success is coupled to envelope decoding. svc.ListBrands + // parses the response body before returning, so a 200 whose body is not + // valid JSON (or whose envelope shape has changed) surfaces as an error + // here exactly like a transport failure, and is reported as + // unknown/probe_failed rather than available/probe_succeeded. This is + // deliberate, not a gap to fix: the command cannot distinguish "access is + // fine but the payload changed" from a genuine failure, and reporting + // available off a response it could not actually read would be a false + // positive — worse than a false probe_failed, which just means "run it + // again." RunE: func(cmd *cobra.Command, args []string) error { svc, err := service(cmd) if err != nil { diff --git a/cmd/tendlc/status_test.go b/cmd/tendlc/status_test.go index 8ea1c9f..7c000f9 100644 --- a/cmd/tendlc/status_test.go +++ b/cmd/tendlc/status_test.go @@ -169,6 +169,33 @@ func TestTendlcStatus_ServerError(t *testing.T) { assertModeUnknown(t, got) } +// TestTendlcStatus_MalformedSuccessBody locks in the documented coupling +// between probe success and envelope decoding: svc.ListBrands parses the +// response body before returning, so a 200 whose body is not valid JSON +// cannot be distinguished from a genuine failure. The command must not +// report available/probe_succeeded off a body it could not read — it reports +// unknown/probe_failed and returns a non-nil error, same as any other +// failure to answer. +func TestTendlcStatus_MalformedSuccessBody(t *testing.T) { + srv := statusStubServerWithCode(t, 200, `not json at all`) + defer srv.Close() + withStubStatusService(t, srv.URL) + + out, err := runStatus(t) + if err == nil { + t.Fatal("Execute() error = nil, want a non-nil error for an undecodable 200 body") + } + + got := decodeStatusOutput(t, out) + if got["access"] != "unknown" { + t.Errorf("access = %v, want %q", got["access"], "unknown") + } + if got["reason"] != "probe_failed" { + t.Errorf("reason = %v, want %q", got["reason"], "probe_failed") + } + assertModeUnknown(t, got) +} + // TestTendlcStatus_TransportFailure is the regression lock for the bug where // a bare transport error (never wrapped in *api.APIError) produced EMPTY // stdout: RunE fell straight through to roleGateError without emitting diff --git a/internal/cmdutil/exitcodes.go b/internal/cmdutil/exitcodes.go index 1da022e..d4baf7a 100644 --- a/internal/cmdutil/exitcodes.go +++ b/internal/cmdutil/exitcodes.go @@ -43,7 +43,15 @@ func NewFlagError(msg string) error { return &FlagError{Message: msg} } // NewMissingFlagsError reports every missing required flag in one error, // sorted for determinism. Cobra's MarkFlagRequired is not used on commands // with conditional requirements, so aggregation happens here instead. +// +// A nil or empty names is a caller bug — there is no missing flag to report — +// but this must still produce a sensible message rather than the malformed +// "missing required flags: " (trailing separator, no names) that naive +// joining would emit. func NewMissingFlagsError(names []string) error { + if len(names) == 0 { + return &FlagError{Message: "missing required flags"} + } sorted := append([]string(nil), names...) sort.Strings(sorted) for i, n := range sorted { @@ -72,6 +80,9 @@ func (e *ConflictError) Error() string { return e.Message } func (e *ConflictError) Unwrap() error { return e.Cause } // ExitCodeForError maps an error to the appropriate exit code. +// FlagError takes precedence over everything else, including a wrapped API +// error: it is a client-side validation failure and no HTTP request was ever +// made, so it must win regardless of what else the error chain contains. // FeatureLimitError takes precedence over the raw API status code so a // 403 caused by a plan/role limit maps to ExitConflict (4) rather than // ExitAuth (2) — agents can then distinguish "stop, escalate" from diff --git a/internal/cmdutil/exitcodes_test.go b/internal/cmdutil/exitcodes_test.go index 5661d29..ac93f22 100644 --- a/internal/cmdutil/exitcodes_test.go +++ b/internal/cmdutil/exitcodes_test.go @@ -133,6 +133,19 @@ func TestMissingFlagsErrorListsAllNames(t *testing.T) { } } +// An empty or nil names slice must not produce the malformed +// "missing required flags: " (trailing separator, no names). Locks the +// guard added for this case. +func TestMissingFlagsErrorEmptyNames(t *testing.T) { + want := "missing required flags" + if got := cmdutil.NewMissingFlagsError(nil).Error(); got != want { + t.Errorf("NewMissingFlagsError(nil).Error() = %q, want %q", got, want) + } + if got := cmdutil.NewMissingFlagsError([]string{}).Error(); got != want { + t.Errorf("NewMissingFlagsError([]string{}).Error() = %q, want %q", got, want) + } +} + // A FlagError must win over a wrapped APIError: it is a client-side // failure and no request was ever sent. func TestFlagErrorTakesPrecedenceOverAPIError(t *testing.T) { diff --git a/internal/customerprofile/service.go b/internal/customerprofile/service.go index 4dd2125..3d40cec 100644 --- a/internal/customerprofile/service.go +++ b/internal/customerprofile/service.go @@ -19,7 +19,8 @@ type Service struct { accountID string } -// NewService returns a Service bound to an account. +// NewService returns a Service bound to an account. c must be a JSON client +// (see cmdutil.PlatformClient). func NewService(c *api.Client, accountID string) *Service { return &Service{client: c, accountID: accountID} } From 3058feb7a651758a72baea4a46d1610ce809e37a Mon Sep 17 00:00:00 2001 From: Kush Date: Fri, 14 Aug 2026 09:34:19 -0400 Subject: [PATCH 21/21] fix(auth): derive tendlc tri-state from the campaign_management capability --- cmd/auth/auth_test.go | 73 +++++++++++++++++++++++++++++++++++++ cmd/auth/status.go | 16 +++++--- internal/api/envelope.go | 4 +- internal/api/filter.go | 9 +++++ internal/api/filter_test.go | 6 +-- 5 files changed, 98 insertions(+), 10 deletions(-) diff --git a/cmd/auth/auth_test.go b/cmd/auth/auth_test.go index 74a0308..cd995b0 100644 --- a/cmd/auth/auth_test.go +++ b/cmd/auth/auth_test.go @@ -6,7 +6,10 @@ import ( "path/filepath" "testing" + "github.com/spf13/cobra" + "github.com/Bandwidth/cli/internal/config" + "github.com/Bandwidth/cli/internal/testutil" ) func TestCmdStructure(t *testing.T) { @@ -305,6 +308,76 @@ func TestTenDLCWiringAgainstLiveRoles(t *testing.T) { } } +// TestStatusPlainTenDLCAgreesWithCapabilities is an end-to-end regression +// test on `band auth status --plain`'s actual JSON output, not just on the +// helper functions in isolation. The bug lived in the call-site wiring +// inside runStatus (status.go), which fed tendlcCapability a *different* +// signal — hasRole(p.Roles, "campaign_management"), a plain substring match +// on the unnormalized snake_case string — than the one behind +// capabilities.campaign_management, which is Contains(rl, "campaign") over +// the same roles. For the display-form role "Campaign Management" those two +// signals disagree: the boolean matches (it contains "campaign") but the old +// hasRole lookup does not (no role contains the literal substring +// "campaign_management" with an underscore). That produced a single JSON +// document asserting both capabilities.campaign_management=true and +// tendlc={"status":"unavailable","reason":"role_absent"} for the same +// credential. Testing tendlcCapability(caps["campaign_management"]) directly +// would trivially pass by construction regardless of what runStatus itself +// does; only driving runStatus end to end (as done here) actually exercises +// the wiring and would have failed before the fix. +func TestStatusPlainTenDLCAgreesWithCapabilities(t *testing.T) { + tests := []struct { + name string + roles []string + }{ + {name: "snake_case role as seen on live account 9901287", roles: []string{"campaign_management"}}, + {name: "display-form role", roles: []string{"Campaign Management"}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, ".config")) + + cfgPath, err := config.DefaultPath() + if err != nil { + t.Fatal(err) + } + cfg := &config.Config{Format: "json"} + cfg.SetProfile("default", &config.Profile{ + ClientID: "id1", + AccountID: "ACCT_A", + Roles: tt.roles, + }) + if err := config.Save(cfgPath, cfg); err != nil { + t.Fatal(err) + } + + wrap := &cobra.Command{Use: "status", RunE: runStatus} + root := testutil.NewTestRoot(wrap) + root.SetArgs([]string{"status", "--plain"}) + + out := testutil.CaptureStdout(t, func() { + if err := root.Execute(); err != nil { + t.Fatalf("Execute() error = %v", err) + } + }) + + var got statusJSON + if err := json.Unmarshal([]byte(out), &got); err != nil { + t.Fatalf("unmarshal output: %v\noutput: %s", err, out) + } + + if !got.Capabilities["campaign_management"] { + t.Fatalf("capabilities.campaign_management = false for roles %v, want true", tt.roles) + } + if got.TenDLC["reason"] == "role_absent" { + t.Errorf("capabilities.campaign_management = true but tendlc = %v — the two must agree in one JSON document", got.TenDLC) + } + }) + } +} + // TestCustomerProfilesMatcherNotOverBroad guards against loosening the // customer_profiles matcher from "customer profiles" to just "customer". // "specialized customer external tns" is a real role from diff --git a/cmd/auth/status.go b/cmd/auth/status.go index 050408a..019c918 100644 --- a/cmd/auth/status.go +++ b/cmd/auth/status.go @@ -85,6 +85,7 @@ func runStatus(cmd *cobra.Command, args []string) error { _, keychainErr := intauth.GetPassword(p.ClientID) if plain { + caps := Capabilities(p.Roles) out := statusJSON{ Authenticated: keychainErr == nil, Profile: profileName, @@ -94,9 +95,9 @@ func runStatus(cmd *cobra.Command, args []string) error { Environment: env, Build: p.Build, Roles: p.Roles, - Capabilities: Capabilities(p.Roles), + Capabilities: caps, SIP: sipCapability(hasRole(p.Roles, "sip credentials")), - TenDLC: tendlcCapability(hasRole(p.Roles, "campaign_management")), + TenDLC: tendlcCapability(caps["campaign_management"]), } if keychainErr != nil { out.Error = "credentials not found in keychain" @@ -127,12 +128,13 @@ func runStatus(cmd *cobra.Command, args []string) error { } else if len(p.Accounts) == 0 && p.AccountID == "" { fmt.Println("Scope: system-wide (use --account-id to target an account)") } + caps := Capabilities(p.Roles) if p.Build { fmt.Printf("Type: %s (voice-only, credit-based)\n", ui.Bold("Bandwidth Build")) - fmt.Printf("Capable of: %s\n", capabilitySummary(Capabilities(p.Roles))) + fmt.Printf("Capable of: %s\n", capabilitySummary(caps)) } fmt.Printf("SIP: %s\n", sipSummary(sipCapability(hasRole(p.Roles, "sip credentials")))) - fmt.Printf("10DLC: %s\n", tendlcSummary(tendlcCapability(hasRole(p.Roles, "campaign_management")))) + fmt.Printf("10DLC: %s\n", tendlcSummary(tendlcCapability(caps["campaign_management"]))) if env != "prod" || cfg.HasMultipleEnvironments() { fmt.Printf("Environment: %s\n", env) } @@ -238,8 +240,10 @@ func sipSummary(sip map[string]string) string { // account-level Registration Center feature; only the role is knowable // offline, so a boolean would over-promise. Mirrors sipCapability. // -// Note this is deliberately separate from the campaign_management boolean, -// which keeps its existing meaning: "the credential holds the role." +// Callers must pass caps["campaign_management"] from the same Capabilities() +// call used for the campaign_management boolean, not a separately-matched +// hasRole lookup — otherwise the two can disagree on a single credential +// (e.g. a display-form role string) even though they describe the same fact. func tendlcCapability(hasRole bool) map[string]string { if !hasRole { return map[string]string{"status": "unavailable", "reason": "role_absent"} diff --git a/internal/api/envelope.go b/internal/api/envelope.go index b8bddff..9e5f681 100644 --- a/internal/api/envelope.go +++ b/internal/api/envelope.go @@ -5,7 +5,9 @@ import ( "fmt" ) -// Page is the pagination block present on every v2 A2P response. +// Page is the pagination block present on every Bandwidth v2 response that +// supports pagination (10DLC A2P registration and customer profiles, among +// others). type Page struct { Number int `json:"pageNumber"` Size int `json:"pageSize"` diff --git a/internal/api/filter.go b/internal/api/filter.go index e3847d2..fc9bc28 100644 --- a/internal/api/filter.go +++ b/internal/api/filter.go @@ -8,9 +8,18 @@ import ( // Filter operators. The API uses OpenAPI deepObject encoding, so a filter is // sent as field[op]=value. Sending the bare field=value form is accepted by // the server and then IGNORED — the result looks successful but is unfiltered. +// +// This is the full set of operators the tendlc and customerprofile deepObject +// filter parameters document (see ~/Developer/api-specs/external/tendlc.yml, +// components.parameters.*Param): string/enum fields support eq and/or +// contains depending on the field, and date fields (createdDate, +// modifiedDate) support gt and lt — NOT gte/lte, which do not appear +// anywhere in the spec. const ( OpEq = "eq" OpContains = "contains" + OpGt = "gt" + OpLt = "lt" ) // Filter is one deepObject query filter. diff --git a/internal/api/filter_test.go b/internal/api/filter_test.go index 4e5296d..f67d8f1 100644 --- a/internal/api/filter_test.go +++ b/internal/api/filter_test.go @@ -58,10 +58,10 @@ func TestEncodeQueryDeepObjectForm(t *testing.T) { { name: "same field, different ops: both kept", filters: []Filter{ - {Field: "createdDate", Op: "gte", Value: "2024-01-01"}, - {Field: "createdDate", Op: "lte", Value: "2024-12-31"}, + {Field: "createdDate", Op: OpGt, Value: "2024-01-01"}, + {Field: "createdDate", Op: OpLt, Value: "2024-12-31"}, }, - want: "?createdDate%5Bgte%5D=2024-01-01&createdDate%5Blte%5D=2024-12-31", + want: "?createdDate%5Bgt%5D=2024-01-01&createdDate%5Blt%5D=2024-12-31", }, } for _, tt := range tests {