feat: migrate domain & dns commands to the v3 Domains API by jpage-godaddy · Pull Request #76 · godaddy/cli

jpage-godaddy · 2026-07-01T22:58:48Z

What

Migrates the domain and dns command groups from the v1/v2 Domains API to the v3 Domain Lifecycle Management API, keeping v1 only where v3 has no equivalent yet.

Highlights

domains-client: regenerated from a merged OAS3 spec — the vendored v3 bundle plus the retained v1 operations (prefixed V1 to avoid schema-name collisions) — via a new merge-spec.py and reworked trim-spec.py.
domain: available / suggest / get / quote / purchase / nameservers set on v3; list + agreements stay on v1 (no v3 equivalent yet). Each subcommand now lives in its own module under src/domain/, with shared helpers in common.rs.
Strict two-step registration: domain quote locks a price and caches the quote locally (src/quote_cache.rs); domain purchase --quote-token <tok> --agree --confirm accepts that exact quote (no re-quote → charges the reviewed price), records consent, registers, and polls the async operation.
Central OAuth scope registry (src/scopes.rs) that the CLI's OAuth client must mirror, drawn from by every command. Read scopes (apps.app-registry:read, domains.domain:read) are granted at login; write scopes are declared per-command and requested on demand (OAuth step-up). In particular the app-registry mutations (application init/update/enable/disable/archive/release/deploy) declare apps.app-registry:write themselves rather than defaulting it into every login, since app-registry writes are rare for most customers.
ISO-4217 minor-unit money formatting (via iso_currency); contacts.toml mapped to the v3 Contact shape.

Testing

cargo fmt --check, cargo clippy --workspace --all-targets -- -D warnings (clean), cargo test --workspace (all pass).
Verified live against the test environment: quote → purchase registers a domain (status: ACTIVE); gap commands (list, dns list/set/delete, agreements) still work via v1.

🤖 Generated with Claude Code

Migrate the `domain` and `dns` command groups from the v1/v2 Domains API to the v3 Domain Lifecycle Management API, keeping v1 only where v3 has no equivalent yet. - domains-client: regenerate from a merged OAS3 spec (vendored v3 bundle + retained v1 ops, V1-prefixed) via new merge-spec.py / reworked trim-spec.py. - domain: available/suggest/get/quote/purchase/nameservers on v3; list + agreements stay on v1. Split each subcommand into its own module under src/domain/, with shared helpers in common.rs. - Strict two-step registration: `domain quote` locks a price + caches the quote (src/quote_cache.rs); `domain purchase --quote-token` accepts it, records consent, registers, and polls the async operation. - Central OAuth scope registry (src/scopes.rs) the client must mirror. - ISO-4217 minor-unit money formatting (iso_currency); contacts.toml mapped to the v3 Contact shape. - Fixes: suggest --limit panic, register response parsing (writeOnly quoteToken), QUOTE_MISMATCH from not re-sending the quoted profile. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot

Copilot wasn't able to review this pull request because it exceeds the maximum number of lines (20,000). Try reducing the number of changed lines and requesting a review from Copilot again.

Copilot

Pull request overview

Copilot reviewed 24 out of 26 changed files in this pull request and generated 2 comments.

…ofile Address Copilot review on #76: - application commands: the 7 app-registry-mutating commands (init, update, enable, disable, archive, release, deploy) now declare [apps.app-registry:read, apps.app-registry:write] via with_scopes, so the write scope is requested on demand (OAuth step-up) rather than granted at every login. DEFAULT_OAUTH_SCOPES stays read-only — app-registry writes are a rarely-used operation for most customers. - domain quote: build_profile now returns InlineRegistrationProfile directly (it always produced Some(..)); dropped the misleading Option and corrected the doc comment. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 25 out of 27 changed files in this pull request and generated 5 comments.

