Add errors to TomlFile and Project for TOML parse failure tracking

packages/app/src/cli/commands/app/config/validate.test.ts

@@ -2,48 +2,65 @@ import Validate from './validate.js'
 import {linkedAppContext} from '../../../services/app-context.js'
 import {validateApp} from '../../../services/validate.js'
 import {testAppLinked} from '../../../models/app/app.test-data.js'
+import {Project} from '../../../models/project/project.js'
+import {outputResult} from '@shopify/cli-kit/node/output'
 import {describe, expect, test, vi} from 'vitest'
 
 vi.mock('../../../services/app-context.js')
 vi.mock('../../../services/validate.js')
+vi.mock('../../../models/project/project.js')
+vi.mock('@shopify/cli-kit/node/output', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('@shopify/cli-kit/node/output')>()
+  return {...actual, outputResult: vi.fn()}
+})
+vi.mock('@shopify/cli-kit/node/ui')
+
+function mockProjectWithNoErrors() {
+  vi.mocked(Project.load).mockResolvedValue({errors: []} as unknown as Project)
+}
 
 describe('app config validate command', () => {
   test('calls validateApp with json: false by default', async () => {
-    // Given
     const app = testAppLinked()
+    mockProjectWithNoErrors()
     vi.mocked(linkedAppContext).mockResolvedValue({app} as Awaited<ReturnType<typeof linkedAppContext>>)
     vi.mocked(validateApp).mockResolvedValue()
 
-    // When
     await Validate.run([], import.meta.url)
 
-    // Then
     expect(validateApp).toHaveBeenCalledWith(app, {json: false})
   })
 
   test('calls validateApp with json: true when --json flag is passed', async () => {
-    // Given
     const app = testAppLinked()
+    mockProjectWithNoErrors()
     vi.mocked(linkedAppContext).mockResolvedValue({app} as Awaited<ReturnType<typeof linkedAppContext>>)
     vi.mocked(validateApp).mockResolvedValue()
 
-    // When
     await Validate.run(['--json'], import.meta.url)
 
-    // Then
     expect(validateApp).toHaveBeenCalledWith(app, {json: true})
   })
 
   test('calls validateApp with json: true when -j flag is passed', async () => {
-    // Given
     const app = testAppLinked()
+    mockProjectWithNoErrors()
     vi.mocked(linkedAppContext).mockResolvedValue({app} as Awaited<ReturnType<typeof linkedAppContext>>)
     vi.mocked(validateApp).mockResolvedValue()
 
-    // When
     await Validate.run(['-j'], import.meta.url)
 
-    // Then
     expect(validateApp).toHaveBeenCalledWith(app, {json: true})
   })
+
+  test('outputs JSON issues when project has TOML parse errors', async () => {
+    vi.mocked(Project.load).mockResolvedValue({
+      errors: [{path: '/app/shopify.app.toml', message: 'Unexpected character at row 1, col 5'}],
+    } as unknown as Project)
+
+    await expect(Validate.run(['--json'], import.meta.url)).rejects.toThrow()
+
+    expect(outputResult).toHaveBeenCalledWith(expect.stringContaining('"valid": false'))
+    expect(linkedAppContext).not.toHaveBeenCalled()
+  })
 })

packages/app/src/cli/commands/app/config/validate.ts

@@ -2,9 +2,11 @@ import {appFlags} from '../../../flags.js'
 import {validateApp} from '../../../services/validate.js'
 import AppLinkedCommand, {AppLinkedCommandOutput} from '../../../utilities/app-linked-command.js'
 import {linkedAppContext} from '../../../services/app-context.js'
+import {Project} from '../../../models/project/project.js'
 import {globalFlags, jsonFlag} from '@shopify/cli-kit/node/cli'
 import {AbortError, AbortSilentError} from '@shopify/cli-kit/node/error'
 import {outputResult, stringifyMessage, unstyled} from '@shopify/cli-kit/node/output'
