Harden agent-facing contracts + fix live-broken Numbers/quickstart commands#11
Open
kshahbw wants to merge 23 commits into
Open
Harden agent-facing contracts + fix live-broken Numbers/quickstart commands#11kshahbw wants to merge 23 commits into
kshahbw wants to merge 23 commits into
Conversation
…check Adds cmd/doccontract_test.go (package cmd) which parses every `band …` usage in README.md and AGENTS.md and asserts the command path and flags exist in the live Cobra tree. Known rename-tracked drift (--site vs --subaccount on location create) is listed in knownDrift. Removes the docs-check CI job that only warned on file-diff heuristics; the new test is a hard gate that runs as part of `go test ./...` in the existing test job on all three OS targets.
…eated sub-account
Research against all six Bandwidth SDKs and internal Confluence confirmed the Messaging API has no public test/sandbox host: the prod endpoint is the entry point for UAT too, and messaging is tested with test accounts/numbers, not a separate host. The earlier change invented test.messaging.bandwidth.com by analogy to test.api/test.voice, which would make --environment test point at a non-existent host. Replace messagingHostForEnvironment(env) with messagingHost() that always returns prod unless BW_MESSAGING_URL overrides it.
…saging is production-only
… wrapper The orders API rejects a bare top-level TelephoneNumberList with HTTP 500 and requires a SiteId (code 5022). Add the required --subaccount flag and emit the correct wrapped body. Verified live: order now returns OrderStatus RECEIVED.
Without the wrapper the disconnects API returns 5001 (invalid TN list). Verified live: release now returns OrderStatus RECEIVED.
…ion, retry VCP assign Reuse number.BuildOrderBody (SiteId + ExistingTelephoneNumberOrderType wrapper). Orders require a sub-account with a default SIP peer (codes 5022/5020), so the VCP path now ensures both. The just-ordered number provisions asynchronously, so the VCP assignment is retried until it succeeds (VCS-0044 until ready). Verified live: VCP quickstart now reaches status=complete.
…ist TNs If --wait polling hits a FeatureLimitError (credential lacks the Numbers role for /tns listing), the order itself already succeeded — surface it with a warning and exit 0 instead of reporting a conflict (exit 4). Verified live.
…rect docs Per code review: - VCP-assign retry now only retries VCS-0044 / 429 / 5xx / transport and fails fast on other 4xx (was retrying any error); add assignErrIsRetryable + test. - Preserve ErrPollTimeout (exit 5) on assign timeout instead of wrapping the last attempt error; surface last attempt + manual 'vcp assign' recovery. - number order docs now show the required --subaccount flag (README/AGENTS). - Correct the quickstart 'partial' resume docs: an ordered-but-unassigned number is not auto-reassigned on re-run.
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
govulncheck flagged 4 Go stdlib CVEs (net, net/http, crypto/x509, net/textproto) reachable via version check + api client; all fixed in go1.26.3/1.26.4. Verified locally: govulncheck now reports 0 called vulns.
…er lookup) - MessagingClient now mints its OAuth token against the PROD API host, not the test realm — a test-realm token is rejected by the prod messaging endpoint (messaging is production-only). - resolveEnvironment normalizes case/whitespace and rejects unknown values instead of silently falling through to prod; --environment leak fixed by resetting EnvironmentOverride every PersistentPreRun. - firstAssignedNumber reads the explicit data[].phoneNumber field via FlattenResponse instead of regex-sniffing any numeric string (deleted firstE164/e164Re/phoneKeyRe); filter scoping verified live. - VCP-assign retry also retries 422; messaging warning reworded to be accurate for non-send commands. Verified live: messaging warning fires; VCP quickstart re-run detects the existing assigned number via the field read and skips ordering.
Removes the byte-identical FakeClient/NewTestRoot/CaptureStdout copies from the call and recording golden tests so an api.Requester change updates one place.
The voice-app find-or-create was duplicated between the VCP and legacy paths; extract it so the app payload can't drift. (The sub-account/SIP-peer find-or- create differs between paths in peer naming and result-field population, so that unification is left as a deliberate follow-up rather than risk the legacy path.)
…account provisioning - README command table: number order shows the required --subaccount. - AGENTS number-order guidance notes --subaccount is required. - README quickstart description no longer implies only --legacy uses a sub-account (the VCP path now provisions one too).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
BLUF
Hardens
band's agent-facing contracts (exit codes,--plain,--environment, messaging safety) and fixes a cluster of Numbers/quickstart commands that were silently broken against the live Bandwidth API. Every behavior-changing fix in part 2 was verified live against a real account.Part 1 — Agent-contract & CI hardening
--waittimeouts now exit 5 (ErrPollTimeoutsentinel +ExitCodeForErrormapping) — previously exit 1, breaking the documented contract agents rely on.messagingHost()with aBW_MESSAGING_URLoverride, and it warns when used under a test environment that the request hits production.--environmentflag is actually wired now — it was declared but read by nothing. Precedence: flag >BW_ENVIRONMENT> profile config. Voice/Platform/Dashboard honor it; messaging stays prod + warns. The account hint reflects the resolved env.api.Requester) + golden--plaincontract tests forcall list/recording listthat exercise the real command bodies.band … --flagusage against the live Cobra tree; replaced the non-blockingdocs-checkwarning that could only detect "no docs touched."gofmtvia a golangci-lintformattersblock (thestandardset doesn't include it); reformatted 18 files.Part 2 — Live-API fixes (found by testing against a real account)
The Dashboard/Iris APIs require typed wrappers + a
SiteIdthat the CLI was omitting; these commands could not have worked:number orderTelephoneNumberList→ HTTP 500SiteId+ExistingTelephoneNumberOrderTypewrapper; new required--subaccountnumber releaseDisconnectTelephoneNumberOrderTypewrapperquickstart(VCP)number order --waitVerified live end-to-end (e.g. VCP quickstart →
status: complete).band number ordernow requires--subaccount <id>— the orders API mandates aSiteId, and the command never actually worked without it. Help, examples, and docs updated.Notes
band vcp assign <vcp-id> <number>recovery hint (it is not auto-reassigned).--site→--subaccountflag rename remains its own effort (the doc-contract test carries one documentedknownDriftentry for it).gofmt, andgolangci-lint(0 issues) are green.