Skip to content

Commit b8361fc

Browse files
committed
Fix remote-only configuration specs rejected as unsupported TOML sections
1 parent 1a46d87 commit b8361fc

6 files changed

Lines changed: 211 additions & 8 deletions

File tree

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

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@ import {ExtensionInstance} from '../extensions/extension-instance.js'
1515
import {configurationFileNames, blocks} from '../../constants.js'
1616
import metadata from '../../metadata.js'
1717
import {loadLocalExtensionsSpecifications} from '../extensions/load-specifications.js'
18-
import {ExtensionSpecification} from '../extensions/specification.js'
18+
import {ExtensionSpecification, createContractBasedModuleSpecification} from '../extensions/specification.js'
19+
import {unifiedConfigurationParserFactory} from '../../utilities/json-schema.js'
1920
import {getCachedAppInfo} from '../../services/local-storage.js'
2021
import use from '../../services/app/config/use.js'
2122
import {WebhooksSchema} from '../extensions/specifications/app_config_webhook_schemas/webhooks_schema.js'
@@ -2666,6 +2667,44 @@ describe('load', () => {
26662667
expect(app).toBeDefined()
26672668
expect(app.name).toBe('for-testing')
26682669
})
2670+
2671+
test('does not flag a contract-based configuration spec section as unsupported', async () => {
2672+
// Given: A TOML config with a section that matches a contract-based config spec
2673+
const configWithContractSection = buildAppConfiguration({
2674+
extra: '[purchase_options]\nbundles = true',
2675+
})
2676+
await writeConfig(configWithContractSection)
2677+
2678+
// Create a contract-based config spec (mimicking a remote-only spec like purchase_options)
2679+
const contractSpec = createContractBasedModuleSpecification({
2680+
identifier: 'purchase_options',
2681+
uidStrategy: 'single',
2682+
experience: 'configuration',
2683+
appModuleFeatures: () => [],
2684+
})
2685+
2686+
// Attach a unified parser with the JSON schema contract, like the real flow does
2687+
const validationSchema = {
2688+
jsonSchema:
2689+
'{"type":"object","additionalProperties":false,"properties":{"bundles":{"type":"boolean","description":"Whether the app supports purchase options on bundle products"}}}',
2690+
}
2691+
const parseConfigurationObject = await unifiedConfigurationParserFactory(
2692+
contractSpec as any,
2693+
validationSchema,
2694+
'strip',
2695+
)
2696+
const specsWithContract = [...specifications, {...contractSpec, parseConfigurationObject}]
2697+
2698+
// When: The app is loaded with the contract-based spec included
2699+
const app = await loadApp({
2700+
directory: tmpDir,
2701+
specifications: specsWithContract,
2702+
userProvidedConfigName: undefined,
2703+
})
2704+
2705+
// Then: No "unsupported section" error for purchase_options
2706+
expect(app.errors.isEmpty()).toBe(true)
2707+
})
26692708
})
26702709

26712710
describe('getAppConfigurationFileName', () => {

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

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -787,8 +787,20 @@ class AppLoader<TConfig extends CurrentAppConfiguration, TModuleSpec extends Ext
787787
return [null, [] as string[]] as const
788788
}
789789
const specConfiguration = specResult.data
790+
const claimedKeys = Object.keys(specConfiguration)
791+
792+
// For contract-based configuration specs (remote-only specs using zod.any()), the
793+
// parsed result may not include the spec identifier as a key because the JSON schema
794+
// strip operates on the section contents rather than the section wrapper. Ensure the
795+
// spec identifier is claimed if it exists in the app configuration.
796+
if (
797+
!claimedKeys.includes(specification.identifier) &&
798+
specification.identifier in (appConfiguration as Record<string, unknown>)
799+
) {
800+
claimedKeys.push(specification.identifier)
801+
}
790802

791-
if (Object.keys(specConfiguration).length === 0) return [null, Object.keys(specConfiguration)] as const
803+
if (Object.keys(specConfiguration).length === 0) return [null, claimedKeys] as const
792804

793805
const instance = await this.createExtensionInstance(
794806
specification.identifier,
@@ -802,7 +814,7 @@ class AppLoader<TConfig extends CurrentAppConfiguration, TModuleSpec extends Ext
802814
extensionInstance,
803815
),
804816
)
805-
return [instance, Object.keys(specConfiguration)] as const
817+
return [instance, claimedKeys] as const
806818
}),
807819
)
808820

packages/app/src/cli/models/extensions/specification.integration.test.ts

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {BaseSchema} from './schemas.js'
99
import {ClientSteps} from '../../services/build/client-steps.js'
1010
import {AppSchema} from '../app/app.js'
1111
import {describe, test, expect, beforeAll} from 'vitest'
12+
import {zod} from '@shopify/cli-kit/node/schema'
1213

