Skip to content

feat(domain): migrate domain list to v3, add premium/pricing support - #211

Merged
jpage-godaddy merged 20 commits into
mainfrom
cli-v3-domain-list
Aug 18, 2026
Merged

feat(domain): migrate domain list to v3, add premium/pricing support#211
jpage-godaddy merged 20 commits into
mainfrom
cli-v3-domain-list

Conversation

@jpage-godaddy

@jpage-godaddy jpage-godaddy commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

Migrates gddy domain list from the v1 API to the v3 Domain Lifecycle Management API's listDomains, plus premium-domain/pricing support that shipped alongside it. v3 domains is now live in production and this has been sign-off'd by the domains team.

  • domain list calls v3 GET /domain-names instead of v1. --status stays repeatable (comma-joined client-side, matching the API's style: form, explode: false); the default "hide non-visible domains" view maps to lifecycleGroups excluding TERMINAL. Output fields and --schema type move to the v3 Domain shape (breaking change for --output json consumers).
  • domain list fetches every page via listDomains's cursor pagination (links[rel=next]), not just the first — an explicit --limit/--offset window (cli-engine's existing pagination pipeline) short-circuits the fetch once satisfied; unflagged, every domain is fetched. Requests the API's max pageSize (200) each time to minimize round trips against a tightly rate-limited API. A defensive cap (50 pages) errors rather than silently returning a partial list if a next link ever loops or is malformed.
  • Afternic premium-domain support end to end: Fee/FeeType, TermPrice.fees/firstTermPrice, RegistrationQuote.fees/inventory, Registration.fees, Consent.acknowledgedFees. A premium quote's fees/inventory are surfaced and cached, and purchase echoes the fees back into consent.acknowledgedFees (required server-side for the purchase to succeed).
  • Bulk check-availability's domain cap corrected to the now-official 25 (was 50).
  • domain available lists every priced registration term (1yr, 2yr, 3yr, ...) as a nested table instead of a single 1-year headline, including any first-term promotion. Also breaking for --output json consumers (price/renewalPrice/period/periodLabel replaced by terms[]). TermPrice.recommended is deliberately not surfaced — it's a hint meant for web UIs, not CLI output.

Deliberately not included:

Production readiness

  • Vendored spec re-diffed against the canonical source and the private prototype bundle: every schema/path this crate uses matches exactly, no drift since the last sync.
  • servers[0].url in the vendored spec is now prod-canonical (api.godaddy.com) — needed correctly by generate-api-catalog's resolve_catalog_base_url, which derives every other environment's URL from it by host substitution.
  • Live-verified directly against api.godaddy.com: domain list/available/suggest/quote all return genuine 200s; domain get/dns list against domains this account doesn't own return proper structured DOMAIN_NOT_FOUND/ZONE_NOT_FOUND errors (not gateway 404s), confirming those routes are correctly wired.
  • Caught and worked around a transient rollout gap (an EFD/routing config issue that had listDomains/getDomain/DNS-records gateway-404ing even though suggest/available/quote were live) — confirmed fixed by the domains team before finalizing this PR.

Test plan

  • cargo check, cargo clippy -- -D warnings, cargo test, cargo fmt --check all clean (656 tests)
  • Live-smoke-tested against prod: domain list (default view, pagination envelope via --limit/--offset), domain available, domain suggest, domain quote, domain get, dns list
  • Copilot review clean across 12+ rounds — every finding fixed or explained on the PR thread (see review history)
  • Rebased onto latest main, no conflicts

🤖 Generated with Claude Code

Copilot AI lite review requested due to automatic review settings August 13, 2026 16:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Preview migration of the Domains CLI to newer v3 Domain Lifecycle Management API shapes, including premium-domain fee acknowledgement and richer pricing/term output, backed by an updated vendored OpenAPI spec.

Changes:

  • Migrate gddy domain list to v3 listDomains and update default fields/schema to the v3 Domain shape.
  • Add premium-domain fee plumbing: cache quote fees, surface them in domain quote output, and echo them into consent.acknowledgedFees on domain purchase.
  • Update domain available to emit per-term pricing as a nested terms[] table and sync spec changes (listDomains, fees, availability cap).

Reviewed changes

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

