Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 40 additions & 1 deletion packages/app/src/cli/models/app/loader.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ import {ExtensionInstance} from '../extensions/extension-instance.js'
import {configurationFileNames, blocks} from '../../constants.js'
import metadata from '../../metadata.js'
import {loadLocalExtensionsSpecifications} from '../extensions/load-specifications.js'
import {ExtensionSpecification} from '../extensions/specification.js'
import {ExtensionSpecification, createContractBasedModuleSpecification} from '../extensions/specification.js'
import {unifiedConfigurationParserFactory} from '../../utilities/json-schema.js'
import {getCachedAppInfo} from '../../services/local-storage.js'
import use from '../../services/app/config/use.js'
import {WebhooksSchema} from '../extensions/specifications/app_config_webhook_schemas/webhooks_schema.js'
Expand Down Expand Up @@ -2666,6 +2667,44 @@ describe('load', () => {
expect(app).toBeDefined()
expect(app.name).toBe('for-testing')
})

test('does not flag a contract-based configuration spec section as unsupported', async () => {
// Given: A TOML config with a section that matches a contract-based config spec
const configWithContractSection = buildAppConfiguration({
extra: '[purchase_options]\nbundles = true',
})
await writeConfig(configWithContractSection)

// Create a contract-based config spec (mimicking a remote-only spec like purchase_options)
const contractSpec = createContractBasedModuleSpecification({
identifier: 'purchase_options',
uidStrategy: 'single',
experience: 'configuration',
appModuleFeatures: () => [],
})

// Attach a unified parser with the JSON schema contract, like the real flow does
const validationSchema = {
jsonSchema:
'{"type":"object","additionalProperties":false,"properties":{"bundles":{"type":"boolean","description":"Whether the app supports purchase options on bundle products"}}}',
}
const parseConfigurationObject = await unifiedConfigurationParserFactory(
contractSpec as any,
validationSchema,
'strip',
)
const specsWithContract = [...specifications, {...contractSpec, parseConfigurationObject}]

// When: The app is loaded with the contract-based spec included
const app = await loadApp({
directory: tmpDir,
specifications: specsWithContract,
userProvidedConfigName: undefined,
})

// Then: No "unsupported section" error for purchase_options
expect(app.errors.isEmpty()).toBe(true)
})
})

describe('getAppConfigurationFileName', () => {
Expand Down
17 changes: 15 additions & 2 deletions packages/app/src/cli/models/app/loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -787,8 +787,21 @@ class AppLoader<TConfig extends CurrentAppConfiguration, TModuleSpec extends Ext
return [null, [] as string[]] as const
}
const specConfiguration = specResult.data
let claimedKeys = Object.keys(specConfiguration)

// For contract-based configuration specs (remote-only specs using zod.any()), the
// parsed result contains section contents (e.g. {bundles: true}) rather than the
// top-level TOML key (e.g. purchase_options). Replace claimedKeys with just the
// identifier to avoid falsely claiming unrelated top-level keys that happen to
// collide with nested section content keys.
if (
!claimedKeys.includes(specification.identifier) &&
specification.identifier in (appConfiguration as Record<string, unknown>)
) {
claimedKeys = [specification.identifier]
}

if (Object.keys(specConfiguration).length === 0) return [null, Object.keys(specConfiguration)] as const
if (Object.keys(specConfiguration).length === 0) return [null, claimedKeys] as const

const instance = await this.createExtensionInstance(
specification.identifier,
Expand All @@ -802,7 +815,7 @@ class AppLoader<TConfig extends CurrentAppConfiguration, TModuleSpec extends Ext
extensionInstance,
),
)
return [instance, Object.keys(specConfiguration)] as const
return [instance, claimedKeys] as const
}),
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {BaseSchema} from './schemas.js'
import {ClientSteps} from '../../services/build/client-steps.js'
import {AppSchema} from '../app/app.js'
import {describe, test, expect, beforeAll} from 'vitest'
import {zod} from '@shopify/cli-kit/node/schema'