Address Copilot re-review on #76 (all doc/test clarity, no behavior change): - quote_cache: module + Lookup::Found docs referenced a 'take' that consumes; the API is read-only get() + remove()-after-success. Reworded to match. - dns v3_records: doc said SRV/MX fields are 'clamped' to u16; they're converted via u16::try_from (out-of-range -> None), and clap already bounds them. - domains-client tests: availability/quote mock 'value's used the v1 micro-unit scale (11_990_000 / 23_980_000); v3 money is ISO-4217 minor units, so use 1199 / 2398 to match format_money and keep the contract clear. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 25 out of 27 changed files in this pull request and generated 5 comments.

…o spec Address Copilot re-review round 2 on #76 (docs/tests, no behavior change): - scopes: APP_REGISTRY_WRITE docstring still said 'requested at login by default'; corrected to describe the on-demand step-up (declared per-command by the application mutations), matching the round-1 default-scope change. - domains-client tests: the register fixtures used consent actor type SHOPPER, which isn't a documented ConsentActorType; switched to DIRECT to match what production (domain purchase) sends and the spec's documented values. Not changed: adding CAA to the DNS record-type allowlist. The pre-migration allowlist was A/AAAA/CNAME/MX/NS/SOA/SRV/TXT (no CAA), so this migration preserves parity; proper CAA support also needs --flag/--tag plumbing that's out of scope here. Tracked as a follow-up. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 25 out of 27 changed files in this pull request and generated 5 comments.

…e/suggest Address Copilot re-review round 3 on #76: - purchase: fail fast when a cached quote's profile can't be deserialized (was silently dropped -> confusing server-side QUOTE_MISMATCH); clear re-quote message instead. - purchase: check the cached quote's agreement types are non-empty *before* the --agree gate, so a corrupt/empty cache yields one accurate 're-quote' error rather than a 'requires agreeing…' prompt with an empty list that a rerun can't satisfy. Regression test added. - domain available / suggest: replace with_json_schema::<Availability/Suggestion> (raw API types) with explicit output_schema! matching the transformed JSON the handlers actually emit (formatted price/currency strings, headline term, listPrice), so --schema/help field metadata is accurate. - contacts: validate_country doc clarified — it checks the ISO-3166 alpha-2 *shape* (e.g. ZZ passes), not list membership; the API validates the code. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 25 out of 27 changed files in this pull request and generated 7 comments.

…rchase/available/suggest) Address Copilot re-review round 4 on #76 (--schema/help accuracy, no runtime change): - available: price/currency/renewalPrice/period marked optional (emitted only when the API returns a headline price). - suggest: listPrice marked optional. - quote: DomainQuoteResult completed to mirror quote_to_json (added renewalPrice/expiresAt/irreversible/agreements/requiredAgreements/resolved) with the conditional fields marked optional. - purchase: operationId/price/currency marked optional. Deliberately not changing the runtime JSON to omit null keys: whether the CLI should emit explicit null vs. omit absent keys is a CLI-wide output-convention decision (it affects every command's json! output, not just these four), so it belongs in a dedicated change rather than this migration. Marking the schema fields optional already makes --schema/help accurate for both shapes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 25 out of 27 changed files in this pull request and generated 3 comments.

…ema optionality Address Copilot re-review round 5 on #76 (help/schema accuracy, no runtime change): - domain contacts: group help said contacts.toml is for 'domain purchase' only; the two-step flow reads it at quote time and binds it into the token, so the help now says edit-and-re-quote to change contacts. - domain group help: noted that 'nameservers set' needs domains.nameserver:update (previously only read + create scopes were mentioned). - nameservers: operationId/status marked optional in NameserversResult (they're serialized from Option values from the async operation). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 25 out of 27 changed files in this pull request and generated 2 comments.

…_api doc Address Copilot re-review round 8 on #76: - purchase: consent_principal now validates the customer subject is a UUID (via the already-present uuid crate) before the paid register call, so a malformed customer:<non-uuid> subject fails fast with a clear message instead of an opaque server rejection. Matches the documented customer:<uuid> format; test added. - contacts: corrected the to_api doc — country validation is an ISO-3166 alpha-2 *shape* check (not list membership), and name_middle/job_title/fax are not struct fields (a leftover such key is simply ignored by serde), not 'kept in the file schema'. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 25 out of 27 changed files in this pull request and generated 1 comment.