+import {renderError} from '@shopify/cli-kit/node/ui'
 
 export default class Validate extends AppLinkedCommand {
   static summary = 'Validate your app configuration and extensions.'
@@ -22,6 +24,34 @@ export default class Validate extends AppLinkedCommand {
   public async run(): Promise<AppLinkedCommandOutput> {
     const {flags} = await this.parse(Validate)
 
+    // Check for TOML parse errors before attempting to link/load.
+    // If the active config has parse errors, report them directly
+    // instead of falling into the linking flow.
+    let project: Project
+    try {
+      project = await Project.load(flags.path)
+    } catch (err) {
+      if (err instanceof AbortError && flags.json) {
+        const message = unstyled(stringifyMessage(err.message)).trim()
+        outputResult(JSON.stringify({valid: false, issues: [{message}]}, null, 2))
+        throw new AbortSilentError()
+      }
+      throw err
+    }
+
+    if (project.errors.length > 0) {
+      const issues = project.errors.map((err) => ({file: err.path, message: err.message}))
+      if (flags.json) {
+        outputResult(JSON.stringify({valid: false, issues}, null, 2))
+        throw new AbortSilentError()
+      }
+      renderError({
+        headline: 'Validation errors found.',
+        body: issues.map((issue) => `• ${issue.message}`).join('\n'),
+      })
+      throw new AbortSilentError()
+    }
+
     let app
     try {
       const context = await linkedAppContext({
@@ -35,7 +65,7 @@ export default class Validate extends AppLinkedCommand {
     } catch (err) {
       if (err instanceof AbortError && flags.json) {
         const message = unstyled(stringifyMessage(err.message)).trim()
-        outputResult(JSON.stringify({valid: false, errors: [{message}]}, null, 2))
+        outputResult(JSON.stringify({valid: false, issues: [{message}]}, null, 2))
         throw new AbortSilentError()
       }
       throw err

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

@@ -38,7 +38,7 @@ import {
 } from '../project/config-selection.js'
 import {showMultipleCLIWarningIfNeeded} from '@shopify/cli-kit/node/multiple-installation-warning'
 import {fileExists, readFile, fileExistsSync} from '@shopify/cli-kit/node/fs'
-import {TomlFile, TomlFileNotFoundError, TomlParseError} from '@shopify/cli-kit/node/toml/toml-file'
+import {TomlFile, TomlFileError} from '@shopify/cli-kit/node/toml/toml-file'
 import {zod} from '@shopify/cli-kit/node/schema'
 import {PackageManager} from '@shopify/cli-kit/node/node-package-manager'
 import {resolveFramework} from '@shopify/cli-kit/node/framework'
@@ -96,10 +96,7 @@ export async function parseConfigurationFile<TSchema extends zod.ZodType>(
       const file = await TomlFile.read(filepath)
       content = file.content
     } catch (err) {
-      if (err instanceof TomlFileNotFoundError) {
-        return {errors: [{file: filepath, message: `Couldn't find the configuration file at ${filepath}`}]}
-      }
-      if (err instanceof TomlParseError) {
+      if (err instanceof TomlFileError) {
         return {errors: [{file: filepath, message: err.message}]}
       }
       throw err

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

@@ -164,25 +164,29 @@ describe('selectActiveConfig', () => {
     })
   })
 
-  test('throws when the only app config is malformed (no valid configs to fall back to)', async () => {
+  test('loads with errors when the only app config is malformed', async () => {
     await inTemporaryDirectory(async (dir) => {
-      // The only config is broken TOML — Project.load skips it and finds 0 valid configs
+      // The only config is broken TOML — Project.load includes it with errors
       await writeFile(joinPath(dir, 'shopify.app.toml'), '{{invalid toml')
 
-      await expect(Project.load(dir)).rejects.toThrow(/Could not find/)
+      const project = await Project.load(dir)
+      expect(project.appConfigFiles).toHaveLength(1)
+      expect(project.appConfigFiles[0]!.errors).toHaveLength(1)
+      expect(project.errors).toHaveLength(1)
+      expect(project.errors[0]!.path).toContain('shopify.app.toml')
     })
   })
 
-  test('surfaces parse error when selecting a broken config while a valid one exists', async () => {
+  test('selects a broken config and exposes its errors', async () => {
     await inTemporaryDirectory(async (dir) => {
-      // Two configs: one good, one broken. Selecting the broken one by name should
-      // surface the real parse error via the fallback re-read, not a generic "not found".
+      // Two configs: one good, one broken. Selecting the broken one succeeds
+      // but the selected file has errors.
       await writeFile(joinPath(dir, 'shopify.app.toml'), 'client_id = "good"')
       await writeFile(joinPath(dir, 'shopify.app.broken.toml'), '{{invalid toml')
 
       const project = await Project.load(dir)
-
-      await expect(selectActiveConfig(project, 'shopify.app.broken.toml')).rejects.toThrow()
+      const activeConfig = await selectActiveConfig(project, 'broken')
+      expect(activeConfig.file.errors).toHaveLength(1)
     })
   })
 

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

@@ -159,44 +159,54 @@ describe('Project', () => {
       })
     })
 
-    test('skips malformed inactive app config without blocking active config', async () => {
+    test('includes malformed inactive app config with errors without blocking active config', async () => {
       await inTemporaryDirectory(async (dir) => {
         await writeAppToml(dir, 'client_id = "good"')
         await writeAppToml(dir, '{{invalid toml content', 'shopify.app.broken.toml')
 
         const project = await Project.load(dir)
 
-        // The broken config is skipped, but the valid one is loaded
-        expect(project.appConfigFiles).toHaveLength(1)
-        expect(project.appConfigFiles[0]!.content.client_id).toBe('good')
+        expect(project.appConfigFiles).toHaveLength(2)
+        const good = project.appConfigFiles.find((file) => file.content.client_id === 'good')
+        const broken = project.appConfigFiles.find((file) => file.errors.length > 0)
+        expect(good).toBeDefined()
+        expect(broken).toBeDefined()
+        expect(broken!.path).toContain('shopify.app.broken.toml')
+        expect(project.errors).toHaveLength(1)
       })
     })
 
-    test('skips malformed extension TOML without blocking project load', async () => {
+    test('includes malformed extension TOML with errors without blocking project load', async () => {
       await inTemporaryDirectory(async (dir) => {
         await writeAppToml(dir, 'client_id = "abc"')
         await writeExtensionToml(dir, 'good-ext', 'type = "function"\nname = "good"')
         await writeExtensionToml(dir, 'bad-ext', '{{broken toml')
 
         const project = await Project.load(dir)
 
-        // Only the valid extension is loaded
-        expect(project.extensionConfigFiles).toHaveLength(1)
-        expect(project.extensionConfigFiles[0]!.content.name).toBe('good')
+        expect(project.extensionConfigFiles).toHaveLength(2)
+        const good = project.extensionConfigFiles.find((file) => file.content.name === 'good')
+        const broken = project.extensionConfigFiles.find((file) => file.errors.length > 0)
+        expect(good).toBeDefined()
+        expect(broken).toBeDefined()
+        expect(project.errors).toHaveLength(1)
       })
     })
 
-    test('skips malformed web TOML without blocking project load', async () => {
+    test('includes malformed web TOML with errors without blocking project load', async () => {
       await inTemporaryDirectory(async (dir) => {
         await writeAppToml(dir, 'client_id = "abc"')
         await writeWebToml(dir, 'good-web', 'name = "good"\nroles = ["backend"]')
         await writeWebToml(dir, 'bad-web', '{{broken toml')
 
         const project = await Project.load(dir)
 
-        // Only the valid web config is loaded
-        expect(project.webConfigFiles).toHaveLength(1)
-        expect(project.webConfigFiles[0]!.content.name).toBe('good')
+        expect(project.webConfigFiles).toHaveLength(2)
+        const good = project.webConfigFiles.find((file) => file.content.name === 'good')
+        const broken = project.webConfigFiles.find((file) => file.errors.length > 0)
+        expect(good).toBeDefined()
+        expect(broken).toBeDefined()
+        expect(project.errors).toHaveLength(1)
       })
     })