Surface malformed discovered configs in validate json

Author: dmerand

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

@@ -29,7 +29,7 @@ import {installNodeModules, PackageJson} from '@shopify/cli-kit/node/node-packag
 import {inTemporaryDirectory, moveFile, mkdir, mkTmpDir, rmdir, writeFile} from '@shopify/cli-kit/node/fs'
 import {joinPath, dirname, cwd, normalizePath} from '@shopify/cli-kit/node/path'
 import {platformAndArch} from '@shopify/cli-kit/node/os'
-import {outputContent, outputToken} from '@shopify/cli-kit/node/output'
+import {outputContent, outputToken, stringifyMessage} from '@shopify/cli-kit/node/output'
 import {zod} from '@shopify/cli-kit/node/schema'
 import colors from '@shopify/cli-kit/node/colors'
 import {showMultipleCLIWarningIfNeeded} from '@shopify/cli-kit/node/multiple-installation-warning'
@@ -599,6 +599,39 @@ describe('load', () => {
     await expect(loadTestingApp()).rejects.toThrow(/Missing handle for extension "my_extension"/)
   })
 
+  test('throws when a discovered extension TOML is malformed', async () => {
+    // Given
+    await writeConfig(appConfiguration)
+    const configurationPath = blockConfigurationPath({name: 'my-extension'})
+    await writeBlockConfig({
+      blockConfiguration: '{{broken toml',
+      name: 'my-extension',
+    })
+
+    // When / Then
+    await expect(loadTestingApp()).rejects.toThrow(/Unknown character/)
+    await expect(loadTestingApp()).rejects.toThrow(configurationPath)
+  })
+
+  test('reports a malformed discovered extension TOML in report mode', async () => {
+    // Given
+    await writeConfig(appConfiguration)
+    const configurationPath = blockConfigurationPath({name: 'my-extension'})
+    await writeBlockConfig({
+      blockConfiguration: '{{broken toml',
+      name: 'my-extension',
+    })
+
+    // When
+    const app = await loadTestingApp({mode: 'report'})
+
+    // Then
+    expect(getRealExtensions(app)).toHaveLength(0)
+    expect(stringifyMessage(app.errors?.getError(configurationPath))).toContain(configurationPath)
+    expect(stringifyMessage(app.errors?.getError(configurationPath))).toContain('Unknown character')
+    expect(app.errors?.getIssues(configurationPath)).toEqual([])
+  })
+
   test('throws an error if the extension configuration is missing both extensions and type', async () => {
     // Given
     await writeConfig(appConfiguration, {
@@ -621,6 +654,33 @@ describe('load', () => {
     await expect(loadTestingApp()).rejects.toThrow(/Invalid extension type/)
   })
 
+  test('throws when a discovered web TOML is malformed', async () => {
+    // Given
+    const {webDirectory} = await writeConfig(appConfiguration)
+    const configurationPath = joinPath(webDirectory, blocks.web.configurationName)
+    await writeFile(configurationPath, '{{broken toml')
+
+    // When / Then
+    await expect(loadTestingApp()).rejects.toThrow(/Unknown character/)
+    await expect(loadTestingApp()).rejects.toThrow(configurationPath)
+  })
+
+  test('reports a malformed discovered web TOML in report mode', async () => {
+    // Given
+    const {webDirectory} = await writeConfig(appConfiguration)
+    const configurationPath = joinPath(webDirectory, blocks.web.configurationName)
+    await writeFile(configurationPath, '{{broken toml')
+
+    // When
+    const app = await loadTestingApp({mode: 'report'})
+
+    // Then
+    expect(app.webs).toHaveLength(0)
+    expect(stringifyMessage(app.errors?.getError(configurationPath))).toContain(configurationPath)
+    expect(stringifyMessage(app.errors?.getError(configurationPath))).toContain('Unknown character')
+    expect(app.errors?.getIssues(configurationPath)).toEqual([])
+  })
+
   test('loads the app with web blocks', async () => {
     // Given
     const {webDirectory} = await writeConfig(appConfiguration)

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

@@ -40,6 +40,8 @@ import {
   resolveDotEnv,
   resolveHiddenConfig,
   extensionFilesForConfig,
+  malformedExtensionFilesForConfig,
+  malformedWebFilesForConfig,
   webFilesForConfig,
 } from '../project/config-selection.js'
 import {showMultipleCLIWarningIfNeeded} from '@shopify/cli-kit/node/multiple-installation-warning'
@@ -565,6 +567,14 @@ class AppLoader<TConfig extends CurrentAppConfiguration, TModuleSpec extends Ext
   async loadWebs(appDirectory: string, webDirectories?: string[]): Promise<{webs: Web[]; usedCustomLayout: boolean}> {
     const activeConfig = this.activeConfigFile
     const webFiles = activeConfig ? webFilesForConfig(this.project, activeConfig) : this.project.webConfigFiles
+    const malformedWebFiles = activeConfig
+      ? malformedWebFilesForConfig(this.project, activeConfig)
+      : this.project.malformedWebConfigFiles
+
+    for (const malformedWebFile of malformedWebFiles) {
+      this.abortOrReport(malformedWebFile.message, undefined, malformedWebFile.path)
+    }
+
     const webTomlPaths = webFiles.map((file) => file.path)
     const webs = await Promise.all(
       webFiles.map((webFile) => loadSingleWeb(webFile.path, this.abortOrReport.bind(this), webFile.content)),
@@ -707,6 +717,13 @@ class AppLoader<TConfig extends CurrentAppConfiguration, TModuleSpec extends Ext
     const extensionFiles = activeConfig
       ? extensionFilesForConfig(this.project, activeConfig)
       : this.project.extensionConfigFiles
+    const malformedExtensionFiles = activeConfig
+      ? malformedExtensionFilesForConfig(this.project, activeConfig)
+      : this.project.malformedExtensionConfigFiles
+
+    for (const malformedExtensionFile of malformedExtensionFiles) {
+      this.abortOrReport(malformedExtensionFile.message, undefined, malformedExtensionFile.path)
+    }
 
     return extensionFiles.map(async (extensionFile) => {
       const configurationPath = extensionFile.path

packages/app/src/cli/models/project/config-selection.test.ts

@@ -1,4 +1,11 @@
-import {resolveDotEnv, resolveHiddenConfig, extensionFilesForConfig, webFilesForConfig} from './config-selection.js'
+import {
+  resolveDotEnv,
+  resolveHiddenConfig,
+  extensionFilesForConfig,
+  malformedExtensionFilesForConfig,
+  malformedWebFilesForConfig,
+  webFilesForConfig,
+} from './config-selection.js'
 import {Project} from './project.js'
 import {describe, expect, test} from 'vitest'
 import {inTemporaryDirectory, writeFile, mkdir} from '@shopify/cli-kit/node/fs'
@@ -230,6 +237,33 @@ describe('extensionFilesForConfig', () => {
       expect(stagingExts[0]!.content.name).toBe('func2')
     })
   })
+
+  test('filters malformed extension files to the active config extension_directories', async () => {
+    await inTemporaryDirectory(async (dir) => {
+      await writeFile(joinPath(dir, 'shopify.app.toml'), 'client_id = "default"\nextension_directories = ["ext-a/*"]')
+      await writeFile(
+        joinPath(dir, 'shopify.app.staging.toml'),
+        'client_id = "staging"\nextension_directories = ["ext-b/*"]',
+      )
+
+      await mkdir(joinPath(dir, 'ext-a', 'broken-default'))
+      await writeFile(joinPath(dir, 'ext-a', 'broken-default', 'shopify.extension.toml'), '{{broken toml')
+      await mkdir(joinPath(dir, 'ext-b', 'broken-staging'))
+      await writeFile(joinPath(dir, 'ext-b', 'broken-staging', 'shopify.extension.toml'), '{{broken toml')
+
+      const project = await Project.load(dir)
+      const defaultConfig = project.appConfigByName('shopify.app.toml')!
+      const stagingConfig = project.appConfigByName('shopify.app.staging.toml')!
+
+      const defaultMalformed = malformedExtensionFilesForConfig(project, defaultConfig)
+      expect(defaultMalformed).toHaveLength(1)
+      expect(defaultMalformed[0]!.path).toContain('ext-a/broken-default/shopify.extension.toml')
+
+      const stagingMalformed = malformedExtensionFilesForConfig(project, stagingConfig)
+      expect(stagingMalformed).toHaveLength(1)
+      expect(stagingMalformed[0]!.path).toContain('ext-b/broken-staging/shopify.extension.toml')
+    })
+  })
 })
 
 describe('webFilesForConfig', () => {
@@ -287,4 +321,28 @@ describe('webFilesForConfig', () => {
       expect(webFiles[0]!.content.name).toBe('web-a')
     })
   })
+
+  test('filters malformed web files to the active config web_directories', async () => {
+    await inTemporaryDirectory(async (dir) => {
+      await writeFile(joinPath(dir, 'shopify.app.toml'), 'client_id = "default"\nweb_directories = ["web-a"]')
+      await writeFile(joinPath(dir, 'shopify.app.staging.toml'), 'client_id = "staging"\nweb_directories = ["web-b"]')
+
+      await mkdir(joinPath(dir, 'web-a'))
+      await writeFile(joinPath(dir, 'web-a', 'shopify.web.toml'), '{{broken toml')
+      await mkdir(joinPath(dir, 'web-b'))
+      await writeFile(joinPath(dir, 'web-b', 'shopify.web.toml'), '{{broken toml')
+
+      const project = await Project.load(dir)
+      const defaultConfig = project.appConfigByName('shopify.app.toml')!
+      const stagingConfig = project.appConfigByName('shopify.app.staging.toml')!
+
+      const defaultMalformed = malformedWebFilesForConfig(project, defaultConfig)
+      expect(defaultMalformed).toHaveLength(1)
+      expect(defaultMalformed[0]!.path).toContain('web-a/shopify.web.toml')
+
+      const stagingMalformed = malformedWebFilesForConfig(project, stagingConfig)
+      expect(stagingMalformed).toHaveLength(1)
+      expect(stagingMalformed[0]!.path).toContain('web-b/shopify.web.toml')
+    })
+  })
 })

packages/app/src/cli/models/project/config-selection.ts

@@ -1,4 +1,4 @@
-import {Project} from './project.js'
+import {Project, type MalformedTomlFile} from './project.js'
 import {AppHiddenConfig} from '../app/app.js'
 import {getAppConfigurationShorthand} from '../app/loader.js'
 import {dotEnvFileNames} from '../../constants.js'
@@ -76,19 +76,23 @@ export async function resolveHiddenConfig(project: Project, clientId: string | u
  * @public
  */
 export function extensionFilesForConfig(project: Project, activeConfig: TomlFile): TomlFile[] {
-  const configDirs = activeConfig.content.extension_directories
-  const dirs = Array.isArray(configDirs) && configDirs.length > 0 ? (configDirs as string[]) : ['extensions/*']
-
-  // Replicate the same glob patterns used by discoverExtensionFiles in project.ts:
-  // each directory pattern becomes "<dir>/*.extension.toml"
-  const globPatterns = dirs.map((dir) => `${dir}/*.extension.toml`)
+  const globPatterns = extensionGlobPatternsForConfig(activeConfig)
 
   return project.extensionConfigFiles.filter((file) => {
     const relPath = relativePath(project.directory, file.path).replace(/\\/g, '/')
     return globPatterns.some((pattern) => matchGlob(relPath, pattern))
   })
 }
 
+export function malformedExtensionFilesForConfig(project: Project, activeConfig: TomlFile): MalformedTomlFile[] {
+  const globPatterns = extensionGlobPatternsForConfig(activeConfig)
+
+  return project.malformedExtensionConfigFiles.filter((file) => {
+    const relPath = relativePath(project.directory, file.path).replace(/\\/g, '/')
+    return globPatterns.some((pattern) => matchGlob(relPath, pattern))
+  })
+}
+
 /**
  * Filter web config files to those belonging to the active config's
  * web_directories. If not specified, returns all web files.
@@ -98,17 +102,35 @@ export function extensionFilesForConfig(project: Project, activeConfig: TomlFile
  * @public
  */
 export function webFilesForConfig(project: Project, activeConfig: TomlFile): TomlFile[] {
-  const configDirs = activeConfig.content.web_directories
-  if (!Array.isArray(configDirs) || configDirs.length === 0) {
-    return project.webConfigFiles
-  }
-
-  // Replicate the same glob patterns used by discoverWebFiles in project.ts:
-  // each directory pattern becomes "<dir>/shopify.web.toml"
-  const globPatterns = (configDirs as string[]).map((dir) => `${dir}/shopify.web.toml`)
+  const globPatterns = webGlobPatternsForConfig(activeConfig)
 
   return project.webConfigFiles.filter((file) => {
     const relPath = relativePath(project.directory, file.path).replace(/\\/g, '/')
     return globPatterns.some((pattern) => matchGlob(relPath, pattern))
   })
 }
+
+export function malformedWebFilesForConfig(project: Project, activeConfig: TomlFile): MalformedTomlFile[] {
+  const globPatterns = webGlobPatternsForConfig(activeConfig)
+
+  return project.malformedWebConfigFiles.filter((file) => {
+    const relPath = relativePath(project.directory, file.path).replace(/\\/g, '/')
+    return globPatterns.some((pattern) => matchGlob(relPath, pattern))
+  })
+}
+
+function extensionGlobPatternsForConfig(activeConfig: TomlFile): string[] {
+  const configDirs = activeConfig.content.extension_directories
+  const dirs = Array.isArray(configDirs) && configDirs.length > 0 ? (configDirs as string[]) : ['extensions/*']
+
+  return dirs.map((dir) => `${dir}/*.extension.toml`)
+}
+
+function webGlobPatternsForConfig(activeConfig: TomlFile): string[] {
+  const configDirs = activeConfig.content.web_directories
+  if (!Array.isArray(configDirs) || configDirs.length === 0) {
+    return ['**/shopify.web.toml']
+  }
+
+  return (configDirs as string[]).map((dir) => `${dir}/shopify.web.toml`)
+}

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

@@ -188,9 +188,11 @@ describe('Project', () => {
 
         const project = await Project.load(dir)
 
-        // Only the valid extension is loaded
+        // Only the valid extension is loaded, but malformed files are retained as discovery metadata
         expect(project.extensionConfigFiles).toHaveLength(1)
         expect(project.extensionConfigFiles[0]!.content.name).toBe('good')
+        expect(project.malformedExtensionConfigFiles).toHaveLength(1)
+        expect(project.malformedExtensionConfigFiles[0]!.path).toContain('bad-ext/shopify.extension.toml')
       })
     })
 
@@ -202,9 +204,11 @@ describe('Project', () => {
 
         const project = await Project.load(dir)
 
-        // Only the valid web config is loaded
+        // Only the valid web config is loaded, but malformed files are retained as discovery metadata
         expect(project.webConfigFiles).toHaveLength(1)
         expect(project.webConfigFiles[0]!.content.name).toBe('good')
+        expect(project.malformedWebConfigFiles).toHaveLength(1)
+        expect(project.malformedWebConfigFiles[0]!.path).toContain('bad-web/shopify.web.toml')
       })
     })