Address Copilot re-review round 9 on #76: domain list mapped request errors to a generic 'listing domains failed: {e}' string, dropping the server response body and the --debug request-id. Switched to the shared api_error helper, so non-2xx failures are as debuggable as the other domain/dns commands. (Last straggler on the old error pattern.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 25 out of 27 changed files in this pull request and generated 1 comment.

… flow Address Copilot re-review round 10 on #76: the scaffolded contacts.toml header still said contacts are used only for 'gddy domain purchase'. In the v3 two-step flow they're read at quote time and locked into the token, so the header now says edit-and-re-quote to change them (matching the group help updated earlier). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 25 out of 27 changed files in this pull request and generated 4 comments.

…olish Address Copilot re-review round 11 on #76: - quote_cache: is_expired now treats an unparseable expires_at as EXPIRED (was not-expired). A malformed timestamp only arises from corruption/manual edit/ old format, so the entry is suspect; expiring it avoids keeping its serialized profile (potential PII) on disk indefinitely. Absent expiry still means never-expires-locally. Test added. - scopes: use the fully-qualified cli_engine::CommandSpec::with_scopes intra-doc link so rustdoc resolves it. - contacts: template header line renamed to 'gddy domain registration contacts' (completing the quote-time-flow wording; body was updated last round). Not changed: the maintainer OAuth-client-registration pointer block in scopes.rs (internal AuthZ console URLs + non-secret project/app/client IDs). It was added intentionally as a maintainer aid; the IDs are identifiers, not secrets, in a private repo. Left for the repo owner to decide whether to genericize. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

0.3.5 renders next_actions as a 'Next steps:' footer in human output (they were only surfaced in JSON before). The domain/dns commands' next actions (quote->purchase, available->quote, list->dns, etc.) now show in the default human view. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 25 out of 27 changed files in this pull request and generated 1 comment.

Address Copilot re-review round 12 on #76: domain quote used serde_json::to_value(&profile).ok(), silently dropping a serialization failure and caching a profile-less quote — which would make purchase re-send a different request than was quoted (server-side QUOTE_MISMATCH). Now a serialization failure is a hard error (symmetric with the purchase-side deserialize fail-fast). The cached profile is therefore always present. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 25 out of 27 changed files in this pull request and generated no new comments.

jbrooks2-godaddy · 2026-07-02T20:25:07Z