// If the AppSchema is not instanced, the dynamic loading of loadLocalExtensionsSpecifications is not working
beforeAll(() => {
Expand Down Expand Up @@ -126,6 +127,63 @@ describe('createConfigExtensionSpecification', () => {
})
})

describe('contributeToAppConfigurationSchema', () => {
test('contract-based config spec with zod.any() contributes its identifier as a known key', () => {
// Given: A contract-based config spec with experience: 'configuration' and zod.any() schema
const spec = createContractBasedModuleSpecification({
identifier: 'purchase_options',
uidStrategy: 'single',
experience: 'configuration',
appModuleFeatures: () => [],
})

// When: It contributes to the app configuration schema
const baseSchema = zod.object({client_id: zod.string()})
const result = spec.contributeToAppConfigurationSchema(baseSchema)

// Then: The resulting schema should accept the identifier as a valid key
const parsed = result.safeParse({client_id: 'test', purchase_options: {bundles: true}})
expect(parsed.success).toBe(true)
})

test('contract-based config spec with experience: extension does not contribute to schema', () => {
// Given: A contract-based spec with experience: 'extension' (not 'configuration')
const spec = createContractBasedModuleSpecification({
identifier: 'some_extension',
uidStrategy: 'uuid',
experience: 'extension',
appModuleFeatures: () => [],
})

// When: It tries to contribute to the app configuration schema
const baseSchema = zod.object({client_id: zod.string()}).strict()
const result = spec.contributeToAppConfigurationSchema(baseSchema)

// Then: The schema should be unchanged — 'some_extension' is not accepted
const parsed = result.safeParse({client_id: 'test', some_extension: {enabled: true}})
expect(parsed.success).toBe(false)
})

test('config spec with explicit zod schema merges its schema shape', () => {
// Given: A locally-defined config spec with an explicit zod schema
const spec = createConfigExtensionSpecification({
identifier: 'test_config',
schema: BaseSchema.extend({
my_section: zod.object({enabled: zod.boolean()}).optional(),
}),
transformConfig: {},
})

// When: It contributes to the app configuration schema
const baseSchema = zod.object({client_id: zod.string()})
const result = spec.contributeToAppConfigurationSchema(baseSchema)

// Then: The schema should accept 'my_section' as a valid key
const parsed = result.safeParse({client_id: 'test', my_section: {enabled: true}})
expect(parsed.success).toBe(true)
})
})