1314
// If the AppSchema is not instanced, the dynamic loading of loadLocalExtensionsSpecifications is not working
1415
beforeAll(() => {
@@ -126,6 +127,63 @@ describe('createConfigExtensionSpecification', () => {
126127
})
127128
})
128129

130+
describe('contributeToAppConfigurationSchema', () => {
131+
test('contract-based config spec with zod.any() contributes its identifier as a known key', () => {
132+
// Given: A contract-based config spec with experience: 'configuration' and zod.any() schema
133+
const spec = createContractBasedModuleSpecification({
134+
identifier: 'purchase_options',
135+
uidStrategy: 'single',
136+
experience: 'configuration',
137+
appModuleFeatures: () => [],
138+
})
139+
140+
// When: It contributes to the app configuration schema
141+
const baseSchema = zod.object({client_id: zod.string()})
142+
const result = spec.contributeToAppConfigurationSchema(baseSchema)
143+
144+
// Then: The resulting schema should accept the identifier as a valid key
145+
const parsed = result.safeParse({client_id: 'test', purchase_options: {bundles: true}})
146+
expect(parsed.success).toBe(true)
147+
})
148+
149+
test('contract-based config spec with experience: extension does not contribute to schema', () => {
150+
// Given: A contract-based spec with experience: 'extension' (not 'configuration')
151+
const spec = createContractBasedModuleSpecification({
152+
identifier: 'some_extension',
153+
uidStrategy: 'uuid',
154+
experience: 'extension',
155+
appModuleFeatures: () => [],
156+
})
157+
158+
// When: It tries to contribute to the app configuration schema
159+
const baseSchema = zod.object({client_id: zod.string()}).strict()
160+
const result = spec.contributeToAppConfigurationSchema(baseSchema)
161+
162+
// Then: The schema should be unchanged — 'some_extension' is not accepted
163+
const parsed = result.safeParse({client_id: 'test', some_extension: {enabled: true}})
164+
expect(parsed.success).toBe(false)
165+
})
166+
167+
test('config spec with explicit zod schema merges its schema shape', () => {
168+
// Given: A locally-defined config spec with an explicit zod schema
169+
const spec = createConfigExtensionSpecification({
170+
identifier: 'test_config',
171+
schema: BaseSchema.extend({
172+
my_section: zod.object({enabled: zod.boolean()}).optional(),
173+
}),
174+
transformConfig: {},
175+
})
176+
177+
// When: It contributes to the app configuration schema
178+
const baseSchema = zod.object({client_id: zod.string()})
179+
const result = spec.contributeToAppConfigurationSchema(baseSchema)
180+
181+
// Then: The schema should accept 'my_section' as a valid key
182+
const parsed = result.safeParse({client_id: 'test', my_section: {enabled: true}})
183+
expect(parsed.success).toBe(true)
184+
})
185+
})
186+
129187
describe('configWithoutFirstClassFields', () => {
130188
test('removes the first class fields from the config', () => {
131189
// When

packages/app/src/cli/models/extensions/specification.ts

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -226,13 +226,20 @@ export function createExtensionSpecification<TConfiguration extends BaseConfigTy
226226
// This filters out webhook subscription specifications from contributing to the app configuration schema
227227
const hasSingleUidStrategy = merged.uidStrategy === 'single'
228228

229-
const canContribute = isConfig && hasSchema && hasSingleUidStrategy
230-
if (!canContribute) {
231-
// no change
229+
if (!isConfig || !hasSingleUidStrategy) {
232230
return appConfigSchema
233231
}
232+
233+
if (hasSchema) {
234+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
235+
return (appConfigSchema as any).merge(merged.schema)
236+
}
237+
238+
// For contract-based config specs (e.g. remote-only specs with zod.any()), contribute
239+
// the spec identifier as a known top-level key so it isn't flagged as unsupported.
240+
const contractSchema = zod.object({[merged.identifier]: zod.any().optional()})
234241
// eslint-disable-next-line @typescript-eslint/no-explicit-any
235-
return (appConfigSchema as any).merge(merged.schema)
242+
return (appConfigSchema as any).merge(contractSchema)
236243
},
237244
parseConfigurationObject: (configurationObject: unknown) => {
238245
const parseResult = merged.schema.safeParse(configurationObject)

packages/app/src/cli/utilities/json-schema.test.ts

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,82 @@ describe('unifiedConfigurationParserFactory', () => {
152152
expect(priceError).toBeDefined()
153153
})
154154

155+
test('scopes validation to the section contents for configuration specs when identifier matches a key in the config', async () => {
156+
// Given: A contract-based config spec (like purchase_options) where the JSON schema
157+
// describes the section contents ({bundles: boolean}), and the parser receives the
158+
// entire app config with the section nested under the identifier key.
159+
const merged = {
160+
identifier: 'purchase_options',
161+
parseConfigurationObject: mockParseConfigurationObject,
162+
validationSchema: {
163+
jsonSchema:
164+
'{"type":"object","additionalProperties":false,"properties":{"bundles":{"type":"boolean","description":"Whether the app supports purchase options on bundle products"}}}',
165+
},
166+
}
167+
168+
// When: The parser receives a full app config where "purchase_options" is a nested section
169+
const parser = await unifiedConfigurationParserFactory(merged as any, merged.validationSchema, 'strip')
170+
const result = parser({
171+
client_id: 'test-id',
172+
name: 'my-app',
173+
purchase_options: {bundles: true},
174+
webhooks: {api_version: '2024-01'},
175+
})
176+
177+
// Then: The parser should scope to the section, validate {bundles: true} against the
178+
// schema, and return only the section contents — not an empty object.
179+
expect(result.state).toBe('ok')
180+
expect(result.data).toEqual({bundles: true})
181+
})
182+
183+
test('returns empty object when config spec identifier is not present in the config', async () => {
184+
// Given: Same contract-based config spec, but the TOML doesn't include the section
185+
const merged = {
186+
identifier: 'purchase_options',
187+
parseConfigurationObject: mockParseConfigurationObject,
188+
validationSchema: {
189+
jsonSchema:
190+
'{"type":"object","additionalProperties":false,"properties":{"bundles":{"type":"boolean","description":"Whether the app supports purchase options on bundle products"}}}',
191+
},
192+
}
193+
194+
// When: The config does NOT contain "purchase_options"
195+
const parser = await unifiedConfigurationParserFactory(merged as any, merged.validationSchema, 'strip')
196+
const result = parser({
197+
client_id: 'test-id',
198+
name: 'my-app',
199+
webhooks: {api_version: '2024-01'},
200+
})
201+
202+
// Then: No scoping happens; strip removes all non-matching keys, leaving {}
203+
expect(result.state).toBe('ok')
204+
expect(result.data).toEqual({})
205+
})
206+
207+
test('validates section contents against the JSON schema when scoped', async () => {
208+
// Given: A config spec with a required field in the JSON schema
209+
const merged = {
210+
identifier: 'my_config',
211+
parseConfigurationObject: mockParseConfigurationObject,
212+
validationSchema: {
213+
jsonSchema:
214+
'{"type":"object","additionalProperties":false,"properties":{"enabled":{"type":"boolean"}},"required":["enabled"]}',
215+
},
216+
}
217+
218+
// When: The section exists but is missing the required field
219+
const parser = await unifiedConfigurationParserFactory(merged as any, merged.validationSchema, 'strip')
220+
const result = parser({
221+
client_id: 'test-id',
222+
my_config: {not_enabled: true},
223+
})
224+
225+
// Then: Validation should fail because "enabled" is required
226+
expect(result.state).toBe('error')
227+
expect(result.errors).toBeDefined()
228+
expect(result.errors!.length).toBeGreaterThan(0)
229+
})
230+
155231
test('adds base properties to the JSON schema', async () => {
156232
// Given
157233
const merged = {

packages/app/src/cli/utilities/json-schema.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,18 @@ export async function unifiedConfigurationParserFactory(
4949

5050
// Then, even if this failed, we try to validate against the contract.
5151
const zodValidatedData = zodParse.state === 'ok' ? zodParse.data : undefined
52-
const subjectForAjv = zodValidatedData ?? (config as JsonMapType)
52+
let subjectForAjv = zodValidatedData ?? (config as JsonMapType)
53+
54+
// For configuration specs with zod.any() schemas (contract-based/remote-only specs), the
55+
// zod parse returns the entire app config. The JSON schema contract describes the section
56+
// contents (e.g. {bundles: boolean} for purchase_options), not the whole config. Scope the
57+
// data to the section identified by the spec identifier.
58+
if (handleInvalidAdditionalProperties === 'strip' && extensionIdentifier in subjectForAjv) {
59+
const sectionData = subjectForAjv[extensionIdentifier as keyof typeof subjectForAjv]
60+
if (sectionData !== null && typeof sectionData === 'object' && !Array.isArray(sectionData)) {
61+
subjectForAjv = sectionData as JsonMapType
62+
}
63+
}
5364

5465
const jsonSchemaParse = jsonSchemaValidate(
5566
subjectForAjv,

0 commit comments

Comments
 (0)