+            // it. Resolve auth *before* consuming the cached quote so a failed
+            // login never touches the cache.
+            let cred = ctx.credential().await?;
+            if cred.provider == SSO_KEY_PROVIDER {


Is this accurate/confirmed with domains? If so we should probably block at least quote in the same way - you could get through quote with apikey then get blocked here.

Missed this. I'm actually just going to remove all SSO key support; it was something I put in place before knowing we'd be ready with OAuth for all APIs.

Done in 1cfa772 — went further and removed all sso-key support (it predated OAuth being ready for these APIs). There's no api-key path in quote or purchase now, so nothing to block: dropped CompositeAuthProvider/SSO_KEY_PROVIDER, the sso-key/Bearer header branch, the env api_key/api_secret config + resolution, and this rejection. GoDaddyAuthProvider (OAuth) is the sole provider.

jbrooks2-godaddy · 2026-07-02T20:35:15Z

+                "price": price,
+                "currency": currency,
+            });
+            Ok(CommandResult::new(result).with_next_actions(vec![


This is always ok, should it be an error if status was FAILED ?

Good catch — fixed in 1cfa772. A terminal FAILED status now returns a non-zero error ("registration for failed (operation ); no domain was registered…") instead of an Ok success payload.

jbrooks2-godaddy · 2026-07-02T20:39:03Z

+                        .send()
+                        .await
+                    {
+                        return Err(api_error("adding DNS record", debug, e).await);


Hmm, if v3 is a single record per call, should this cli command mimic and only allow a single? Failing in the middle might be somewhat confusing, I don't think we'd be clear about what happened on this error case if it was the 2nd out of 3 records.

I think we either:

Restrict this command to one record

Make this try to apply all records, continuing through if one fails, and then return details on success/failure for each apply

Went with option 2 in 1cfa772: dns add attempts every --data record (no longer bails on the first failure), returns a per-record created/failed report on success, and on any failure exits non-zero with a ✓/✗ breakdown naming exactly which values were created and which failed (and why). Extracted a pure summarize_add_outcomes helper with tests for the all-created and partial-failure cases.

sswaminathan-godaddy

Reviewed the v3 migration. Overall the design is solid — the quote cache, scope registry macro, body surfacing, and fail-fast purchase gates are all well done. A few things worth discussing before merge, noted below as inline comments.

sswaminathan-godaddy · 2026-07-02T20:41:50Z

+            };
+
+            let client = make_client_with_cred(&ctx.middleware.env, &cred)?;
+            let idempotency_key = uuid::Uuid::new_v4().to_string();


Idempotency key is generated fresh on every invocation — retry risk for a paid call.

uuid::Uuid::new_v4() is called each time domain purchase runs. If the first register_domain() request reaches the server and is processed (charge initiated) but the response is lost (network drop, timeout), retrying with --quote-token generates a new idempotency key — the server has no way to recognise it as a retry and could initiate a second charge or registration attempt.

Suggested fix: add idempotency_key: Option<String> to CachedQuote, set it on the first purchase attempt, and reuse it on subsequent calls with the same quote token. Remove it alongside the quote after a successful register. This is the canonical purpose of idempotency keys.

Fixed in 1cfa772. The register idempotency key is now minted once at quote time, stored on the cached quote, and reused on every purchase attempt for that token — so a retry after a lost/timed-out response is recognized server-side as the same request rather than risking a second charge. It's dropped with the quote on success. (Older cache entries without one fall back to a fresh key.)

sswaminathan-godaddy · 2026-07-02T20:41:50Z

+                .unwrap_or_else(|| "SUBMITTED".to_string());
+            let operation_id = accepted.operation_id.clone();
+            if let Some(op_id) = operation_id.as_ref() {
+                for _ in 0..20 {


Poll timeout exits silently — user can't distinguish "still running" from "polling gave up".

The loop runs at most 20 × 3 s = 60 s, then falls through with the last-known status (e.g. SUBMITTED) in the JSON output. The domain get next action is always shown (unconditionally), so the user can't tell from the output whether polling completed normally or stopped short. A non-terminal exit from this loop is worth logging: tracing::info!("operation still in progress after polling; check status with 'gddy domain get {domain}'").

Fixed in 1cfa772. When the bounded poll exits without a terminal status, purchase now emits tracing::info!("registration still in progress after polling; check later with gddy domain get ") so a timeout is distinguishable from completion.

sswaminathan-godaddy · 2026-07-02T20:41:50Z

+/// agreements (the `--agree` gate) and only [`remove`]s it once the registration
+/// actually succeeds, so an aborted/gated attempt leaves the quote reusable.
+pub fn get(token: &str) -> Lookup {
+    let Some(path) = quotes_path() else {


quotes_path() returning None returns Lookup::Missing, which purchase surfaces as "no cached quote for that token."

If dirs::config_dir() returns None (stripped containers, some CI environments) the user gets the same error as "you never ran domain quote" with no indication that their system configuration is the issue. This is unlikely in practice but worth a distinct message, e.g. checking quotes_path() before the lookup in purchase and surfacing "could not locate the config directory" separately.

Fixed in 1cfa772. Added a distinct Lookup::NoConfigDir returned by quote_cache::get when quotes_path() is None; purchase now surfaces "could not locate a config directory to read the quote cache from…" separately from the genuine "no cached quote for that token" case.

Resolves reviewer feedback on #76: - Remove sso-key dual-mode auth entirely (Jacob, purchase.rs:208). API keys are deprecated at launch, so the CLI is OAuth-only: dropped CompositeAuthProvider, SSO_KEY_PROVIDER, the Bearer/sso-key header branch, the env api_key/api_secret config + resolution, and the purchase-time sso-key rejection. GoDaddyAuthProvider is the sole provider. (No quote/purchase split needed — no api-key path exists.) - purchase: return a non-zero error on a terminal FAILED operation status instead of reporting success (Jacob, purchase.rs:351). - dns add: attempt every --data record, don't stop at the first failure; report per-record created/failed and exit non-zero if any failed, with a ✓/✗ breakdown (Jacob, dns/mod.rs:368). New pure summarize_add_outcomes helper + tests. - purchase: reuse a single idempotency key across retries — minted at quote time, stored in the cached quote — so a lost-response retry can't double-charge (Swaminathan, purchase.rs:296). - purchase: tracing::info when bounded polling exits non-terminal, pointing at `domain get` (Swaminathan, purchase.rs:321). - quote_cache: distinct Lookup::NoConfigDir so a missing config dir surfaces its own error rather than "no cached quote" (Swaminathan, quote_cache.rs:144). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 26 out of 28 changed files in this pull request and generated 3 comments.

+    if let Some(price) = quote.price.as_ref()
+        && let Some(p) = format_money(price)
+    {
+        out["price"] = json!(p);
+        out["currency"] = json!(price.currency_code.as_ref().map(|c| c.to_string()));
+    }


+                if let Some(price) = term.price.as_ref().and_then(format_money) {
+                    result["price"] = json!(price);
+                    result["currency"] = json!(
+                        term.price
+                            .as_ref()
+                            .and_then(|m| m.currency_code.as_ref())
+                            .map(|c| c.to_string())
+                    );
+                }


+        let list = agreement_titles
+            .iter()
+            .map(|t| format!("  - {t}"))
+            .collect::<Vec<_>>()
+            .join("\n");


jpage-godaddy requested a review from Copilot July 1, 2026 22:59

Copilot started reviewing on behalf of jpage-godaddy July 1, 2026 22:59 View session

Copilot AI reviewed Jul 1, 2026

jpage-godaddy changed the base branch from main to rust-port July 1, 2026 23:05

jpage-godaddy requested a review from Copilot July 1, 2026 23:05

Copilot started reviewing on behalf of jpage-godaddy July 1, 2026 23:06 View session

Copilot AI reviewed Jul 1, 2026

View reviewed changes

Comment thread rust/src/environments/mod.rs

Comment thread rust/src/domain/quote.rs Outdated

jpage-godaddy requested a review from Copilot July 1, 2026 23:27

Copilot started reviewing on behalf of jpage-godaddy July 1, 2026 23:27 View session

Copilot AI reviewed Jul 1, 2026

View reviewed changes

Comment thread rust/src/quote_cache.rs Outdated

Comment thread rust/src/quote_cache.rs

Comment thread rust/src/dns/mod.rs Outdated

Comment thread rust/domains-client/src/lib.rs

Comment thread rust/domains-client/src/lib.rs

jpage-godaddy requested a review from Copilot July 1, 2026 23:32

Copilot started reviewing on behalf of jpage-godaddy July 1, 2026 23:33 View session

Copilot AI reviewed Jul 1, 2026

View reviewed changes

Comment thread rust/src/scopes.rs Outdated

Comment thread rust/src/dns/mod.rs

Comment thread rust/domains-client/src/lib.rs

Comment thread rust/domains-client/src/lib.rs

Comment thread rust/domains-client/src/lib.rs

jpage-godaddy requested a review from Copilot July 1, 2026 23:40

Copilot started reviewing on behalf of jpage-godaddy July 1, 2026 23:40 View session

Copilot AI reviewed Jul 1, 2026

View reviewed changes

Comment thread rust/src/domain/purchase.rs Outdated

Comment thread rust/src/domain/purchase.rs Outdated

Comment thread rust/src/contacts/mod.rs Outdated

Comment thread rust/src/domain/available.rs

Comment thread rust/src/domain/suggest.rs

jpage-godaddy requested a review from Copilot July 1, 2026 23:48

Copilot started reviewing on behalf of jpage-godaddy July 1, 2026 23:48 View session

Copilot AI reviewed Jul 1, 2026

View reviewed changes

jpage-godaddy requested a review from Copilot July 2, 2026 00:26

Copilot started reviewing on behalf of jpage-godaddy July 2, 2026 00:26 View session