From 9dca5eb58b78dec8c5f218a457b268d1efced90b Mon Sep 17 00:00:00 2001 From: River Date: Wed, 3 Jun 2026 07:43:33 +0000 Subject: [PATCH 1/3] Scope cached signed upload URL per app and command The App Management client caches generateSignedUploadUrl responses for 59 minutes using only the organization ID as the cache key. When two `shopify app deploy --path=...` processes run concurrently for different apps in the same organization, both retrieve the same cached signed upload URL and upload their bundles to the same destination. App Management then reads whichever upload arrived last when each appVersionCreate is called, so apps end up with the wrong bundles. Add the app's apiKey and the current command id to cacheExtraKey so the cache remains useful for repeated dev hot-reload uploads of the same app, while concurrent deploys of different apps in the same org no longer share a signed upload destination. Fixes #7696. Co-authored-by: Isaac Roldan Co-authored-by: Mitch Lillie --- .../river-fix-parallel-deploy-cache-key.md | 5 +++ .../app-management-client.test.ts | 40 ++++++++++++++++++- .../app-management-client.ts | 15 ++++++- 3 files changed, 57 insertions(+), 3 deletions(-) create mode 100644 .changeset/river-fix-parallel-deploy-cache-key.md diff --git a/.changeset/river-fix-parallel-deploy-cache-key.md b/.changeset/river-fix-parallel-deploy-cache-key.md new file mode 100644 index 0000000000..5e2905d6d6 --- /dev/null +++ b/.changeset/river-fix-parallel-deploy-cache-key.md @@ -0,0 +1,5 @@ +--- +'@shopify/app': patch +--- + +Fix `app deploy` cross-contaminating bundles between concurrent deploys of different apps in the same organization. The App Management `generateSignedUploadUrl` response was cached for 59 minutes using only the organization ID, so parallel `shopify app deploy --path=...` invocations would receive the same signed upload URL and overwrite each other's bundles. The cache key now also includes the app's API key and the current command, preserving the cache's benefit for `dev` hot-reload while keeping per-app deploys isolated. diff --git a/packages/app/src/cli/utilities/developer-platform-client/app-management-client.test.ts b/packages/app/src/cli/utilities/developer-platform-client/app-management-client.test.ts index 0ce583e51b..e78dc08078 100644 --- a/packages/app/src/cli/utilities/developer-platform-client/app-management-client.test.ts +++ b/packages/app/src/cli/utilities/developer-platform-client/app-management-client.test.ts @@ -50,6 +50,7 @@ import { businessPlatformRequestDoc, } from '@shopify/cli-kit/node/api/business-platform' import {appManagementRequestDoc} from '@shopify/cli-kit/node/api/app-management' +import {setCurrentCommandId} from '@shopify/cli-kit/node/global-context' import {BugError} from '@shopify/cli-kit/node/error' import {randomUUID} from '@shopify/cli-kit/node/crypto' import {webhooksRequestDoc} from '@shopify/cli-kit/node/api/webhooks' @@ -1212,7 +1213,7 @@ describe('deploy', () => { describe('AppManagementClient', () => { describe('generateSignedUploadUrl', () => { - test('passes Brotli format for uploads', async () => { + test('passes Brotli format for uploads and scopes the cache key to the app and command', async () => { // Given const client = AppManagementClient.getInstance() const mockResponse = { @@ -1224,6 +1225,7 @@ describe('AppManagementClient', () => { // Mock the app management request vi.mocked(appManagementRequestDoc).mockResolvedValueOnce(mockResponse) + setCurrentCommandId('app:deploy') const app: MinimalAppIdentifiers = { apiKey: 'test-api-key', @@ -1249,9 +1251,45 @@ describe('AppManagementClient', () => { }, cacheOptions: { cacheTTL: {minutes: 59}, + cacheExtraKey: 'test-api-key-app:deploy', }, }) }) + + test('produces different cache keys for different apps in the same organization', async () => { + // Given + const client = AppManagementClient.getInstance() + const mockResponse = { + appRequestSourceUploadUrl: { + sourceUploadUrl: 'https://example.com/upload-url', + userErrors: [], + }, + } + vi.mocked(appManagementRequestDoc).mockResolvedValue(mockResponse) + setCurrentCommandId('app:deploy') + + const appOne: MinimalAppIdentifiers = { + apiKey: 'app-one-api-key', + organizationId: '213141', + id: 'app-one-id', + } + const appTwo: MinimalAppIdentifiers = { + apiKey: 'app-two-api-key', + organizationId: '213141', + id: 'app-two-id', + } + + // When + client.token = () => Promise.resolve('token') + await client.generateSignedUploadUrl(appOne) + await client.generateSignedUploadUrl(appTwo) + + // Then + const calls = vi.mocked(appManagementRequestDoc).mock.calls.map( + ([options]) => (options as {cacheOptions?: {cacheExtraKey?: string}}).cacheOptions?.cacheExtraKey, + ) + expect(calls).toEqual(['app-one-api-key-app:deploy', 'app-two-api-key-app:deploy']) + }) }) describe('bundleFormat', () => { diff --git a/packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts b/packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts index 5d8ac4d1c8..9fa44b3cbe 100644 --- a/packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts +++ b/packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts @@ -161,6 +161,7 @@ import { import {CLI_KIT_VERSION} from '@shopify/cli-kit/common/version' import {versionSatisfies} from '@shopify/cli-kit/node/node-package-manager' import {outputDebug} from '@shopify/cli-kit/node/output' +import {getCurrentCommandId} from '@shopify/cli-kit/node/global-context' import {developerDashboardFqdn, normalizeStoreFqdn} from '@shopify/cli-kit/node/context/fqdn' import {TokenItem} from '@shopify/cli-kit/node/ui' import {functionsRequestDoc, FunctionsRequestOptions} from '@shopify/cli-kit/node/api/functions' @@ -718,7 +719,7 @@ export class AppManagementClient implements DeveloperPlatformClient { } } - async generateSignedUploadUrl({organizationId}: MinimalAppIdentifiers): Promise { + async generateSignedUploadUrl({apiKey, organizationId}: MinimalAppIdentifiers): Promise { const variables = { sourceExtension: 'BR' as SourceExtension, organizationId: gidFromOrganizationIdForShopify(organizationId), @@ -726,7 +727,17 @@ export class AppManagementClient implements DeveloperPlatformClient { const result = await this.appManagementRequest({ query: CreateAssetUrl, variables, - cacheOptions: {cacheTTL: {minutes: 59}}, + cacheOptions: { + cacheTTL: {minutes: 59}, + // Scope the cached signed upload URL to this specific app and command + // run. Without this, concurrent deploys of different apps in the same + // organization can share a single cached signed upload URL — each + // process uploads its own bundle to the same destination and whichever + // upload App Management reads last gets used for every concurrent + // appVersionCreate, cross-contaminating app bundles. + // See https://github.com/Shopify/cli/issues/7696. + cacheExtraKey: `${apiKey}-${getCurrentCommandId()}`, + }, }) return { assetUrl: result.appRequestSourceUploadUrl.sourceUploadUrl, From 867fb076bb2d23cd98c8192f0226b62efb0ed20c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Wed, 3 Jun 2026 09:52:02 +0200 Subject: [PATCH 2/3] Simplify deploy cache changeset wording --- .changeset/river-fix-parallel-deploy-cache-key.md | 2 +- .../developer-platform-client/app-management-client.ts | 8 +------- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/.changeset/river-fix-parallel-deploy-cache-key.md b/.changeset/river-fix-parallel-deploy-cache-key.md index 5e2905d6d6..4717e0d88a 100644 --- a/.changeset/river-fix-parallel-deploy-cache-key.md +++ b/.changeset/river-fix-parallel-deploy-cache-key.md @@ -2,4 +2,4 @@ '@shopify/app': patch --- -Fix `app deploy` cross-contaminating bundles between concurrent deploys of different apps in the same organization. The App Management `generateSignedUploadUrl` response was cached for 59 minutes using only the organization ID, so parallel `shopify app deploy --path=...` invocations would receive the same signed upload URL and overwrite each other's bundles. The cache key now also includes the app's API key and the current command, preserving the cache's benefit for `dev` hot-reload while keeping per-app deploys isolated. +Fix `app deploy` reusing signed upload URLs across apps in the same organization. diff --git a/packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts b/packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts index 9fa44b3cbe..5e6f345f60 100644 --- a/packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts +++ b/packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts @@ -729,13 +729,7 @@ export class AppManagementClient implements DeveloperPlatformClient { variables, cacheOptions: { cacheTTL: {minutes: 59}, - // Scope the cached signed upload URL to this specific app and command - // run. Without this, concurrent deploys of different apps in the same - // organization can share a single cached signed upload URL — each - // process uploads its own bundle to the same destination and whichever - // upload App Management reads last gets used for every concurrent - // appVersionCreate, cross-contaminating app bundles. - // See https://github.com/Shopify/cli/issues/7696. + // Avoid reusing signed upload URLs across apps or commands. cacheExtraKey: `${apiKey}-${getCurrentCommandId()}`, }, }) From 2db0fb56e690426e34ed32b5ef830d5e5fe2824e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Wed, 3 Jun 2026 10:01:51 +0200 Subject: [PATCH 3/3] Address deploy cache PR feedback --- .../app-management-client.test.ts | 23 +++++++++++-------- .../app-management-client.ts | 4 ++-- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/packages/app/src/cli/utilities/developer-platform-client/app-management-client.test.ts b/packages/app/src/cli/utilities/developer-platform-client/app-management-client.test.ts index e78dc08078..eb58a9f893 100644 --- a/packages/app/src/cli/utilities/developer-platform-client/app-management-client.test.ts +++ b/packages/app/src/cli/utilities/developer-platform-client/app-management-client.test.ts @@ -62,8 +62,9 @@ vi.mock('@shopify/organizations') vi.mock('@shopify/cli-kit/node/api/webhooks') beforeEach(() => { - // Reset the singleton instance before each test AppManagementClient.resetInstance() + setCurrentCommandId('') + delete process.env.COMMAND_RUN_ID }) const extensionA = await testUIExtension({uid: 'extension-a-uuid'}) @@ -1213,7 +1214,7 @@ describe('deploy', () => { describe('AppManagementClient', () => { describe('generateSignedUploadUrl', () => { - test('passes Brotli format for uploads and scopes the cache key to the app and command', async () => { + test('passes Brotli format for uploads and scopes the cache key to the app and command run', async () => { // Given const client = AppManagementClient.getInstance() const mockResponse = { @@ -1225,7 +1226,7 @@ describe('AppManagementClient', () => { // Mock the app management request vi.mocked(appManagementRequestDoc).mockResolvedValueOnce(mockResponse) - setCurrentCommandId('app:deploy') + process.env.COMMAND_RUN_ID = 'test-run-id' const app: MinimalAppIdentifiers = { apiKey: 'test-api-key', @@ -1251,7 +1252,7 @@ describe('AppManagementClient', () => { }, cacheOptions: { cacheTTL: {minutes: 59}, - cacheExtraKey: 'test-api-key-app:deploy', + cacheExtraKey: 'test-api-key-test-run-id', }, }) }) @@ -1265,8 +1266,8 @@ describe('AppManagementClient', () => { userErrors: [], }, } - vi.mocked(appManagementRequestDoc).mockResolvedValue(mockResponse) - setCurrentCommandId('app:deploy') + vi.mocked(appManagementRequestDoc).mockResolvedValueOnce(mockResponse).mockResolvedValueOnce(mockResponse) + process.env.COMMAND_RUN_ID = 'test-run-id' const appOne: MinimalAppIdentifiers = { apiKey: 'app-one-api-key', @@ -1285,10 +1286,12 @@ describe('AppManagementClient', () => { await client.generateSignedUploadUrl(appTwo) // Then - const calls = vi.mocked(appManagementRequestDoc).mock.calls.map( - ([options]) => (options as {cacheOptions?: {cacheExtraKey?: string}}).cacheOptions?.cacheExtraKey, - ) - expect(calls).toEqual(['app-one-api-key-app:deploy', 'app-two-api-key-app:deploy']) + const calls = vi + .mocked(appManagementRequestDoc) + .mock.calls.map( + ([options]) => (options as {cacheOptions?: {cacheExtraKey?: string}}).cacheOptions?.cacheExtraKey, + ) + expect(calls).toEqual(['app-one-api-key-test-run-id', 'app-two-api-key-test-run-id']) }) }) diff --git a/packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts b/packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts index 5e6f345f60..5f48c7ea74 100644 --- a/packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts +++ b/packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts @@ -729,8 +729,8 @@ export class AppManagementClient implements DeveloperPlatformClient { variables, cacheOptions: { cacheTTL: {minutes: 59}, - // Avoid reusing signed upload URLs across apps or commands. - cacheExtraKey: `${apiKey}-${getCurrentCommandId()}`, + // Avoid reusing signed upload URLs across apps or command runs. + cacheExtraKey: `${apiKey}-${process.env.COMMAND_RUN_ID ?? getCurrentCommandId()}`, }, }) return {