From b05e0a53864d77ea572a415bcb70cd3dbd93ea20 Mon Sep 17 00:00:00 2001 From: Iain Wilson Date: Wed, 14 Oct 2026 19:08:54 +0200 Subject: [PATCH 1/4] Route theme mutations through requestThemeAdminDoc --- .changeset/theme-mutations-missing-scope.md | 5 +++++ .../cli-kit/src/public/node/themes/api.test.ts | 16 ++++++++++++++++ packages/cli-kit/src/public/node/themes/api.ts | 12 ++++++------ 3 files changed, 27 insertions(+), 6 deletions(-) create mode 100644 .changeset/theme-mutations-missing-scope.md diff --git a/.changeset/theme-mutations-missing-scope.md b/.changeset/theme-mutations-missing-scope.md new file mode 100644 index 00000000000..fa65debeb95 --- /dev/null +++ b/.changeset/theme-mutations-missing-scope.md @@ -0,0 +1,5 @@ +--- +'@shopify/cli-kit': patch +--- + +Show the missing theme access scope message for theme changes, as theme reads already do. diff --git a/packages/cli-kit/src/public/node/themes/api.test.ts b/packages/cli-kit/src/public/node/themes/api.test.ts index 2fd5d1a860a..a4e143cd2b3 100644 --- a/packages/cli-kit/src/public/node/themes/api.test.ts +++ b/packages/cli-kit/src/public/node/themes/api.test.ts @@ -539,6 +539,14 @@ describe('themePublish', () => { expect(theme!.role).toEqual(role) }) } + + test('throws a friendly error when access is denied by a missing theme access scope', async () => { + vi.mocked(adminRequestDoc).mockRejectedValue(themeAccessDeniedError('`write_themes` access scope.')) + + await expect(themePublish(123, session)).rejects.toThrow( + 'The authenticated account or access token is missing `write_themes` access scope.', + ) + }) }) describe('deleteThemeAssets', () => { @@ -625,6 +633,14 @@ describe('themeDelete', () => { expect(response).toBe(true) }) } + + test('throws a friendly error when access is denied by a missing theme access scope', async () => { + vi.mocked(adminRequestDoc).mockRejectedValue(themeAccessDeniedError('`write_themes` access scope.')) + + await expect(themeDelete(123, session)).rejects.toThrow( + 'The authenticated account or access token is missing `write_themes` access scope.', + ) + }) }) describe('themeDuplicate', () => { diff --git a/packages/cli-kit/src/public/node/themes/api.ts b/packages/cli-kit/src/public/node/themes/api.ts index efefab6be9d..94512914ab8 100644 --- a/packages/cli-kit/src/public/node/themes/api.ts +++ b/packages/cli-kit/src/public/node/themes/api.ts @@ -242,7 +242,7 @@ export async function deleteThemeAssets(id: number, filenames: Key[], session: A for (let i = 0; i < filenames.length; i += batchSize) { const batch = filenames.slice(i, i + batchSize) // eslint-disable-next-line no-await-in-loop - const {themeFilesDelete} = await adminRequestDoc({ + const {themeFilesDelete} = await requestThemeAdminDoc({ query: ThemeFilesDelete, session, variables: { @@ -332,7 +332,7 @@ async function uploadFiles( files: {filename: string; body: {type: OnlineStoreThemeFileBodyInputType; value: string}}[], session: AdminSession, ): Promise { - return adminRequestDoc({ + return requestThemeAdminDoc({ query: ThemeFilesUpsert, session, variables: {themeId: themeGid(themeId), files}, @@ -420,7 +420,7 @@ export async function themeUpdate(id: number, params: ThemeParams, session: Admi } recordEvent('theme-api:update-theme') - const {themeUpdate} = await adminRequestDoc({ + const {themeUpdate} = await requestThemeAdminDoc({ query: ThemeUpdate, session, variables: {id: composeThemeGid(id), input}, @@ -451,7 +451,7 @@ export async function themeUpdate(id: number, params: ThemeParams, session: Admi export async function themePublish(id: number, session: AdminSession): Promise { recordEvent('theme-api:publish-theme') - const {themePublish} = await adminRequestDoc({ + const {themePublish} = await requestThemeAdminDoc({ query: ThemePublish, session, variables: {id: composeThemeGid(id)}, @@ -482,7 +482,7 @@ export async function themePublish(id: number, session: AdminSession): Promise { recordEvent('theme-api:delete-theme') - const {themeDelete} = await adminRequestDoc({ + const {themeDelete} = await requestThemeAdminDoc({ query: ThemeDelete, session, variables: {id: composeThemeGid(id)}, @@ -521,7 +521,7 @@ export async function themeDuplicate( let requestId: string | undefined recordEvent('theme-api:duplicate-theme') - const {themeDuplicate} = await adminRequestDoc({ + const {themeDuplicate} = await requestThemeAdminDoc({ query: ThemeDuplicate, session, variables: {id: composeThemeGid(id), name}, From 6e63c65a63b71107f3608d9889248d6df6a7ccd9 Mon Sep 17 00:00:00 2001 From: Iain Wilson Date: Thu, 15 Oct 2026 14:54:15 +0200 Subject: [PATCH 2/4] Read the access scope from mapped GraphQL errors Theme mutations keep the default Admin error handling. It changes the ClientError into a GraphQLClientError, so the missing-scope check did not see the ACCESS_DENIED error. The check now reads the GraphQL errors from both error types. The tests use the mapped error that theme mutations get. --- .../cli-kit/src/public/node/themes/api.test.ts | 9 +++++++-- packages/cli-kit/src/public/node/themes/api.ts | 18 +++++++++++------- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/packages/cli-kit/src/public/node/themes/api.test.ts b/packages/cli-kit/src/public/node/themes/api.test.ts index a4e143cd2b3..365999c93c7 100644 --- a/packages/cli-kit/src/public/node/themes/api.test.ts +++ b/packages/cli-kit/src/public/node/themes/api.test.ts @@ -28,6 +28,7 @@ import {GetThemes} from '../../../cli/api/graphql/admin/generated/get_themes.js' import {GetTheme} from '../../../cli/api/graphql/admin/generated/get_theme.js' import {FindDevelopmentThemeByName} from '../../../cli/api/graphql/admin/generated/find_development_theme_by_name.js' import {MetafieldDefinitionsByOwnerType} from '../../../cli/api/graphql/admin/generated/metafield_definitions_by_owner_type.js' +import {errorHandler} from '../../../private/node/api/graphql.js' import {adminRequestDoc, supportedApiVersions} from '../api/admin.js' import {AbortError} from '../error.js' @@ -541,7 +542,9 @@ describe('themePublish', () => { } test('throws a friendly error when access is denied by a missing theme access scope', async () => { - vi.mocked(adminRequestDoc).mockRejectedValue(themeAccessDeniedError('`write_themes` access scope.')) + vi.mocked(adminRequestDoc).mockRejectedValue( + errorHandler('Admin')(themeAccessDeniedError('`write_themes` access scope.'), 'request-id'), + ) await expect(themePublish(123, session)).rejects.toThrow( 'The authenticated account or access token is missing `write_themes` access scope.', @@ -635,7 +638,9 @@ describe('themeDelete', () => { } test('throws a friendly error when access is denied by a missing theme access scope', async () => { - vi.mocked(adminRequestDoc).mockRejectedValue(themeAccessDeniedError('`write_themes` access scope.')) + vi.mocked(adminRequestDoc).mockRejectedValue( + errorHandler('Admin')(themeAccessDeniedError('`write_themes` access scope.'), 'request-id'), + ) await expect(themeDelete(123, session)).rejects.toThrow( 'The authenticated account or access token is missing `write_themes` access scope.', diff --git a/packages/cli-kit/src/public/node/themes/api.ts b/packages/cli-kit/src/public/node/themes/api.ts index 94512914ab8..4235b43e129 100644 --- a/packages/cli-kit/src/public/node/themes/api.ts +++ b/packages/cli-kit/src/public/node/themes/api.ts @@ -27,6 +27,7 @@ import {GetThemes} from '../../../cli/api/graphql/admin/generated/get_themes.js' import {GetTheme} from '../../../cli/api/graphql/admin/generated/get_theme.js' import {FindDevelopmentThemeByName} from '../../../cli/api/graphql/admin/generated/find_development_theme_by_name.js' import {OnlineStorePasswordProtection} from '../../../cli/api/graphql/admin/generated/online_store_password_protection.js' +import {GraphQLClientError} from '../../../private/node/api/headers.js' import {RequestModeInput} from '../http.js' import {adminRequestDoc, type AdminRequestOptions} from '../api/admin.js' import {AdminSession} from '../session.js' @@ -637,8 +638,6 @@ async function requestThemeAdminDoc( } function abortIfMissingThemeAccessScope(error: unknown): void { - if (!(error instanceof ClientError)) return - const requiredAccess = getThemeAccessRequirementForAccessDeniedError(error) if (!requiredAccess) return @@ -687,11 +686,10 @@ function abortIfMissingThemeAccessScope(error: unknown): void { ) } -function getThemeAccessRequirementForAccessDeniedError(error: ClientError): string | undefined { - const graphQLErrors = error.response.errors - if (!Array.isArray(graphQLErrors)) return undefined - - const accessDeniedError = graphQLErrors.find((graphQLError) => graphQLError.extensions?.code === 'ACCESS_DENIED') +function getThemeAccessRequirementForAccessDeniedError(error: unknown): string | undefined { + const accessDeniedError = themeGraphQLErrors(error).find( + (graphQLError) => graphQLError.extensions?.code === 'ACCESS_DENIED', + ) if (!accessDeniedError) return undefined const requiredAccess = accessDeniedError.extensions?.requiredAccess @@ -700,6 +698,12 @@ function getThemeAccessRequirementForAccessDeniedError(error: ClientError): stri return formatThemeAccessRequirement(requiredAccess) } +function themeGraphQLErrors(error: unknown): NonNullable { + if (error instanceof ClientError) return Array.isArray(error.response.errors) ? error.response.errors : [] + if (error instanceof GraphQLClientError) return Array.isArray(error.errors) ? error.errors : [] + return [] +} + function formatThemeAccessRequirement(requiredAccess: string): string { const requirement = requiredAccess .trim() From 3ec3de99fd8649b706ab2bf9909d5d153c715b6b Mon Sep 17 00:00:00 2001 From: Iain Wilson Date: Thu, 15 Oct 2026 15:08:04 +0200 Subject: [PATCH 3/4] Share one reader for GraphQL errors before and after mapping extractGraphQLErrors returns the GraphQL errors of a ClientError or of the GraphQLClientError that errorHandler maps it to. The theme access scope check uses it instead of its own copy. --- .../src/private/node/api/graphql.test.ts | 22 ++++++++++++++++++- .../cli-kit/src/private/node/api/graphql.ts | 6 +++++ .../cli-kit/src/public/node/themes/api.ts | 12 +++------- 3 files changed, 30 insertions(+), 10 deletions(-) diff --git a/packages/cli-kit/src/private/node/api/graphql.test.ts b/packages/cli-kit/src/private/node/api/graphql.test.ts index b886b08eba9..b4b692c8279 100644 --- a/packages/cli-kit/src/private/node/api/graphql.test.ts +++ b/packages/cli-kit/src/private/node/api/graphql.test.ts @@ -1,4 +1,4 @@ -import {debugLogRequestInfo, extractGraphQLErrorMessages, errorHandler} from './graphql.js' +import {debugLogRequestInfo, extractGraphQLErrorMessages, errorHandler, extractGraphQLErrors} from './graphql.js' import {GraphQLClientError} from './headers.js' import {AbortError} from '../../../public/node/error.js' import {runWithCommandEventsForCommand} from '../../../public/node/command-events.js' @@ -218,3 +218,23 @@ describe('errorHandler', () => { expect(handler(error)).toBe(error) }) }) + +describe('extractGraphQLErrors', () => { + const graphQLErrors = [{message: 'Challenge Required', extensions: {code: 'CHALLENGE_REQUIRED'}}] + const clientError = new ClientError( + {status: 200, headers: new Map(), errors: graphQLErrors} as unknown as ClientError['response'], + {query: 'query {}'}, + ) + + test('returns the same GraphQL errors before and after errorHandler maps the error', () => { + const mappedError = errorHandler('Admin')(clientError, 'req-123') + + expect(mappedError).toBeInstanceOf(GraphQLClientError) + expect(extractGraphQLErrors(clientError)).toEqual(graphQLErrors) + expect(extractGraphQLErrors(mappedError)).toEqual(graphQLErrors) + }) + + test('returns no GraphQL errors for an error that is not a GraphQL error', () => { + expect(extractGraphQLErrors(new AbortError('Network error'))).toEqual([]) + }) +}) diff --git a/packages/cli-kit/src/private/node/api/graphql.ts b/packages/cli-kit/src/private/node/api/graphql.ts index 9d99517f271..f01ebd9ea08 100644 --- a/packages/cli-kit/src/private/node/api/graphql.ts +++ b/packages/cli-kit/src/private/node/api/graphql.ts @@ -159,3 +159,9 @@ ${outputToken.json(error.response.errors)} } } } + +export function extractGraphQLErrors(error: unknown): NonNullable { + if (error instanceof ClientError) return Array.isArray(error.response.errors) ? error.response.errors : [] + if (error instanceof GraphQLClientError) return Array.isArray(error.errors) ? error.errors : [] + return [] +} diff --git a/packages/cli-kit/src/public/node/themes/api.ts b/packages/cli-kit/src/public/node/themes/api.ts index 4235b43e129..e9ecee5d692 100644 --- a/packages/cli-kit/src/public/node/themes/api.ts +++ b/packages/cli-kit/src/public/node/themes/api.ts @@ -27,14 +27,14 @@ import {GetThemes} from '../../../cli/api/graphql/admin/generated/get_themes.js' import {GetTheme} from '../../../cli/api/graphql/admin/generated/get_theme.js' import {FindDevelopmentThemeByName} from '../../../cli/api/graphql/admin/generated/find_development_theme_by_name.js' import {OnlineStorePasswordProtection} from '../../../cli/api/graphql/admin/generated/online_store_password_protection.js' -import {GraphQLClientError} from '../../../private/node/api/headers.js' +import {extractGraphQLErrors} from '../../../private/node/api/graphql.js' import {RequestModeInput} from '../http.js' import {adminRequestDoc, type AdminRequestOptions} from '../api/admin.js' import {AdminSession} from '../session.js' import {AbortError} from '../error.js' import {outputDebug} from '../output.js' import {recordTiming, recordEvent, recordError} from '../analytics.js' -import {ClientError, type Variables} from 'graphql-request' +import {type Variables} from 'graphql-request' import type {InlineToken, TokenItem} from '../ui.js' export type ThemeParams = Partial> @@ -687,7 +687,7 @@ function abortIfMissingThemeAccessScope(error: unknown): void { } function getThemeAccessRequirementForAccessDeniedError(error: unknown): string | undefined { - const accessDeniedError = themeGraphQLErrors(error).find( + const accessDeniedError = extractGraphQLErrors(error).find( (graphQLError) => graphQLError.extensions?.code === 'ACCESS_DENIED', ) if (!accessDeniedError) return undefined @@ -698,12 +698,6 @@ function getThemeAccessRequirementForAccessDeniedError(error: unknown): string | return formatThemeAccessRequirement(requiredAccess) } -function themeGraphQLErrors(error: unknown): NonNullable { - if (error instanceof ClientError) return Array.isArray(error.response.errors) ? error.response.errors : [] - if (error instanceof GraphQLClientError) return Array.isArray(error.errors) ? error.errors : [] - return [] -} - function formatThemeAccessRequirement(requiredAccess: string): string { const requirement = requiredAccess .trim() From 18a57f054737ee92734b3d2f08d71cda7375ddbe Mon Sep 17 00:00:00 2001 From: Iain Wilson Date: Fri, 16 Oct 2026 16:24:44 +0200 Subject: [PATCH 4/4] Cover every theme write in the missing-scope message and tests - The `write_themes` next step now names `theme publish`, `theme delete`, `theme rename` and `theme duplicate` as well as `theme push` and `theme dev`, because those commands now reach this message too. - Add the missing-scope test for `themeUpdate`, `deleteThemeAssets`, `themeDuplicate` and `bulkUploadThemeAssets`, so that all six theme mutations that use `requestThemeAdminDoc` are covered. --- .../src/public/node/themes/api.test.ts | 40 +++++++++++++++++++ .../cli-kit/src/public/node/themes/api.ts | 11 ++++- 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/packages/cli-kit/src/public/node/themes/api.test.ts b/packages/cli-kit/src/public/node/themes/api.test.ts index 365999c93c7..abf10d832f0 100644 --- a/packages/cli-kit/src/public/node/themes/api.test.ts +++ b/packages/cli-kit/src/public/node/themes/api.test.ts @@ -503,6 +503,16 @@ describe('themeUpdate', () => { expect(theme!.name).toEqual(name) expect(theme!.role).toEqual(role) }) + + test('throws a friendly error when access is denied by a missing theme access scope', async () => { + vi.mocked(adminRequestDoc).mockRejectedValue( + errorHandler('Admin')(themeAccessDeniedError('`write_themes` access scope.'), 'request-id'), + ) + + await expect(themeUpdate(123, {name: 'renamed theme'}, session)).rejects.toThrow( + 'The authenticated account or access token is missing `write_themes` access scope.', + ) + }) }) describe('themePublish', () => { @@ -608,6 +618,16 @@ describe('deleteThemeAssets', () => { }) expect(output).toEqual([{key, success: true, operation: 'DELETE'}]) }) + + test('throws a friendly error when access is denied by a missing theme access scope', async () => { + vi.mocked(adminRequestDoc).mockRejectedValue( + errorHandler('Admin')(themeAccessDeniedError('`write_themes` access scope.'), 'request-id'), + ) + + await expect(deleteThemeAssets(123, ['snippets/product-variant-picker.liquid'], session)).rejects.toThrow( + 'The authenticated account or access token is missing `write_themes` access scope.', + ) + }) }) describe('themeDelete', () => { @@ -681,6 +701,16 @@ describe('themeDuplicate', () => { }) }) } + + test('throws a friendly error when access is denied by a missing theme access scope', async () => { + vi.mocked(adminRequestDoc).mockRejectedValue( + errorHandler('Admin')(themeAccessDeniedError('`write_themes` access scope.'), 'request-id'), + ) + + await expect(themeDuplicate(123, 'duplicate theme', session)).rejects.toThrow( + 'The authenticated account or access token is missing `write_themes` access scope.', + ) + }) }) describe('request errors', () => { @@ -846,6 +876,16 @@ describe('bulkUploadThemeAssets', async () => { 'Error uploading theme files: Something went wrong', ) }) + + test('throws a friendly error when access is denied by a missing theme access scope', async () => { + vi.mocked(adminRequestDoc).mockRejectedValue( + errorHandler('Admin')(themeAccessDeniedError('`write_themes` access scope.'), 'request-id'), + ) + + await expect( + bulkUploadThemeAssets(123, [{key: 'snippets/product-variant-picker.liquid', value: 'content'}], session), + ).rejects.toThrow('The authenticated account or access token is missing `write_themes` access scope.') + }) }) describe('parseThemeFileContent', () => { diff --git a/packages/cli-kit/src/public/node/themes/api.ts b/packages/cli-kit/src/public/node/themes/api.ts index e9ecee5d692..ddbbdbc831f 100644 --- a/packages/cli-kit/src/public/node/themes/api.ts +++ b/packages/cli-kit/src/public/node/themes/api.ts @@ -662,9 +662,18 @@ function abortIfMissingThemeAccessScope(error: unknown): void { [ 'For', {command: 'theme push'}, - 'and', + {char: ','}, {command: 'theme dev'}, {char: ','}, + {command: 'theme publish'}, + {char: ','}, + {command: 'theme delete'}, + {char: ','}, + {command: 'theme rename'}, + {char: ','}, + 'and', + {command: 'theme duplicate'}, + {char: ','}, 'add both the', {command: 'read_themes'}, 'and',