Skip to content

Commit 7a9cdb3

Browse files
ryancbahanclaude
andcommitted
Extract mergeAllWebhooks transform from Zod schema into breakdown-extensions
Remove the parse-time mergeAllWebhooks transform from WebhooksConfigSchema so that parsed webhook subscriptions preserve their raw TOML form (multi-topic arrays stay intact). This fixes the round-trip asymmetry where compliance subscriptions got reordered to the front on re-parse. The only consumer that needs canonical one-topic-per-subscription form is breakdown-extensions.ts (for comparing local vs remote config), so the mergeAllWebhooks normalization is applied there before diffing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 65c98c2 commit 7a9cdb3

5 files changed

Lines changed: 23 additions & 104 deletions

File tree

packages/app/src/cli/models/app/loader.test.ts

Lines changed: 3 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -2172,10 +2172,7 @@ redirect_urls = [ "https://example.com/api/auth" ]
21722172
{
21732173
webhooks: {
21742174
api_version: '2024-01',
2175-
subscriptions: [
2176-
{topics: ['orders/create'], uri: 'https://example.com'},
2177-
{topics: ['orders/delete'], uri: 'https://example.com'},
2178-
],
2175+
subscriptions: [{topics: ['orders/create', 'orders/delete'], uri: 'https://example.com'}],
21792176
},
21802177
name: 'for-testing-webhooks',
21812178
},
@@ -3084,39 +3081,10 @@ describe('WebhooksSchema', () => {
30843081
],
30853082
}
30863083

3087-
const expandedWebhookConfig: WebhooksConfig = {
3088-
api_version: '2021-07',
3089-
subscriptions: [
3090-
{
3091-
uri: 'arn:aws:events:us-west-2::event-source/aws.partner/shopify.com/1234567890/SOME_PATH',
3092-
topics: ['products/create'],
3093-
},
3094-
{
3095-
uri: 'arn:aws:events:us-west-2::event-source/aws.partner/shopify.com/1234567890/SOME_PATH',
3096-
topics: ['products/update'],
3097-
},
3098-
{
3099-
uri: 'https://example.com',
3100-
topics: ['products/create'],
3101-
},
3102-
{
3103-
uri: 'https://example.com',
3104-
topics: ['products/update'],
3105-
},
3106-
{
3107-
uri: 'pubsub://my-project-123:my-topic',
3108-
topics: ['products/create'],
3109-
},
3110-
{
3111-
uri: 'pubsub://my-project-123:my-topic',
3112-
topics: ['products/update'],
3113-
},
3114-
],
3115-
}
3116-
31173084
const {abortOrReport, parsedConfiguration} = await setupParsing({}, webhookConfig)
31183085
expect(abortOrReport).not.toHaveBeenCalled()
3119-
expect(parsedConfiguration.webhooks).toMatchObject(expandedWebhookConfig)
3086+
// Without the parse-time mergeAllWebhooks transform, subscriptions preserve their raw multi-topic form
3087+
expect(parsedConfiguration.webhooks).toMatchObject(webhookConfig)
31203088
})
31213089

31223090
test('throws an error if we have duplicate subscriptions in same topics array', async () => {

packages/app/src/cli/models/extensions/specifications/app_config_webhook_schemas/webhooks_schema.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import {WebhookSubscriptionSchema} from './webhook_subscription_schema.js'
22
import {webhookValidator} from '../validation/app_config_webhook.js'
33
import {WebhookSubscriptionUriValidation} from '../validation/common.js'
44
import {SingleWebhookSubscriptionSchema} from '../app_config_webhook_subscription.js'
5-
import {mergeAllWebhooks} from '../transform/app_config_webhook.js'
65
import {BaseSchemaWithoutHandle} from '../../schemas.js'
76
import {zod} from '@shopify/cli-kit/node/schema'
87

@@ -15,10 +14,7 @@ const WebhooksConfigSchema = zod.object({
1514
shop_deletion_url: WebhookSubscriptionUriValidation.optional(),
1615
})
1716
.optional(),
18-
subscriptions: zod
19-
.array(WebhookSubscriptionSchema)
20-
.optional()
21-
.transform((value) => mergeAllWebhooks(value ?? [])),
17+
subscriptions: zod.array(WebhookSubscriptionSchema).optional(),
2218
})
2319

2420
export type SingleWebhookSubscriptionType = zod.infer<typeof SingleWebhookSubscriptionSchema>

packages/app/src/cli/services/app/__snapshots__/config-pipeline-snapshot.test.ts.snap

Lines changed: 0 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1,57 +1,5 @@
11
// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
22

