diff --git a/packages/app/src/cli/models/app/loader.test.ts b/packages/app/src/cli/models/app/loader.test.ts index 41cc61e8abe..5829e7e2a4f 100644 --- a/packages/app/src/cli/models/app/loader.test.ts +++ b/packages/app/src/cli/models/app/loader.test.ts @@ -2237,10 +2237,7 @@ describe('load', () => { { webhooks: { api_version: '2024-01', - subscriptions: [ - {topics: ['orders/create'], uri: 'https://example.com'}, - {topics: ['orders/delete'], uri: 'https://example.com'}, - ], + subscriptions: [{topics: ['orders/create', 'orders/delete'], uri: 'https://example.com'}], }, name: 'for-testing-webhooks', }, @@ -3062,39 +3059,10 @@ describe('WebhooksSchema', () => { ], } - const expandedWebhookConfig: WebhooksConfig = { - api_version: '2021-07', - subscriptions: [ - { - uri: 'arn:aws:events:us-west-2::event-source/aws.partner/shopify.com/1234567890/SOME_PATH', - topics: ['products/create'], - }, - { - uri: 'arn:aws:events:us-west-2::event-source/aws.partner/shopify.com/1234567890/SOME_PATH', - topics: ['products/update'], - }, - { - uri: 'https://example.com', - topics: ['products/create'], - }, - { - uri: 'https://example.com', - topics: ['products/update'], - }, - { - uri: 'pubsub://my-project-123:my-topic', - topics: ['products/create'], - }, - { - uri: 'pubsub://my-project-123:my-topic', - topics: ['products/update'], - }, - ], - } - const {abortOrReport, parsedConfiguration} = await setupParsing({}, webhookConfig) expect(abortOrReport).not.toHaveBeenCalled() - expect(parsedConfiguration.webhooks).toMatchObject(expandedWebhookConfig) + // Without the parse-time mergeAllWebhooks transform, subscriptions preserve their raw multi-topic form + expect(parsedConfiguration.webhooks).toMatchObject(webhookConfig) }) test('throws an error if we have duplicate subscriptions in same topics array', async () => { diff --git a/packages/app/src/cli/models/extensions/specifications/app_config_webhook_schemas/webhooks_schema.ts b/packages/app/src/cli/models/extensions/specifications/app_config_webhook_schemas/webhooks_schema.ts index 98df8807a53..710b6226a53 100644 --- a/packages/app/src/cli/models/extensions/specifications/app_config_webhook_schemas/webhooks_schema.ts +++ b/packages/app/src/cli/models/extensions/specifications/app_config_webhook_schemas/webhooks_schema.ts @@ -2,7 +2,6 @@ import {WebhookSubscriptionSchema} from './webhook_subscription_schema.js' import {webhookValidator} from '../validation/app_config_webhook.js' import {WebhookSubscriptionUriValidation} from '../validation/common.js' import {SingleWebhookSubscriptionSchema} from '../app_config_webhook_subscription.js' -import {mergeAllWebhooks} from '../transform/app_config_webhook.js' import {BaseSchemaWithoutHandle} from '../../schemas.js' import {zod} from '@shopify/cli-kit/node/schema' @@ -15,10 +14,7 @@ const WebhooksConfigSchema = zod.object({ shop_deletion_url: WebhookSubscriptionUriValidation.optional(), }) .optional(), - subscriptions: zod - .array(WebhookSubscriptionSchema) - .optional() - .transform((value) => mergeAllWebhooks(value ?? [])), + subscriptions: zod.array(WebhookSubscriptionSchema).optional(), }) export type SingleWebhookSubscriptionType = zod.infer diff --git a/packages/app/src/cli/services/app/__snapshots__/config-pipeline-snapshot.test.ts.snap b/packages/app/src/cli/services/app/__snapshots__/config-pipeline-snapshot.test.ts.snap index 4117ad56e9b..3e0493323bc 100644 --- a/packages/app/src/cli/services/app/__snapshots__/config-pipeline-snapshot.test.ts.snap +++ b/packages/app/src/cli/services/app/__snapshots__/config-pipeline-snapshot.test.ts.snap @@ -1,63 +1,5 @@ // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html -exports[`Config pipeline snapshots > config round-trip (write → read → write) reorders webhook subscriptions 1`] = ` -"# Learn more about configuring your app at https://shopify.dev/docs/apps/tools/cli/configuration - -client_id = \\"12345\\" -name = \\"My Test App\\" -application_url = \\"https://myapp.example.com\\" -embedded = true - -[build] -automatically_update_urls_on_dev = true -dev_store_url = \\"test-store.myshopify.com\\" -include_config_on_deploy = true - -[access.admin] -direct_api_mode = \\"online\\" -embedded_app_direct_api_access = true - -[access_scopes] -# Learn more at https://shopify.dev/docs/apps/tools/cli/configuration#access_scopes -scopes = \\"read_products,write_orders\\" -required_scopes = [ \\"read_products\\" ] -optional_scopes = [ \\"write_orders\\" ] -use_legacy_install_flow = false - -[auth] -redirect_urls = [ - \\"https://myapp.example.com/auth/callback\\", - \\"https://myapp.example.com/auth/shopify/callback\\" -] - -[webhooks] -api_version = \\"2024-01\\" - - [[webhooks.subscriptions]] - uri = \\"/webhooks/compliance\\" - compliance_topics = [ \\"customers/data_request\\", \\"customers/redact\\" ] - - [[webhooks.subscriptions]] - topics = [ \\"orders/create\\" ] - uri = \\"/webhooks/orders\\" - - [[webhooks.subscriptions]] - topics = [ \\"products/create\\", \\"products/update\\" ] - uri = \\"/webhooks/products\\" - -[app_proxy] -url = \\"https://myapp.example.com/proxy\\" -subpath = \\"app\\" -prefix = \\"apps\\" - -[pos] -embedded = false - -[app_preferences] -url = \\"https://myapp.example.com/preferences\\" -" -`; - exports[`Config pipeline snapshots > minimal config without webhooks produces stable output 1`] = ` "# Learn more about configuring your app at https://shopify.dev/docs/apps/tools/cli/configuration @@ -69,172 +11,6 @@ embedded = true [access_scopes] # Learn more at https://shopify.dev/docs/apps/tools/cli/configuration#access_scopes scopes = \\"read_products\\" - -[auth] -redirect_urls = [ \\"https://example.com/auth/callback\\" ] -" -`; - -exports[`Config pipeline snapshots > subscription with both topics and compliance_topics on same URI splits after round-trip 1`] = ` -"# Learn more about configuring your app at https://shopify.dev/docs/apps/tools/cli/configuration - -client_id = \\"12345\\" -name = \\"My Test App\\" -application_url = \\"https://myapp.example.com\\" -embedded = true - -[build] -automatically_update_urls_on_dev = true -dev_store_url = \\"test-store.myshopify.com\\" -include_config_on_deploy = true - -[access.admin] -direct_api_mode = \\"online\\" -embedded_app_direct_api_access = true - -[access_scopes] -# Learn more at https://shopify.dev/docs/apps/tools/cli/configuration#access_scopes -scopes = \\"read_products,write_orders\\" -required_scopes = [ \\"read_products\\" ] -optional_scopes = [ \\"write_orders\\" ] -use_legacy_install_flow = false - -[auth] -redirect_urls = [ - \\"https://myapp.example.com/auth/callback\\", - \\"https://myapp.example.com/auth/shopify/callback\\" -] - -[webhooks] -api_version = \\"2024-01\\" - - [[webhooks.subscriptions]] - topics = [ \\"orders/create\\" ] - uri = \\"/webhooks\\" - compliance_topics = [ \\"customers/data_request\\", \\"customers/redact\\" ] - -[app_proxy] -url = \\"https://myapp.example.com/proxy\\" -subpath = \\"app\\" -prefix = \\"apps\\" - -[pos] -embedded = false - -[app_preferences] -url = \\"https://myapp.example.com/preferences\\" -" -`; - -exports[`Config pipeline snapshots > subscriptions with same URI but different filters stay separate through round-trip 1`] = ` -"# Learn more about configuring your app at https://shopify.dev/docs/apps/tools/cli/configuration - -client_id = \\"12345\\" -name = \\"My Test App\\" -application_url = \\"https://myapp.example.com\\" -embedded = true - -[build] -automatically_update_urls_on_dev = true -dev_store_url = \\"test-store.myshopify.com\\" -include_config_on_deploy = true - -[access.admin] -direct_api_mode = \\"online\\" -embedded_app_direct_api_access = true - -[access_scopes] -# Learn more at https://shopify.dev/docs/apps/tools/cli/configuration#access_scopes -scopes = \\"read_products,write_orders\\" -required_scopes = [ \\"read_products\\" ] -optional_scopes = [ \\"write_orders\\" ] -use_legacy_install_flow = false - -[auth] -redirect_urls = [ - \\"https://myapp.example.com/auth/callback\\", - \\"https://myapp.example.com/auth/shopify/callback\\" -] - -[webhooks] -api_version = \\"2024-01\\" - - [[webhooks.subscriptions]] - topics = [ \\"orders/create\\" ] - uri = \\"/webhooks/orders\\" - filter = \\"status:paid\\" - - [[webhooks.subscriptions]] - topics = [ \\"orders/update\\" ] - uri = \\"/webhooks/orders\\" - filter = \\"status:pending\\" - -[app_proxy] -url = \\"https://myapp.example.com/proxy\\" -subpath = \\"app\\" -prefix = \\"apps\\" - -[pos] -embedded = false - -[app_preferences] -url = \\"https://myapp.example.com/preferences\\" -" -`; - -exports[`Config pipeline snapshots > subscriptions with same URI but different include_fields stay separate through round-trip 1`] = ` -"# Learn more about configuring your app at https://shopify.dev/docs/apps/tools/cli/configuration - -client_id = \\"12345\\" -name = \\"My Test App\\" -application_url = \\"https://myapp.example.com\\" -embedded = true - -[build] -automatically_update_urls_on_dev = true -dev_store_url = \\"test-store.myshopify.com\\" -include_config_on_deploy = true - -[access.admin] -direct_api_mode = \\"online\\" -embedded_app_direct_api_access = true - -[access_scopes] -# Learn more at https://shopify.dev/docs/apps/tools/cli/configuration#access_scopes -scopes = \\"read_products,write_orders\\" -required_scopes = [ \\"read_products\\" ] -optional_scopes = [ \\"write_orders\\" ] -use_legacy_install_flow = false - -[auth] -redirect_urls = [ - \\"https://myapp.example.com/auth/callback\\", - \\"https://myapp.example.com/auth/shopify/callback\\" -] - -[webhooks] -api_version = \\"2024-01\\" - - [[webhooks.subscriptions]] - topics = [ \\"products/create\\" ] - uri = \\"/webhooks/products\\" - include_fields = [ \\"id\\", \\"title\\" ] - - [[webhooks.subscriptions]] - topics = [ \\"products/update\\" ] - uri = \\"/webhooks/products\\" - include_fields = [ \\"id\\" ] - -[app_proxy] -url = \\"https://myapp.example.com/proxy\\" -subpath = \\"app\\" -prefix = \\"apps\\" - -[pos] -embedded = false - -[app_preferences] -url = \\"https://myapp.example.com/preferences\\" " `; @@ -251,15 +27,9 @@ automatically_update_urls_on_dev = true dev_store_url = \\"test-store.myshopify.com\\" include_config_on_deploy = true -[access.admin] -direct_api_mode = \\"online\\" -embedded_app_direct_api_access = true - [access_scopes] # Learn more at https://shopify.dev/docs/apps/tools/cli/configuration#access_scopes scopes = \\"read_products,write_orders\\" -required_scopes = [ \\"read_products\\" ] -optional_scopes = [ \\"write_orders\\" ] use_legacy_install_flow = false [auth] @@ -301,7 +71,7 @@ url = \\"https://myapp.example.com/preferences\\" " `; -exports[`Config pipeline snapshots > webhook subscriptions with mixed topics and compliance topics produce stable output 2`] = ` +exports[`Config pipeline snapshots > writeAppConfigurationFile produces stable TOML output for a full config 1`] = ` "# Learn more about configuring your app at https://shopify.dev/docs/apps/tools/cli/configuration client_id = \\"12345\\" @@ -314,15 +84,9 @@ automatically_update_urls_on_dev = true dev_store_url = \\"test-store.myshopify.com\\" include_config_on_deploy = true -[access.admin] -direct_api_mode = \\"online\\" -embedded_app_direct_api_access = true - [access_scopes] # Learn more at https://shopify.dev/docs/apps/tools/cli/configuration#access_scopes scopes = \\"read_products,write_orders\\" -required_scopes = [ \\"read_products\\" ] -optional_scopes = [ \\"write_orders\\" ] use_legacy_install_flow = false [auth] @@ -335,21 +99,16 @@ redirect_urls = [ api_version = \\"2024-01\\" [[webhooks.subscriptions]] - uri = \\"/webhooks/compliance\\" - compliance_topics = [ \\"customers/data_request\\", \\"customers/redact\\", \\"shop/redact\\" ] - - [[webhooks.subscriptions]] - topics = [ \\"app/uninstalled\\" ] - uri = \\"/webhooks/app\\" + topics = [ \\"products/create\\", \\"products/update\\" ] + uri = \\"/webhooks/products\\" [[webhooks.subscriptions]] - topics = [ \\"orders/cancelled\\", \\"orders/create\\", \\"orders/updated\\" ] + topics = [ \\"orders/create\\" ] uri = \\"/webhooks/orders\\" [[webhooks.subscriptions]] - topics = [ \\"products/create\\" ] - uri = \\"/webhooks/products\\" - include_fields = [ \\"id\\", \\"title\\" ] + uri = \\"/webhooks/compliance\\" + compliance_topics = [ \\"customers/data_request\\", \\"customers/redact\\" ] [app_proxy] url = \\"https://myapp.example.com/proxy\\" diff --git a/packages/app/src/cli/services/app/config-pipeline-snapshot.test.ts b/packages/app/src/cli/services/app/config-pipeline-snapshot.test.ts index 309876880b7..04894dff671 100644 --- a/packages/app/src/cli/services/app/config-pipeline-snapshot.test.ts +++ b/packages/app/src/cli/services/app/config-pipeline-snapshot.test.ts @@ -79,39 +79,37 @@ const REALISTIC_CONFIG = { } describe('Config pipeline snapshots', () => { - test('config round-trip (write → read → write) reorders webhook subscriptions', async () => { - // KNOWN ASYMMETRY: The current pipeline does NOT round-trip cleanly. - // mergeAllWebhooks() sorts compliance subscriptions first during parse, - // so the second write has a different subscription order than the first. - // This test documents the current behavior as a snapshot. + test('config round-trip (write → read → write) preserves subscription order', async () => { + // With the mergeAllWebhooks transform extracted from the Zod schema, + // the parse pipeline no longer reorders webhook subscriptions. + // This verifies a clean round-trip: first write === second write. await inTemporaryDirectory(async (tmp) => { const filePath = joinPath(tmp, 'shopify.app.toml') const {schema, configSpecifications: specs} = await buildVersionedAppSchema() // First write await writeAppConfigurationFile(REALISTIC_CONFIG as CurrentAppConfiguration, schema, filePath) + const firstWrite = await readFile(filePath) - // Read back through the full parse pipeline (which fires Zod transforms) + // Read back through the full parse pipeline (no more Zod transforms on webhooks) const parsedConfig = await parseConfigAsCurrentApp(getAppVersionedSchema(specs), filePath) - // Second write from the parsed (transformed) config + // Second write from the parsed config await writeAppConfigurationFile(parsedConfig, schema, filePath) const secondWrite = await readFile(filePath) - // Snapshot the round-tripped output — it differs from the first write - // because compliance subscriptions get sorted first by mergeAllWebhooks - expect(secondWrite).toMatchSnapshot() + // Clean round-trip: second write matches the first + expect(secondWrite).toEqual(firstWrite) }) }) test('config round-trip stabilizes after second write', async () => { - // After the first round-trip reorders subscriptions, subsequent - // round-trips should be stable (idempotent). + // Subsequent round-trips should always be stable (idempotent). await inTemporaryDirectory(async (tmp) => { const filePath = joinPath(tmp, 'shopify.app.toml') const {schema, configSpecifications: specs} = await buildVersionedAppSchema() - // First write + read + second write (reordering happens here) + // First write + read + second write await writeAppConfigurationFile(REALISTIC_CONFIG as CurrentAppConfiguration, schema, filePath) const parsed1 = await parseConfigAsCurrentApp(getAppVersionedSchema(specs), filePath) await writeAppConfigurationFile(parsed1, schema, filePath) @@ -162,11 +160,11 @@ describe('Config pipeline snapshots', () => { const firstWrite = await readFile(filePath) expect(firstWrite).toMatchSnapshot() - // Round-trip to verify reordering behavior on the most complex fixture + // Round-trip should be clean now that webhook transforms are extracted const parsedConfig = await parseConfigAsCurrentApp(getAppVersionedSchema(specs), filePath) await writeAppConfigurationFile(parsedConfig, schema, filePath) const secondWrite = await readFile(filePath) - expect(secondWrite).toMatchSnapshot() + expect(secondWrite).toEqual(firstWrite) }) }) diff --git a/packages/app/src/cli/services/context/breakdown-extensions.ts b/packages/app/src/cli/services/context/breakdown-extensions.ts index 84413719791..3ce113e010b 100644 --- a/packages/app/src/cli/services/context/breakdown-extensions.ts +++ b/packages/app/src/cli/services/context/breakdown-extensions.ts @@ -19,6 +19,7 @@ import {ExtensionSpecification} from '../../models/extensions/specification.js' import {rewriteConfiguration} from '../app/write-app-configuration-file.js' import {AppConfigurationUsedByCli} from '../../models/extensions/specifications/types/app_config.js' import {removeTrailingSlash} from '../../models/extensions/specifications/validation/common.js' +import {mergeAllWebhooks} from '../../models/extensions/specifications/transform/app_config_webhook.js' import {throwUidMappingError} from '../../prompts/uid-mapping-error.js' import {deepCompare, deepDifference} from '@shopify/cli-kit/common/object' import {zod} from '@shopify/cli-kit/node/schema' @@ -196,30 +197,12 @@ async function resolveRemoteConfigExtensionIdentifiersBreakdown( : app.configuration ) as CurrentAppConfiguration - // modify relative path webhook subscriptions to include app url so client vs server diff checking is consistent - if (baselineConfig?.webhooks?.subscriptions?.length) { - baselineConfig.webhooks.subscriptions = baselineConfig.webhooks.subscriptions.map((subscription) => { - if (subscription.uri.startsWith('/')) { - subscription.uri = `${removeTrailingSlash(baselineConfig.application_url)}${subscription.uri}` - } - return subscription - }) - - // Sort webhook subscriptions by uri & topics in both baseline and remote config - // Ensuring that deepCompare will not fail if the order is different - baselineConfig.webhooks.subscriptions.sort((webhookA, webhookB) => { - const keyA = webhookA.uri + (webhookA.topics?.sort().join(',') ?? '') - const keyB = webhookB.uri + (webhookB.topics?.sort().join(',') ?? '') - return keyA.localeCompare(keyB) - }) - remoteConfig.webhooks?.subscriptions?.sort((webhookA, webhookB) => { - const keyA = webhookA.uri + (webhookA.topics?.sort().join(',') ?? '') - const keyB = webhookB.uri + (webhookB.topics?.sort().join(',') ?? '') - return keyA.localeCompare(keyB) - }) - } + // Build comparison-ready configs with normalized webhooks — derived copies only, + // originals are never mutated (baselineConfig may reference app.configuration). + const comparisonBaseline = normalizeWebhooksForComparison(baselineConfig) + const comparisonRemote = normalizeWebhooksForComparison(remoteConfig) - const diffFieldNames = buildDiffFieldNames(baselineConfig, remoteConfig, app.configSchema) + const diffFieldNames = buildDiffFieldNames(comparisonBaseline, comparisonRemote, app.configSchema) // List of field included in the config except the ones that only affect the CLI and are not pushed to the server // (versioned fields) @@ -439,3 +422,36 @@ function loadDashboardIdentifiersBreakdown(currentRegistrations: RemoteSource[], unchanged: toUpdate, } } + +/** + * Creates a comparison-ready copy of a config with webhook subscriptions normalized: + * - Exploded to one-topic-per-subscription (via mergeAllWebhooks) + * - Relative URIs expanded to absolute (using application_url) + * - Sorted by URI + topics for stable comparison + * + * Returns a new object — the input is never mutated. + */ +function normalizeWebhooksForComparison( + config: T, +): T { + if (!config?.webhooks?.subscriptions?.length) return config + + const subscriptions = config.webhooks.subscriptions as Parameters[0] + const applicationUrl = 'application_url' in config ? (config.application_url as string) : undefined + + // Normalize to canonical one-topic-per-subscription form, expand relative URIs + const normalized = (mergeAllWebhooks(subscriptions) ?? []).map((subscription) => + subscription.uri.startsWith('/') && applicationUrl + ? {...subscription, uri: `${removeTrailingSlash(applicationUrl)}${subscription.uri}`} + : subscription, + ) + + // Sort by URI + topics for order-independent comparison (without mutating topics arrays) + const sorted = [...normalized].sort((webhookA, webhookB) => { + const keyA = webhookA.uri + [...(webhookA.topics ?? [])].sort().join(',') + const keyB = webhookB.uri + [...(webhookB.topics ?? [])].sort().join(',') + return keyA.localeCompare(keyB) + }) + + return {...config, webhooks: {...config.webhooks, subscriptions: sorted}} +}