Show a summary per file
File Description
rust/src/quote_cache.rs Cache premium quote fees alongside existing quote metadata.
rust/src/domain/suggest.rs Update test fixtures for new v3 suggestion fields (fees/first-term/recommended).
rust/src/domain/quote.rs Surface premium inventory/fees in quote output and cache fees for purchase.
rust/src/domain/purchase.rs Echo cached quote fees into purchase consent; align request struct fields with new spec.
rust/src/domain/list.rs Switch domain list to v3 listDomains and adjust filtering/default fields.
rust/src/domain/common.rs Remove now-unused headline pricing helper (available now emits all terms).
rust/src/domain/available.rs Replace headline price output with per-term terms[] nested table output.
rust/domains-client/src/lib.rs Update generated-client test expectations for new consent/registration fields.
rust/domains-client/openapi/swagger_domains.v3.yaml Vendor spec updates for listDomains, fee types, acknowledgedFees, and availability cap.
rust/domains-client/openapi/domains.oas3.json Mirror spec updates in merged OAS3 JSON consumed by codegen.
Suppressed comments (1)

rust/src/domain/list.rs:113

  • listDomains is cursor-paginated (returns a DomainCollection with pagination links), but this handler only sends a single request and returns items from the first page. For accounts with more domains than the API default page size, gddy domain list will silently omit the remaining domains.