3-
exports[`Config pipeline snapshots > config round-trip (write → read → write) reorders webhook subscriptions 1`] = `
4-
"# Learn more about configuring your app at https://shopify.dev/docs/apps/tools/cli/configuration
5-
6-
client_id = \\"12345\\"
7-
name = \\"My Test App\\"
8-
application_url = \\"https://myapp.example.com\\"
9-
embedded = true
10-
11-
[build]
12-
automatically_update_urls_on_dev = true
13-
dev_store_url = \\"test-store.myshopify.com\\"
14-
include_config_on_deploy = true
15-
16-
[access_scopes]
17-
# Learn more at https://shopify.dev/docs/apps/tools/cli/configuration#access_scopes
18-
scopes = \\"read_products,write_orders\\"
19-
use_legacy_install_flow = false
20-
21-
[auth]
22-
redirect_urls = [
23-
\\"https://myapp.example.com/auth/callback\\",
24-
\\"https://myapp.example.com/auth/shopify/callback\\"
25-
]
26-
27-
[webhooks]
28-
api_version = \\"2024-01\\"
29-
30-
[[webhooks.subscriptions]]
31-
uri = \\"/webhooks/compliance\\"
32-
compliance_topics = [ \\"customers/data_request\\", \\"customers/redact\\" ]
33-
34-
[[webhooks.subscriptions]]
35-
topics = [ \\"orders/create\\" ]
36-
uri = \\"/webhooks/orders\\"
37-
38-
[[webhooks.subscriptions]]
39-
topics = [ \\"products/create\\", \\"products/update\\" ]
40-
uri = \\"/webhooks/products\\"
41-
42-
[app_proxy]
43-
url = \\"https://myapp.example.com/proxy\\"
44-
subpath = \\"app\\"
45-
prefix = \\"apps\\"
46-
47-
[pos]
48-
embedded = false
49-
50-
[app_preferences]
51-
url = \\"https://myapp.example.com/preferences\\"
52-
"
53-
`;
54-
553
exports[`Config pipeline snapshots > minimal config without webhooks produces stable output 1`] = `
564
"# Learn more about configuring your app at https://shopify.dev/docs/apps/tools/cli/configuration
575

packages/app/src/cli/services/app/config-pipeline-snapshot.test.ts

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -77,30 +77,27 @@ describe('Config pipeline snapshots', () => {
7777
})
7878
})
7979

80-
test('config round-trip (write → read → write) reorders webhook subscriptions', async () => {
81-
// KNOWN ASYMMETRY: The current pipeline does NOT round-trip cleanly.
82-
// mergeAllWebhooks() sorts compliance subscriptions first during parse,
83-
// so the second write has a different subscription order than the first.
84-
// This test documents the current behavior as a snapshot.
85-
// When the webhook transform is extracted from the schema (Phase 2),
86-
// this test should be updated to verify clean round-trips.
80+
test('config round-trip (write → read → write) preserves subscription order', async () => {
81+
// With the mergeAllWebhooks transform extracted from the Zod schema,
82+
// the parse pipeline no longer reorders webhook subscriptions.
83+
// This verifies a clean round-trip: first write === second write.
8784
await inTemporaryDirectory(async (tmp) => {
8885
const filePath = joinPath(tmp, 'shopify.app.toml')
8986
const {schema, configSpecifications: specs} = await buildVersionedAppSchema()
9087

9188
// First write
9289
await writeAppConfigurationFile(REALISTIC_CONFIG as CurrentAppConfiguration, schema, filePath)
90+
const firstWrite = await readFile(filePath)
9391

94-
// Read back through the full parse pipeline (which fires Zod transforms)
92+
// Read back through the full parse pipeline (no more Zod transforms on webhooks)
9593
const parsedConfig = await parseConfigurationFile(getAppVersionedSchema(specs), filePath)
9694

97-
// Second write from the parsed (transformed) config
95+
// Second write from the parsed config
9896
await writeAppConfigurationFile(parsedConfig, schema, filePath)
9997
const secondWrite = await readFile(filePath)
10098

101-
// Snapshot the round-tripped output — it differs from the first write
102-
// because compliance subscriptions get sorted first by mergeAllWebhooks
103-
expect(secondWrite).toMatchSnapshot()
99+
// Clean round-trip: second write matches the first
100+
expect(secondWrite).toEqual(firstWrite)
104101
})
105102
})
106103

packages/app/src/cli/services/context/breakdown-extensions.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {ExtensionSpecification} from '../../models/extensions/specification.js'
1919
import {rewriteConfiguration} from '../app/write-app-configuration-file.js'
2020
import {AppConfigurationUsedByCli} from '../../models/extensions/specifications/types/app_config.js'
2121
import {removeTrailingSlash} from '../../models/extensions/specifications/validation/common.js'
22+
import {mergeAllWebhooks} from '../../models/extensions/specifications/transform/app_config_webhook.js'
2223
import {throwUidMappingError} from '../../prompts/uid-mapping-error.js'
2324
import {deepCompare, deepDifference} from '@shopify/cli-kit/common/object'
2425
import {zod} from '@shopify/cli-kit/node/schema'
@@ -196,6 +197,15 @@ async function resolveRemoteConfigExtensionIdentifiersBreakdown(
196197
: app.configuration
197198
) as CurrentAppConfiguration
198199

200+
// Normalize webhook subscriptions to canonical one-topic-per-subscription form for comparison.
201+
// This ensures local (raw multi-topic) and remote (exploded single-topic) subscriptions are comparable.
202+
if (baselineConfig?.webhooks?.subscriptions?.length) {
203+
baselineConfig.webhooks.subscriptions = mergeAllWebhooks(baselineConfig.webhooks.subscriptions) ?? []
204+
}
205+
if (remoteConfig?.webhooks?.subscriptions?.length) {
206+
remoteConfig.webhooks.subscriptions = mergeAllWebhooks(remoteConfig.webhooks.subscriptions) ?? []
207+
}
208+
199209
// modify relative path webhook subscriptions to include app url so client vs server diff checking is consistent
200210
if (baselineConfig?.webhooks?.subscriptions?.length) {
201211
baselineConfig.webhooks.subscriptions = baselineConfig.webhooks.subscriptions.map((subscription) => {

0 commit comments

Comments
 (0)