Support numeric store IDs and Shop GIDs for the shared --store flag#7802
Support numeric store IDs and Shop GIDs for the shared --store flag#7802amcaplan wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds centralized --store value resolution so shopify store commands can accept a myshopify domain (existing behavior), a numeric store ID, or a Shop GID, resolving IDs/GIDs to a store domain via Business Platform without requiring per-command changes.
Changes:
- Introduces
resolveStore()to normalize domain inputs locally and resolve numeric IDs / Shop GIDs to a store domain via BP org enumeration + per-org lookup. - Hooks resolution into
StoreCommand.parse()post-parse to avoid BP auth/network calls before oclif flag validation. - Updates flag/help text and regenerates CLI manifest and shopify.dev docs to reflect the expanded accepted
--storeformats.
Reviewed changes
Copilot reviewed 12 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/store/src/cli/utilities/store-resolution.ts | Adds store input parsing + BP-backed ID/GID-to-domain resolution logic. |
| packages/store/src/cli/utilities/store-resolution.test.ts | Adds unit tests covering domain vs numeric vs GID paths, org scanning, and error handling. |
| packages/store/src/cli/utilities/store-command.ts | Resolves --store after full oclif parsing/validation via an overridden parse(). |
| packages/store/src/cli/flags.ts | Updates shared --store flag description and removes flag-level parsing hook. |
| packages/store/src/cli/api/graphql/business-platform-organizations/queries/store-by-shop-id.graphql | Adds Organizations API query for accessibleShop(id:). |
| packages/store/src/cli/api/graphql/business-platform-organizations/generated/store-by-shop-id.ts | Adds generated typed document/types for the new Organizations API query. |
| packages/store/src/cli/api/graphql/business-platform-destinations/queries/list-organizations.graphql | Adds Destinations API query to enumerate organizations. |
| packages/store/src/cli/api/graphql/business-platform-destinations/generated/list-organizations.ts | Adds generated typed document/types for the org enumeration query. |
| packages/cli/README.md | Regenerates CLI README help text for updated --store description. |
| packages/cli/oclif.manifest.json | Regenerates oclif manifest reflecting updated --store description. |
| docs-shopify.dev/generated/generated_docs_data_v2.json | Regenerates reference docs data for updated --store description. |
| docs-shopify.dev/commands/interfaces/store-info.interface.ts | Updates generated interface docs for --store description. |
| docs-shopify.dev/commands/interfaces/store-execute.interface.ts | Updates generated interface docs for --store description. |
| docs-shopify.dev/commands/interfaces/store-auth.interface.ts | Updates generated interface docs for --store description. |
| .changeset/store-flag-id-gid.md | Adds changeset documenting expanded --store accepted formats and whitespace trimming. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| handler: async () => { | ||
| const newToken = await ensureAuthenticatedBusinessPlatform() | ||
| return {token: newToken} | ||
| }, |
| // BP's ShopifyShopID *input* scalar expects a base64-encoded `gid://organization/ShopifyShop/<id>`, | ||
| // not the bare numeric id (the bare numeric is only how the scalar is *serialized in responses*). | ||
| // Mirrors app-management-client.ts `encodedGidFromShopId`. | ||
| const encodedShopId = encodeGid(`gid://organization/ShopifyShop/${shopId}`) |
There was a problem hiding this comment.
Should we have a encodeGidFromShopId function in cli-kit for these two cases? :thinking_face:
|
|
||
| expect(result).toBe(expected) | ||
| expect(businessPlatformRequestDoc).not.toHaveBeenCalled() | ||
| expect(businessPlatformOrganizationsRequestDoc).not.toHaveBeenCalled() |
There was a problem hiding this comment.
Should we also validate that ensureAuthenticatedBusinessPlatformis not called? (to ensure that auth is not being triggered even if no request is actually executed).
The shared --store flag (defined in the store package) previously accepted only myshopify.com domains. It now also accepts a numeric store ID and a Shop GID (gid://shopify/Shop/<id>), each resolved to the store's domain via the Business Platform. Resolution is centralized in StoreCommand.parse (a post-parse hook), so every store command gains the capability with no per-command changes. IDs and GIDs resolve through a single org-agnostic Business Platform "destinations" lookup (currentUserAccount.destination(id:)); the domain path is unchanged and stays pure string normalization with zero network calls. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
fff0a97 to
bbc99b4
Compare
|
Updated the resolution approach based on review feedback — thanks for the nudge on the org loop. What changed: the first revision resolved IDs/GIDs by enumerating the user's organizations and probing each with Why it's valid:
Net effect: one BP request instead of N+1; the per-org loop (old D4) and the 100-org cap (old D5) are gone. Decisions D4/D5 in the description are updated accordingly. On the local auth-cache idea: deferred (D8). The cache is keyed by domain and stores no shop ID today; we'll add the store ID once the in-flight cache-iteration work lands. I force-pushed this as a single clean commit (rebase-merge hygiene) rather than stacking a revert-then-redo. 🤖 This comment was written by Claude Code on Ariel's behalf. |
Summary
The shared
--storeflag (defined in thestorepackage) previously accepted only myshopify.com domains. Per the project brief, it should also accept numeric store IDs and Shop GIDs. This PR adds centralized resolution so allstorecommands accept all three input forms with no per-command changes:shop.myshopify.com,shop, or an admin URL (unchanged behavior, zero network calls)123gid://shopify/Shop/123How it works
Resolution lives in a new
resolveStore()utility, invoked from aStoreCommand.parsepost-parse hook:/^gid:\/\//) → validated againstgid://shopify/Shop/<id>; non-Shop GIDs are rejected./^\d+$/) → treated as a store ID.normalizeStoreFqdn(no network).IDs and GIDs resolve to the store's domain through a single, org-agnostic Business Platform "destinations" call:
This works because a shop destination's
publicIdis the shop'sshopify_shop_id(backenddestination/shop.rb:public_id: shop.shopify_shop_id). The numeric id / Shop GID is encoded into theDestinationPublicIDinput —base64("gid://organization/ShopifyShop/<id>")— and the canonical*.myshopify.comhost is read from the destination'swebUrl.Is this easier or harder than what we do now?
Slightly harder than the domain path — that path is pure string normalization with zero network calls, whereas an ID/GID needs Business Platform auth and one GraphQL request plus new codegen. But it's only marginally so: it's a single request (see the design note below), and the domain path is preserved exactly and stays network-free, so the added cost is paid only when an ID or GID is actually passed.
How do we distinguish the formats?
gid://and contain slashes, which a domain never does. (As the brief notes, we don't need to worry about GID-style domains.)Decisions made (flagged for your review)
The task said to make informed decisions on points of ambiguity and document them here rather than ask upfront:
123.myshopify.com), but a bare123resolves as a store ID. A user with an all-numeric subdomain must pass the full123.myshopify.com. Rationale: store IDs are the headline new capability and the common case; the workaround for the rare all-numeric subdomain is trivial and the flag help text spells out the accepted forms.StoreCommand.parse, not in the flag'sparsecallback. A flag-levelparsefires before oclif validates the other flags, so a user with an unrelated flag typo would get an auth prompt ahead of the plain validation error. Moving resolution to a post-parse hook (mirroringBaseCommand.parse) means all flag validation happens first, and it keeps the logic centralized — no command has to opt in.destination(id:)lookup (no org enumeration). ⭐ Updated after review. An earlier revision enumerated the user's organizations and probed each withaccessibleShop(id:)(org-scoped API) — an N+1 loop. After confirming against the BP backend thatcurrentUserAccount.destination(id:)is org-agnostic and that a shop'spublicId == shopify_shop_id, this is now a single request — no enumeration, no per-org loop.webUrl, falling back toprimaryDomain. A destination'swebUrl(=shop.uri) is the canonical*.myshopify.comURL; itsprimaryDomain(=shop.primary_domain || shop.uri) may be a merchant custom domain. ReadingwebUrlfirst avoids resolving a custom-domain store to e.g.www.example.com. (This replaces the previous D5, the "first:100 org cap", which no longer exists now that there's no enumeration.)gid://shopify/Product/1) with a friendly error, rather than extracting the trailing number and guessing intent.DestinationPublicID/ShopifyShopIDinput encoding. These BP input scalars expect a base64-encoded GID (gid://organization/ShopifyShop/<id>), even though shop ids serialize in responses as the bare numeric. We encode viaencodeGid, mirroringapp-management-client.ts'sencodedGidFromShopId. (An in-repo comment describing the output format initially misled the research here; caught in cross-provider review.)store authsession cache is keyed by domain and stores no shop ID today, so it can't map ID→domain. There's separate in-flight work to allow iterating all cached stores; once that lands, we'll add the store ID to the cache and short-circuit resolution for already-authed stores. Punted for now — with resolution down to one cheap call, the payoff is small.The resolver is unit-tested against a mocked Business Platform (21 tests covering the domain / numeric / GID paths, the input encoding, the
webUrl-over-primaryDomainpriority, not-found, no-domain, auth/network failure, and whitespace trimming). It has not been exercised against the live Business Platform. TheDestinationPublicIDinput encoding (D7) and thedestination(id:)response shape should be confirmed against a real store before this ships.Testing
storepackage: type-check ✅ · lint ✅ · unit tests ✅ (21/21 instore-resolution.test.ts)packages/cli/README.md, and the shopify.dev reference docs were regenerated for the flag-description change; the flag descriptions match.🤖 Generated with Claude Code