describe('configWithoutFirstClassFields', () => {
test('removes the first class fields from the config', () => {
// When
Expand Down
15 changes: 11 additions & 4 deletions packages/app/src/cli/models/extensions/specification.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,13 +226,20 @@ export function createExtensionSpecification<TConfiguration extends BaseConfigTy
// This filters out webhook subscription specifications from contributing to the app configuration schema
const hasSingleUidStrategy = merged.uidStrategy === 'single'

const canContribute = isConfig && hasSchema && hasSingleUidStrategy
if (!canContribute) {
// no change
if (!isConfig || !hasSingleUidStrategy) {
return appConfigSchema
}

if (hasSchema) {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
return (appConfigSchema as any).merge(merged.schema)
}

// For contract-based config specs (e.g. remote-only specs with zod.any()), contribute
// the spec identifier as a known top-level key so it isn't flagged as unsupported.
const contractSchema = zod.object({[merged.identifier]: zod.any().optional()})
// eslint-disable-next-line @typescript-eslint/no-explicit-any
return (appConfigSchema as any).merge(merged.schema)
return (appConfigSchema as any).merge(contractSchema)
},
parseConfigurationObject: (configurationObject: unknown) => {
const parseResult = merged.schema.safeParse(configurationObject)
Expand Down
79 changes: 79 additions & 0 deletions packages/app/src/cli/utilities/json-schema.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,85 @@ describe('unifiedConfigurationParserFactory', () => {
expect(priceError).toBeDefined()
})

test('scopes validation to the section contents for configuration specs when identifier matches a key in the config', async () => {
// Given: A contract-based config spec (like purchase_options) where the JSON schema
// describes the section contents ({bundles: boolean}), and the parser receives the
// entire app config with the section nested under the identifier key.
const merged = {
identifier: 'purchase_options',
experience: 'configuration',
parseConfigurationObject: mockParseConfigurationObject,
validationSchema: {
jsonSchema:
'{"type":"object","additionalProperties":false,"properties":{"bundles":{"type":"boolean","description":"Whether the app supports purchase options on bundle products"}}}',
},
}

// When: The parser receives a full app config where "purchase_options" is a nested section
const parser = await unifiedConfigurationParserFactory(merged as any, merged.validationSchema, 'strip')
const result = parser({
client_id: 'test-id',
name: 'my-app',
purchase_options: {bundles: true},
webhooks: {api_version: '2024-01'},
})

// Then: The parser should scope to the section, validate {bundles: true} against the
// schema, and return only the section contents — not an empty object.
expect(result.state).toBe('ok')
expect(result.data).toEqual({bundles: true})
})

test('returns empty object when config spec identifier is not present in the config', async () => {
// Given: Same contract-based config spec, but the TOML doesn't include the section
const merged = {
identifier: 'purchase_options',
experience: 'configuration',
parseConfigurationObject: mockParseConfigurationObject,
validationSchema: {
jsonSchema:
'{"type":"object","additionalProperties":false,"properties":{"bundles":{"type":"boolean","description":"Whether the app supports purchase options on bundle products"}}}',
},
}

// When: The config does NOT contain "purchase_options"
const parser = await unifiedConfigurationParserFactory(merged as any, merged.validationSchema, 'strip')
const result = parser({
client_id: 'test-id',
name: 'my-app',
webhooks: {api_version: '2024-01'},
})

// Then: No scoping happens; strip removes all non-matching keys, leaving {}
expect(result.state).toBe('ok')
expect(result.data).toEqual({})
})

test('validates section contents against the JSON schema when scoped', async () => {
// Given: A config spec with a required field in the JSON schema
const merged = {
identifier: 'my_config',
experience: 'configuration',
parseConfigurationObject: mockParseConfigurationObject,
validationSchema: {
jsonSchema:
'{"type":"object","additionalProperties":false,"properties":{"enabled":{"type":"boolean"}},"required":["enabled"]}',
},
}

// When: The section exists but is missing the required field
const parser = await unifiedConfigurationParserFactory(merged as any, merged.validationSchema, 'strip')
const result = parser({
client_id: 'test-id',
my_config: {not_enabled: true},
})

// Then: Validation should fail because "enabled" is required
expect(result.state).toBe('error')
expect(result.errors).toBeDefined()
expect(result.errors!.length).toBeGreaterThan(0)
})

test('adds base properties to the JSON schema', async () => {
// Given
const merged = {
Expand Down
13 changes: 12 additions & 1 deletion packages/app/src/cli/utilities/json-schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,18 @@ export async function unifiedConfigurationParserFactory(

// Then, even if this failed, we try to validate against the contract.
const zodValidatedData = zodParse.state === 'ok' ? zodParse.data : undefined
const subjectForAjv = zodValidatedData ?? (config as JsonMapType)
let subjectForAjv = zodValidatedData ?? (config as JsonMapType)

// For contract-based specs (remote-only specs with zod.any()), the zod parse returns the
// entire app config. The JSON schema contract describes the section contents (e.g.
// {bundles: boolean} for purchase_options), not the whole config. If the spec identifier
// exists as a key in the parsed data, scope to that section before validating.
if (extensionIdentifier in subjectForAjv) {
const sectionData = subjectForAjv[extensionIdentifier as keyof typeof subjectForAjv]
if (sectionData !== null && typeof sectionData === 'object' && !Array.isArray(sectionData)) {
subjectForAjv = sectionData as JsonMapType
}
}

const jsonSchemaParse = jsonSchemaValidate(
subjectForAjv,
Expand Down
Loading