镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

feat(platform-app): convert App Registry GraphQL client to a spec-generated client - #282

Merged
jpage-godaddy merged 3 commits into
mainfrom
app-registry-client
Oct 21, 2026
Merged

jpage-godaddy merged 3 commits into
mainfrom
app-registry-client

Conversation

@jpage-godaddy

@jpage-godaddy jpage-godaddy commented Sep 19, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Replaces the hand-written GraphQL client in src/platform/app/client.rs with the typed platform-app-client crate (chore(platform-app): add the generated App Registry GraphQL client crate [DEVEX-1173] #326), generated from the pinned App Registry schema (feat(api): add App Registry to the browsable API catalog [DEVEX-1173] #325).
  • Every ApplicationClient method and its callers across the platform app command tree (including import, native Android extensions, and redirectUris) now use typed request/response structs end to end. The one remaining untyped field is presentation/metadata, which the schema itself types as an opaque JSONObject scalar.
  • Mutations that return no data (enable/disable/archive/update/create-release/activate-release) now fail with an error instead of reporting success.
  • App Registry traffic is registered with --debug transport, including the add native-extension path.
  • Unset optional inputs stay out of update/create requests (wire-shape test), so an omitted manifest key still leaves remote values unchanged.
  • Closes DEVEX-1173.

Third of three PRs for DEVEX-1173, stacked on #326 (which is stacked on #325). This PR's own diff is the migration only.

Test plan

  • cargo check --workspace
  • cargo clippy --workspace --all-targets -- -D warnings
  • cargo test --workspace
  • cargo fmt --check
  • ./rust/scripts/check-module-size.sh
  • ./rust/scripts/smoke-test.sh (mocked App Registry server)

🤖 Generated with Claude Code

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.

Copilot review overview

🟡 Changes recommended

Several commands treat None mutation results as success (serializing to null), and the API catalog generator’s new multi-org support can clobber clones when repo names collide across orgs.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 5 Medium severity · 1 Low severity

Open (6)
What changed in this PR

This PR migrates the gddy platform app command tree from a hand-written, serde_json::Value-based App Registry GraphQL client to a typed, graphql_client-generated client (platform-app-client), and adds a generate-api-catalog refresh step to vendor the schema from the App Registry service repo.

Changes:

  • Introduces platform-app-client (build-time generated GraphQL operations + vendored schema) and updates the workspace to include it.
  • Refactors platform app commands and ApplicationClient to use typed request/response structs end-to-end (with serde_json conversion at CommandResult boundaries).
  • Extends the API catalog generator to support non-default GitHub orgs and explicit specPath sources, adding an App Registry schema refresh path.
File Description
rust/​tools/​generate-api-catalog/​src/​manifest.rs Adds org + specPath fields to remote catalog sources and updates manifest tests.
rust/​tools/​generate-api-catalog/​src/​main.rs Triggers App Registry schema refresh when the app-registry source is present.
rust/​tools/​generate-api-catalog/​src/​github.rs Supports per-source org/specPath and derives spec version from repo SHA for path-pinned sources.
rust/​tools/​generate-api-catalog/​src/​app_registry_spec.rs New helper to validate and write the vendored App Registry GraphQL schema.
rust/​src/​platform/​app/​commands/​validate.rs Switches validation logic to typed Application structs and updates tests.
rust/​src/​platform/​app/​commands/​update.rs Uses typed mutation input and typed response serialization for app update.
rust/​src/​platform/​app/​commands/​release.rs Uses typed release inputs (actions/subscriptions/extensions/settings) and typed response serialization.
rust/​src/​platform/​app/​commands/​list.rs Serializes typed application lists to JSON for output.
rust/​src/​platform/​app/​commands/​lifecycle.rs Uses typed enable/disable inputs and serializes typed results for output.
rust/​src/​platform/​app/​commands/​init.rs Replaces raw JSON traversal with typed “application with latest release” structs and updates tests.
rust/​src/​platform/​app/​commands/​info.rs Uses typed get_application result and serializes to JSON for output.
rust/​src/​platform/​app/​commands/​enablements.rs Moves enablements flattening from JSON pointers to typed structs.
rust/​src/​platform/​app/​commands/​deploy/​mod.rs Updates deploy orchestration to use typed application+release structures.
rust/​src/​platform/​app/​commands/​deploy/​extensions.rs Uses typed upload-url inputs/results and preserves header parsing behavior.
rust/​src/​platform/​app/​client.rs Replaces hand-written GraphQL transport/query building with the generated typed client.
rust/​src/​platform/​app/​client_tests.rs Moves/updates client tests to cover typed operations and error mapping.
rust/​schemas/​api/​manifest.json Registers the app-registry schema in the API catalog manifest.
rust/​platform-app-client/​src/​lib.rs New generated-client wrapper crate for App Registry GraphQL operations + transport.
rust/​platform-app-client/​graphql/​UpdateApplication.graphql New typed operation doc for updating an application.
rust/​platform-app-client/​graphql/​GenerateReleaseUploadUrl.graphql New typed operation doc for presigned upload URL generation.
rust/​platform-app-client/​graphql/​EnabledStoreApplications.graphql New typed operation doc for store enablements.
rust/​platform-app-client/​graphql/​EnableApplication.graphql New typed operation doc for enabling an app on a store.
rust/​platform-app-client/​graphql/​DisableApplication.graphql New typed operation doc for disabling an app on a store.
rust/​platform-app-client/​graphql/​CreateRelease.graphql New typed operation doc for creating a release (incl. settings/extensions).
rust/​platform-app-client/​graphql/​CreateApplication.graphql New typed operation doc for creating an application (incl. credentials).
rust/​platform-app-client/​graphql/​ArchiveApplication.graphql New typed operation doc for archiving an application.
rust/​platform-app-client/​graphql/​ApplicationWithLatestRelease.graphql New typed operation doc for application lookup + latest release/subscriptions.
rust/​platform-app-client/​graphql/​ApplicationsList.graphql New typed operation doc for listing applications with status filter.
rust/​platform-app-client/​graphql/​Application.graphql New typed operation doc for getting a single application by name.
rust/​platform-app-client/​graphql/​ActivateRelease.graphql New typed operation doc for activating a release.
rust/​platform-app-client/​graphql/​schema.graphql Vendored App Registry schema used for code generation.
rust/​platform-app-client/​Cargo.toml New crate manifest for the generated GraphQL client.
rust/​platform-app-client/​build.rs Build-time codegen step that produces codegen.rs from schema + operations.
rust/​examples/​smoke_mock_server.rs Updates mock server response shaping to satisfy newly-selected typed fields.
rust/​Cargo.toml Adds platform-app-client to the workspace and main crate deps.
rust/​Cargo.lock Locks new GraphQL client/codegen dependencies and platform-app-client crate.
rust/​api-catalog-sources.json Adds app-registry remote source pinned to gdcorp-commerce/app-registry-api + specPath.

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

Comment thread rust/src/platform/app/client.rs Outdated
Comment thread rust/src/platform/app/commands/lifecycle.rs
Comment thread rust/src/platform/app/commands/release.rs Outdated
Comment thread rust/src/platform/app/commands/update.rs Outdated
Comment thread rust/tools/generate-api-catalog/src/github.rs Outdated
Comment thread rust/platform-app-client/build.rs Outdated
jpage-godaddy added a commit that referenced this pull request Sep 19, 2026
…lti-org clone path collision

Addresses Copilot review feedback on #282:
- enable/disable/archiveApplication, updateApplication, and createRelease
  now error instead of silently reporting success when the mutation
  returns no data, matching the pattern already used for
  createApplication/generateReleaseUploadUrl.
- generate-api-catalog: nest cloned repos under their org, not just repo
  name, so two catalog sources with the same repo name in different orgs
  don't clobber each other's temp clone.
- Corrected a stale build.rs comment about why variables derive Deserialize.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@jpage-godaddy
jpage-godaddy requested a lite review from Copilot September 19, 2026 18:44

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.

Copilot review overview

🟡 Changes recommended

The new org-nested temp clone path needs parent-directory creation, and App Registry GraphQL transport debug logging is not registered so --debug transport loses visibility for these requests.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity

Open (1)
Resolved since last review (6)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Register TransportObserver for App Registry GraphQL debug logging

rust/​src/​platform/​app/​client.rs:129

GraphQL requests now rely on generated-client-support’s TransportObserver for --debug transport logging (via notify_request/notify_response_result), but platform app doesn’t register that observer anywhere. As a result, App Registry GraphQL traffic won’t appear in transport debug logs unless some other generated client happened to be initialized earlier in the same process. Register the shared observer before constructing the GraphQL client.

Comment thread rust/tools/generate-api-catalog/src/github.rs
jpage-godaddy added a commit that referenced this pull request Sep 19, 2026
…istry GraphQL calls

Addresses Copilot review feedback on #282: platform app's make_client
(and deploy's direct ApplicationClient::new, which bypasses make_client)
never called ensure_generated_client_transport_observer_registered, so
App Registry GraphQL traffic was invisible to --debug transport unless
some other generated client happened to register the observer first in
the same process. Both call sites now register it, matching
domain/email/shopping's make_client.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@jpage-godaddy

Copy link
Copy Markdown
Collaborator Author

Re: the "Previously missed" finding on registering the TransportObserver for App Registry GraphQL debug logging (client.rs:129, in this round's review body — not a resolvable thread since Copilot didn't post it as one).

Fixed in ae07c49: platform app's make_client now calls crate::http::ensure_generated_client_transport_observer_registered(), matching what domain/email/shopping's own make_client already do. Also caught that deploy's streaming command constructs ApplicationClient::new directly rather than going through make_client, so it needed the same call added separately.

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.

Copilot review overview

🔵 Needs a closer look

It introduces a new generated client crate and refactors a wide surface area of platform-app commands and tooling, which warrants final human review of behavior parity and long-term maintenance implications.

Review effort: Lite
Findings: None

Resolved since last review (1)
Previously missed (3)

In code that hasn't changed since last review

Medium severity Watch GraphQL directory for newly added operation files

rust/​platform-app-client/​build.rs:23

The build script only emits rerun-if-changed for files that exist at the time it runs; adding a new graphql/*.graphql operation document won’t trigger a rebuild/codegen until some already-tracked file changes. Add a directory-level rerun-if-changed so new operation files are picked up automatically.

Medium severity Support org/repo keys for unambiguous API catalog overrides

rust/​tools/​generate-api-catalog/​src/​github.rs:349

API_CATALOG_REPO_REFS overrides are looked up by repo_name only. Now that sources can come from multiple orgs, two sources with the same repo name in different orgs can't be overridden independently. Consider checking for an org/repo key first (with a fallback to the legacy repo key) so overrides remain unambiguous.

Low severity Format GraphQL errors using concise message fields

rust/​platform-app-client/​src/​lib.rs:127

ClientError::GraphQL currently formats the errors array with Debug ({errors:?}), which tends to produce noisy, unstable output for end users. Consider extracting and joining the message fields (and optionally adding location/path info later) so the CLI’s surfaced error string stays concise and predictable.

jpage-godaddy added a commit that referenced this pull request Sep 19, 2026
…verrides, concise GraphQL error text

Addresses Copilot review feedback on #282 (prose-only "previously missed"
findings, not resolvable threads):
- platform-app-client/build.rs: cargo:rerun-if-changed the graphql/
  directory itself, not just the files present when the script last ran —
  a newly added *.graphql operation doc wouldn't otherwise trigger codegen
  until some other watched file changed.
- generate-api-catalog: API_CATALOG_REPO_REFS overrides are now looked up
  by "org/repo" first, falling back to the legacy bare "repo" key, so two
  sources sharing a repo name across orgs can be overridden independently.
- platform-app-client: ClientError::GraphQL now joins graphql_client::
  Error's own Display (path:line:col: message) instead of Debug-formatting
  the whole error struct.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@jpage-godaddy

Copy link
Copy Markdown
Collaborator Author

Re: this round's three "Previously missed" findings (prose in the review body, not resolvable threads):

Fixed in 75b471d:

  • Watch GraphQL directory for newly added operation files — build.rs now does cargo:rerun-if-changed=graphql (the directory itself) in addition to the individual files, so a newly added .graphql doc triggers codegen.
  • Support org/repo keys for unambiguous API catalog overrides — API_CATALOG_REPO_REFS lookups now try org/repo first, falling back to the legacy bare repo key.
  • Format GraphQL errors using concise message fields — turns out graphql_client::Error already implements Display as path:line:col: message; ClientError::GraphQL now joins that instead of Debug-formatting the whole struct.

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.

Copilot review overview

🔵 Needs a closer look

It introduces a new generated client crate plus multi-file refactors across the platform app command tree and build tooling, warranting final human review for API/CLI output compatibility and long-term maintenance risk.

Review effort: Lite
Findings: None

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.

🟡 Changes recommended

Unresolved catalog endpoint, runtime registration, and repository-reference issues remain.

2 open findings
2 resolved since last review

🧠 Review effort: Lite


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

Comment thread rust/api-catalog-sources.json Outdated
Comment thread rust/tools/generate-api-catalog/src/github.rs Outdated
@jpage-godaddy
jpage-godaddy force-pushed the app-registry-client branch 2 times, most recently from fb79625 to 48366a3 Compare October 7, 2026 23:08
@jpage-godaddy
jpage-godaddy requested a lite review from Copilot October 7, 2026 23:08

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.

🔵 Needs a closer look

The broad generated-client and API integration changes require final human review.

0 open findings

2 resolved since last review

🧠 Review effort: Lite


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

@jpage-godaddy
jpage-godaddy force-pushed the app-registry-client branch 2 times, most recently from 8ca666a to 4df10fb Compare October 14, 2026 19:29
@jpage-godaddy
jpage-godaddy removed this pull request from stack #283 October 14, 2026 19:34
@jpage-godaddy
jpage-godaddy changed the base branch from main to platform-app-client-crate October 14, 2026 19:38
@jpage-godaddy
jpage-godaddy added this pull request to stack #327 October 14, 2026 19:39
…#325)

`gddy api` now lists the App Registry GraphQL API: its operations, types
and schema are browsable and callable like the other catalog domains.

The schema is pinned in the repo, vendored from gdcorp-commerce/app-registry-api
(the service's own repo) rather than the stale gdcorp-platform specification
repo, and refreshed by generate-api-catalog.

Catalog sources can now name their own GitHub org and an explicit spec path.
A GraphQL schema carries no server URL, so GraphQL sources must declare
`baseUrl` and `endpointPath`; App Registry is served from
https://api.godaddy.com/v1/apps/app-registry-subgraph. `API_CATALOG_REPOS`
and `API_CATALOG_REPO_REFS` accept `org/repository`, and a bare repository
name that more than one source shares is rejected (repos) or ignored (refs)
instead of silently picking one.

A test now fails if a committed catalog file is not embedded in the CLI, and
another if a GraphQL catalog entry drifts from its declared endpoint.

Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
…ate [DEVEX-1173]

Adds platform-app-client, a typed client for the App Registry subgraph
generated at build time by graphql_client from the pinned schema and one
.graphql document per operation, matching the generated-client pattern used
for domains/hosting/shopping/email. It is not used by the CLI yet.

Unset optional mutation inputs are omitted from the request rather than sent
as explicit nulls, since the service treats an omitted field as "leave
unchanged" and a null as "clear". Responses to credential-bearing operations
(CreateApplication's secrets, GenerateReleaseUploadUrl's pre-signed URL) are
never shown to the transport observer, and a response that fails to decode is
reported without its contents, so `--debug transport` and error output cannot
expose them.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…erated client [DEVEX-1173]

Replaces the hand-written GraphQL client in src/platform/app/client.rs with
the typed platform-app-client crate, generated from the pinned App Registry
subgraph schema.

Every ApplicationClient method and its callers across the platform app
command tree now use typed request/response structs end to end; the only
remaining untyped fields are `presentation`/`metadata`, which the schema
itself types as an opaque JSONObject scalar.

Mutations that return no data (enable/disable/archive/update/create-release/
activate-release) now fail with an error instead of reporting success, and
App Registry traffic is registered with `--debug transport`, including the
`add native-extension` path.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Base automatically changed from platform-app-client-crate to main October 21, 2026 18:08
@jpage-godaddy
jpage-godaddy merged commit 9442ee0 into main Oct 21, 2026
6 checks passed
@jpage-godaddy
jpage-godaddy deleted the app-registry-client branch October 21, 2026 18:10
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.

2 participants