Consider looping while a links entry with rel == "next" exists: extract pageToken from its href, call list_domains().page_token(...) (and direction if required), and accumulate items until no next link remains (or until the CLI --limit is satisfied).

            let client = make_client(&ctx).await?;
            let mut req = client.list_domains();
            if !statuses.is_empty() {
                // `statuses` is `style: form, explode: false` — one
                // comma-joined value, not repeated `statuses=` pairs
                // (progenitor always seq-serializes a `Vec` as repeated pairs
                // regardless of the spec's `explode` setting; see
                // `comma_joined`'s doc comment / DEVEX-882).
                req = req.statuses(comma_joined(statuses));
            } else if visible_only {
                req = req.lifecycle_groups(
                    comma_joined(
                        DEFAULT_VISIBLE_GROUPS
                            .into_iter()
                            .map(str::to_string)
                            .collect(),
                    )
                    .into_iter()
                    .map(types::DomainLifecycleGroup::from)
                    .collect::<Vec<_>>(),
                );
            }
            let resp = match req.send().await {
                Ok(r) => r,
                Err(e) => return Err(api_error("listing domains", debug, e).await),
            };
            let domains: Vec<serde_json::Value> = resp
                .into_inner()
                .items
                .unwrap_or_default()
                .iter()
                .map(serde_json::to_value)
                .collect::<std::result::Result<_, _>>()
                .map_err(|e| {

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread rust/src/quote_cache.rs

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Suppressed comments (2)

rust/src/domain/list.rs:106

  • listDomains returns a paginated DomainCollection (default pageSize is 100, with cursor-based pageToken + links[rel=next]). The handler currently performs a single send() and serializes only that page’s items, so accounts with more than one page of domains will get truncated results.

Consider looping until there is no next link (parse pageToken/optional direction from the href) and accumulating items across pages; also consider setting pageSize to the API max (200) to reduce requests.

            let resp = match req.send().await {
                Ok(r) => r,
                Err(e) => return Err(api_error("listing domains", debug, e).await),
            };
            let domains: Vec<serde_json::Value> = resp

rust/src/domain/list.rs:88

  • The statuses/lifecycleGroups query params require the comma_joined workaround (style: form, explode: false), but there’s no regression test here to prove we keep sending a single statuses=ACTIVE,EXPIRED (and the default visible lifecycleGroups=...) rather than repeated statuses= pairs.

Add an httpmock-based test similar to rust/src/domain/agreements.rs’s tlds_are_sent_as_a_single_comma_joined_query_param, covering both multiple --status values and the default visible-only lifecycleGroups path.

This issue also appears on line 102 of the same file.

                // comma-joined value, not repeated `statuses=` pairs
                // (progenitor always seq-serializes a `Vec` as repeated pairs
                // regardless of the spec's `explode` setting; see
                // `comma_joined`'s doc comment / DEVEX-882).
                req = req.statuses(comma_joined(statuses));

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Suppressed comments (1)

rust/src/domain/list.rs:30

  • MAX_PAGES’ doc comment says hitting the cap “is treated as a bug … rather than silently returned as if it were the complete list”, but fetch_domains currently just exits the loop and returns Ok(items) even if a rel=next token is still present. That’s misleading for maintainers and future debugging of truncated results.
/// Defensive cap on pages fetched for one invocation. No real account should
/// ever approach `MAX_PAGE_SIZE * MAX_PAGES` (10,000) domains; hitting this
/// is treated as a bug (a malformed or looping `next` link) rather than
/// silently returned as if it were the complete list.
const MAX_PAGES: usize = 50;

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Suppressed comments (2)

rust/src/domain/list.rs:343

  • The pagination tests only exercise absolute href values, but the spec’s link examples commonly use relative paths (e.g. /v3/domains/...). Using a relative href here would better pin the real-world behavior and would have caught the Url::parse issue.
    fn next_page_token_parses_token_and_direction_from_the_next_link() {
        let links = next_link(
            "https://api.example.com/v3/domains/domain-names?pageToken=abc123&pageTokenDirection=forward",
        );
        let (token, direction) = next_page_token(&links).expect("next link present");

rust/src/domain/available.rs:78

  • shared_currency looks only at price/renewal_price, but the handler can emit firstTermPrice in terms. If (now or in a future API version) a term includes only firstTermPrice with a currency code, the top-level currency field would be omitted even though the information is available.
/// The currency code shared by a domain's priced terms, sourced from whichever
/// term has a price or renewal price first (all terms use the same currency in
/// practice, so one top-level field covers every row in `terms`).
fn shared_currency(prices: &[types::TermPrice]) -> Option<String> {
    prices
        .iter()
        .find_map(|t| t.price.as_ref().or(t.renewal_price.as_ref()))
        .and_then(|m| m.currency_code.as_ref())
        .map(|c| c.to_string())

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Suppressed comments (3)

rust/src/domain/quote.rs:384

  • fees caching silently drops JSON serialization errors via .ok(). If serialization ever fails (e.g., a future type change), the quote will still be shown but domain purchase will later fail with quote_mismatch because consent.acknowledgedFees can’t be echoed back. This should fail fast with a clear message (similar to profile_json) instead of silently caching None.
                    fees: quote
                        .fees
                        .as_ref()
                        .filter(|f| !f.is_empty())
                        .and_then(|f| serde_json::to_value(f).ok()),

rust/src/domain/list.rs:105

  • The PR description says true cursor pagination for domain list is “deliberately not included” and that the handler fetches only the first page. This implementation now follows links[rel=next] across pages via fetch_domains, so the description looks out of date and could mislead reviewers/users about behavior and rate-limit impact.
/// Fetch every domain matching `statuses`/`visible_only`, following v3's
/// `links[rel=next]` cursor until the API reports no further page — or until
/// `stop_at` items have been accumulated, when an explicit `--limit`/
/// `--offset` window needs no more than that many (cli-engine's own
/// pagination pipeline slices the exact window from whatever this returns;

rust/domains-client/openapi/swagger_domains.v3.yaml:1156

  • A safety note explaining why replaceDNSRecord (PUT) was removed is no longer present. Given the historical zone-wipe behavior (#136) and the PR description stating this endpoint is still deliberately excluded, keeping a short warning here helps prevent accidental reintroduction without a re-verification.
  /zones/{zone}/dns-records/{recordId}:

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Suppressed comments (3)

rust/src/domain/list.rs:43

  • parse_statuses uppercases each --status value twice (to_uppercase() in both validation and return). This does extra allocation/work for every status value; compute the uppercase string once and reuse it for both the enum validation and the returned wire form.
fn parse_statuses(raw: &[String]) -> Result<Vec<String>> {
    raw.iter()
        .map(|s| {
            types::DomainStatus::try_from(s.to_uppercase().as_str())
                .map(|_| s.to_uppercase())
                .map_err(|_| CliCoreError::message(format!("invalid --status {s:?}")))

rust/domains-client/openapi/domains.oas3.json:18

  • The OpenAPI servers[0].url is now set to the OTE (test) host. If any code (or future consumers) uses the generated client’s default base URL from the spec, this can cause unintended calls to test instead of prod. Since the CLI already selects the environment via config (config.domains_api_url), consider keeping the spec’s server URL pointed at the production host and relying on runtime config for test/prod selection.
  "servers": [
    {
      "url": "https://api.ote-godaddy.com",
      "description": "Domains API host"
    }

rust/src/domain/quote.rs:56

  • fees_to_json always emits a type key, but its value can be null when f.type_ is absent. That null will leak into --output json and the nested table output; it’s clearer to omit the key when it’s missing (consistent with how other optional fields are handled in this file).
                let mut out = json!({
                    "type": f.type_.as_ref().map(|t| t.to_string()),
                });

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Suppressed comments (1)

rust/src/domain/quote.rs:265

  • inventory and fees were added to the quote output/table view, but there are no unit tests asserting that view_columns() correctly renders these new nested fields (or that they stay omitted when absent). Since this file already has view-rendering regression tests for other nested fields, add a similar test that builds an envelope containing inventory + a non-empty fees array and asserts the human view includes the expected nested rows/labels.
        TableColumn::new("inventory", "Inventory"),
        TableColumn::new("fees", "Fees").nested(vec![
            TableColumn::new("type", "Type"),
            TableColumn::new("amount", "Amount"),
            TableColumn::new("currency", "Currency"),
        ]),

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Suppressed comments (1)

rust/src/domain/list.rs:159

  • PR description says the defensive pagination cap should error when a links[rel=next] is malformed; however fetch_domains currently treats an unparseable/missing pageToken as end-of-list and returns a partial result silently (next_page_token returns None both for “no next page” and “malformed next link”). Consider failing fast when a next rel is present but pageToken can’t be extracted, so callers don’t get truncated domain lists that look complete. (If you keep the current behavior, the PR description should be updated to match.)
        page_token = collection.links.and_then(|links| next_page_token(&links));
        if page_token.is_none() {
            return Ok(items);
        }

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Suppressed comments (1)

rust/src/domain/list.rs:236

  • stop_at is computed via ctx.middleware.offset.max(0) + limit, which can overflow for large --offset values (panics in debug builds; wraps in release). Using a saturating/checked add avoids overflow while preserving the existing fallback-to-usize::MAX behavior.
            let limit = ctx.middleware.limit;
            let stop_at = (limit > 0).then(|| {
                usize::try_from(ctx.middleware.offset.max(0) + limit).unwrap_or(usize::MAX)
            });

jpage-godaddy and others added 17 commits August 18, 2026 10:57
CachedQuote.fees's doc comment said "register" must echo the fees back
into consent.acknowledgedFees; the user-facing command is `domain
purchase`.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Regression tests mirroring agreements.rs's existing tlds test: httpmock
asserts `domain list` sends one `statuses=ACTIVE,EXPIRED` (or the
default-view `lifecycleGroups=...`) query param, not repeated pairs —
the exact shape the live API rejects with MISMATCH_FORMAT.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
`listDomains` is cursor-paginated (DomainCollection + links[rel=next]),
but the handler only ever sent one request and returned that page's
items — accounts with more domains than one API page got silently
truncated results. Flagged by Copilot review.

fetch_domains() now follows links[rel=next] until the API reports no
further page, requesting the API's max pageSize (200) each time to
minimize round trips against a tightly rate-limited API. An explicit
--limit/--offset window (already wired via cli-engine's pagination
pipeline) short-circuits the fetch once satisfied, so a small --limit
doesn't pay for pages it'll just discard; unflagged, every domain is
fetched, matching the command's pre-v3 behavior and the pagination
opt-in's own documented invariant.

A defensive MAX_PAGES cap treats a malformed/looping next link as an
error rather than a silent partial result.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
fetch_domains()'s doc comment already claimed exceeding MAX_PAGES "is
treated as a bug ... rather than silently returned as if it were the
complete list," but the loop just broke and returned Ok(items)
regardless of whether a next page was still pending. Flagged by
Copilot review.

fetch_domains now returns cli_engine::Result directly (folding the
domains_client-error conversion in, since the loop needs its own error
path too) and only falls through past the loop when MAX_PAGES pages
were exhausted with page_token still Some — every other exit returns
early. Added a regression test asserting the error path fires after
exactly MAX_PAGES requests against a mock that always advertises a
next page.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
next_page_token() used url::Url::parse(href), which rejects relative
references outright — but the v3 spec's own link examples use relative
paths (e.g. /v3/domains/domain-names?...). Against a real response
shaped that way, pagination would have silently stopped after page
one. Flagged by Copilot review.

Parse only the query string (everything after the first '?') via
url::form_urlencoded instead of the whole href as a URL, which works
for both relative and absolute hrefs. Added a regression test using a
relative href, and switched the existing multi-page/error-path tests
to relative hrefs too so they'd have caught this.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… serialization

Two more Copilot findings:

- The comment explaining why replaceDNSRecord (PUT) was removed
  (#136 — silently wipes the zone) had been dropped from
  the vendored spec at some point during this branch's history,
  despite being deliberately restored earlier. Restored it verbatim;
  confirmed the rest of the file is otherwise byte-identical to
  origin/main for this path.
- quote.rs cached a quote's `fees` via `serde_json::to_value(f).ok()`,
  silently discarding a serialization failure as `None`. That would
  surface at `domain purchase` as a confusing `quote_mismatch` instead
  of here. Now fails fast with a clear message, mirroring the existing
  `profile_json` pattern in the same function.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ees_to_json

Two more Copilot findings:

- parse_statuses called s.to_uppercase() twice per value (once for
  DomainStatus validation, again for the returned wire form). Compute
  it once and reuse it.
- fees_to_json always emitted a "type" key, leaking a JSON null into
  --output json / the nested table when a fee's type is absent.
  Now omitted when missing, consistent with how every other optional
  field in this file is handled.

(A third finding — the vendored spec's cosmetic servers[0].url — was
evaluated and left as-is: this field is never read at runtime, every
client construction in this codebase goes through client_with_auth
with an explicitly-resolved base URL, and this branch is deliberately
scoped to the test environment per the PR description, so pointing it
at the OTE host is correct for now rather than a drive-by prod
repoint.)

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Mirrors the existing requiredAgreements/resolved nested-block test:
asserts view_columns() renders a premium quote's inventory/fees as a
nested table, and that a non-premium quote's blank Inventory:/Fees:
rows never leak a literal "null" (the regression fees_to_json's `type`
key omission targets).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…lently

next_page_token() conflated "no next link" (genuinely done) with "next
link present but couldn't extract a pageToken" (a bug) into one
Option — both returned None, both silently stopped pagination.
Flagged by Copilot review.

Per the spec, links[rel=next] is only ever present when more items
are actually available, so a present-but-unparseable link is a
guarantee of more data this CLI failed to reach — that should error,
not look like a clean finish. A domain list that silently stops N
pages short and returns 200 OK is worse than one that errors loudly:
the caller has no way to tell "that's everything" from "that's
whatever we got before giving up".

Replaced next_page_token()'s Option return with a three-state
NextPage enum (Done/Token/Unparseable) so fetch_domains can tell the
two apart and error immediately on Unparseable rather than only after
exhausting MAX_PAGES. Added a regression test proving the error fires
on page one (a single request), not after looping to the cap.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
ctx.middleware.offset.max(0) + limit is an i64 addition that panics on
overflow in debug builds (and wraps in release) for a large enough
--offset — an unbounded, user-controlled CLI flag. Flagged by Copilot
review. saturating_add avoids that while preserving the existing
fallback to usize::MAX when the sum doesn't fit.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
shared_currency() only checked price/renewalPrice; a term carrying
only a firstTermPrice promotion (schema permits it, even though every
live example so far pairs it with a price) would render firstTermPrice
values with no top-level currency to interpret them against. Flagged
by Copilot review. Added a regression test for that case.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ent as []

Both Consent.acknowledgedFees (minItems: 1 when present, "omit when
the quote carries no purchase fees") and Registration.fees (readOnly)
are constructed with vec![] for the common non-premium purchase.
That's only correct because progenitor generates
skip_serializing_if = "Vec::is_empty" for both fields — verified by
inspection, now pinned by a test so a future spec/codegen change can't
silently start sending an empty array and breaking every non-premium
purchase. Flagged by Copilot review.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…5 cap

AvailabilityCheckCriteria's maxItems was corrected 50->25 earlier this
branch, but three other description strings still said 1-50 (the top-
level API description, the Discovery tag description, and
checkAvailability's own operation description). Flagged by Copilot
review — the spec was internally inconsistent for readers and
downstream tooling that surfaces these descriptions.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Copilot review flagged that generate-api-catalog's
resolve_catalog_base_url derives every non-prod environment's URL by
substituting the host in this spec's servers[0].url, which requires
that value to be prod-canonical — but the currently-checked-in
domains.oas3.json on this branch has it set to OTE, since this
preview branch's spec syncs were done by hand against the test
environment rather than by running this script.

Leaving the OTE value as-is per discussion — this branch isn't meant
to merge until v3 ships to prod and gets a real spec sync, at which
point running this script restores the prod-canonical value. Added a
comment here so the next person regenerating the spec understands
the discrepancy instead of being surprised by it.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…rrency/terms consistent

Two more Copilot findings:

- next_page_token silently dropped an unrecognized pageTokenDirection
  (e.g. a value outside backward/forward) to None instead of erroring,
  inconsistent with treating every other unparseable part of a
  guaranteed-more-data next link as a bug. Distinguished "absent"
  (fine, the field is documented optional) from "present but invalid"
  (now Unparseable) so an ordinary link with no direction still works.
- domain available could emit a top-level currency with no
  corresponding terms rows if every TermPrice happened to lack a
  period (term_to_json drops those, shared_currency doesn't care about
  period at all) — a self-inconsistent payload. currency is now only
  set alongside a non-empty terms array.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
My previous fix (keeping currency from appearing with no terms) went
too far the other way: gating terms on shared_currency(&prices)
succeeding meant terms disappeared entirely when every priced term
lacked a currencyCode (itself optional per simple-money) even though
the prices/terms were perfectly valid. Flagged by Copilot review.

terms is now emitted whenever any priced term exists, full stop;
currency is the one gated on terms being non-empty, so it still never
appears with nothing to interpret it against, but pricing with no
currency code no longer loses its terms entirely.

(The other repeated finding this round — domains.oas3.json's OTE
servers[0].url breaking generate-api-catalog's env-derivation
convention — is already covered by the explanatory comment added in
574d5e6; leaving as-is per prior discussion, this branch isn't meant
to merge before a real spec sync against prod.)

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
shared_currency stopped at the first term with any price/renewal/
first-term money, then looked only at that money's currencyCode. Since
currencyCode is itself optional, a first term whose price lacks one
made the whole function return None even when a later term (or a
later field on the same term) carried a valid currency. Flagged by
Copilot review.

Now searches every price-like value across every term for the first
one with a currencyCode, rather than fixing on the first price-like
value regardless of whether it has one.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@jpage-godaddy
jpage-godaddy marked this pull request as draft August 18, 2026 18:17
Comment thread rust/src/domain/list.rs
Comment thread rust/domains-client/openapi/swagger_domains.v3.yaml
v3 domains is now live in prod (confirmed live: an EFD/routing config
gap that had listDomains/getDomain/DNS-records gateway-404ing was
fixed). Re-diffed the vendored spec against the canonical source and
the private prototype bundle: every schema/path this crate actually
uses (listDomains, getDomain, DNS records, suggest, availability,
quote, register, operations) matches exactly — no field-level drift
since the last sync. The only upstream additions are still the
deliberately-deferred v3.2 auto-renew/transfer-lock endpoints and the
still-excluded DNS PUT (#136), neither touched here.

The one real change: servers[0].url moves from the OTE placeholder
back to the prod-canonical host, now that this is correct rather than
a temporary stand-in — generate-api-catalog's resolve_catalog_base_url
derives every other environment's URL from this value by host
substitution, so it needs to be prod. Dropped the now-resolved
TEMPORARY note in regenerate-spec.sh and fixed its file reference
(environments/mod.rs -> environments/catalog.rs, after this branch's
rebase onto main's environments module split).

Live-verified against api.godaddy.com: domain list/available/suggest/
quote all return genuine 200s; domain get/dns list against a domain
this account doesn't own return proper structured DOMAIN_NOT_FOUND/
ZONE_NOT_FOUND errors (not gateway 404s), confirming those routes are
correctly wired too.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@jpage-godaddy jpage-godaddy changed the title feat(domain): migrate domain list to v3, add premium/pricing support (preview, do not merge) feat(domain): migrate domain list to v3, add premium/pricing support Aug 18, 2026
@jpage-godaddy
jpage-godaddy marked this pull request as ready for review August 18, 2026 19:56
@jpage-godaddy
jpage-godaddy requested a lite review from Copilot August 18, 2026 19:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

…atching

`domain list` moved to v3's listDomains; v1's list operation had no
remaining callers, so drop it from trim-spec.py/merge-spec.py's retained
op set, the vendored spec JSON, and the crate's test suite.

Replace the manual "re-apply these deviations by hand" comment in
regenerate-spec.sh with an automated jq patch step that re-applies the
two deliberate deviations from the upstream contract (statuses query
param stays a plain string; replaceDNSRecord PUT stays excluded) to
domains.oas3.json on every regeneration, so refreshing the vendored v3
spec never requires a human to remember a manual edit. Two new tests
assert directly on the vendored spec file as a safety net in case the
patch itself ever stops applying.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@jpage-godaddy
jpage-godaddy merged commit 8c2367a into main Aug 18, 2026
4 checks passed
@jpage-godaddy
jpage-godaddy deleted the cli-v3-domain-list branch August 18, 2026 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants