From 0bd1e5b8377467e78bbca299295168a568ea9290 Mon Sep 17 00:00:00 2001 From: Ryan Bahan Date: Mon, 30 Mar 2026 13:52:43 -0600 Subject: [PATCH 1/4] Add errors field to TomlFile and Project for TOML parse failure tracking Co-Authored-By: Claude Opus 4.6 (1M context) --- .../cli/models/project/active-config.test.ts | 9 ++- .../app/src/cli/models/project/project.ts | 59 +++++++++++-------- .../cli-kit/src/public/node/toml/toml-file.ts | 7 +++ 3 files changed, 47 insertions(+), 28 deletions(-) diff --git a/packages/app/src/cli/models/project/active-config.test.ts b/packages/app/src/cli/models/project/active-config.test.ts index c50a64d257..2f775ed747 100644 --- a/packages/app/src/cli/models/project/active-config.test.ts +++ b/packages/app/src/cli/models/project/active-config.test.ts @@ -164,12 +164,15 @@ 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 records the error instead of throwing 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(0) + expect(project.errors).toHaveLength(1) + expect(project.errors[0]!.path).toContain('shopify.app.toml') }) }) diff --git a/packages/app/src/cli/models/project/project.ts b/packages/app/src/cli/models/project/project.ts index 7ffbba3bd5..01c1bb6688 100644 --- a/packages/app/src/cli/models/project/project.ts +++ b/packages/app/src/cli/models/project/project.ts @@ -1,5 +1,5 @@ import {configurationFileNames} from '../../constants.js' -import {TomlFile} from '@shopify/cli-kit/node/toml/toml-file' +import {TomlFile, type TomlFileError} from '@shopify/cli-kit/node/toml/toml-file' import {readAndParseDotEnv, DotEnvFile} from '@shopify/cli-kit/node/dot-env' import {fileExists, glob, findPathUp, readFile} from '@shopify/cli-kit/node/fs' import { @@ -10,7 +10,6 @@ import { } from '@shopify/cli-kit/node/node-package-manager' import {joinPath, basename} from '@shopify/cli-kit/node/path' import {AbortError} from '@shopify/cli-kit/node/error' -import {outputDebug} from '@shopify/cli-kit/node/output' import {JsonMapType} from '@shopify/cli-kit/node/toml' const APP_CONFIG_GLOB = 'shopify.app*.toml' @@ -43,10 +42,11 @@ export class Project { */ static async load(startDirectory: string): Promise { const directory = await findProjectRoot(startDirectory) + const errors: TomlFileError[] = [] // Discover all app config files - const appConfigFiles = await discoverAppConfigFiles(directory) - if (appConfigFiles.length === 0) { + const appConfigFiles = await discoverAppConfigFiles(directory, errors) + if (appConfigFiles.length === 0 && errors.length === 0) { throw new AbortError(`Could not find a Shopify app TOML file in ${directory}`) } @@ -61,7 +61,7 @@ export class Project { allExtensionDirs.add(DEFAULT_EXTENSION_DIR) } } - const extensionConfigFiles = await discoverExtensionFiles(directory, [...allExtensionDirs]) + const extensionConfigFiles = await discoverExtensionFiles(directory, [...allExtensionDirs], errors) // Discover web files from all app configs' web_directories (union) const allWebDirs = new Set() @@ -71,7 +71,7 @@ export class Project { for (const dir of dirs) allWebDirs.add(dir as string) } } - const webConfigFiles = await discoverWebFiles(directory, allWebDirs.size > 0 ? [...allWebDirs] : undefined) + const webConfigFiles = await discoverWebFiles(directory, allWebDirs.size > 0 ? [...allWebDirs] : undefined, errors) // Project metadata const packageJSONPath = joinPath(directory, 'package.json') @@ -96,6 +96,7 @@ export class Project { webConfigFiles, dotenvFiles, hiddenConfigRaw, + errors, }) } @@ -113,6 +114,9 @@ export class Project { /** Raw .shopify/project.json content — selection logic looks up by client_id */ readonly hiddenConfigRaw: JsonMapType + /** TOML parse errors from discovery. Files with errors are not in appConfigFiles/extensionConfigFiles/webConfigFiles. */ + readonly errors: TomlFileError[] + private constructor(options: { directory: string packageManager: PackageManager @@ -123,6 +127,7 @@ export class Project { webConfigFiles: TomlFile[] dotenvFiles: Map hiddenConfigRaw: JsonMapType + errors: TomlFileError[] }) { this.directory = options.directory this.packageManager = options.packageManager @@ -133,6 +138,7 @@ export class Project { this.webConfigFiles = options.webConfigFiles this.dotenvFiles = options.dotenvFiles this.hiddenConfigRaw = options.hiddenConfigRaw + this.errors = options.errors } // ── File lookup ─────────────────────────────────────────── @@ -174,47 +180,50 @@ async function findProjectRoot(startDirectory: string): Promise { return found } -async function discoverAppConfigFiles(directory: string): Promise { +async function discoverAppConfigFiles(directory: string, errors: TomlFileError[]): Promise { const pattern = joinPath(directory, APP_CONFIG_GLOB) const paths = await glob(pattern) const validPaths = paths.filter((filePath) => APP_CONFIG_REGEX.test(basename(filePath))) - return readTomlFilesSafe(validPaths) + return readTomlFilesCollectingErrors(validPaths, errors) } -async function discoverExtensionFiles(directory: string, extensionDirectories?: string[]): Promise { - const dirs = extensionDirectories ?? [DEFAULT_EXTENSION_DIR] +async function discoverExtensionFiles( + directory: string, + extensionDirectories: string[], + errors: TomlFileError[], +): Promise { + const dirs = extensionDirectories.length > 0 ? extensionDirectories : [DEFAULT_EXTENSION_DIR] const patterns = dirs.map((dir) => joinPath(directory, dir, EXTENSION_TOML)) patterns.push(`!${joinPath(directory, NODE_MODULES_EXCLUDE)}`) const paths = await glob(patterns) - return readTomlFilesSafe(paths) + return readTomlFilesCollectingErrors(paths, errors) } -async function discoverWebFiles(directory: string, webDirectories?: string[]): Promise { +async function discoverWebFiles( + directory: string, + webDirectories: string[] | undefined, + errors: TomlFileError[], +): Promise { const dirs = webDirectories ?? ['**'] const patterns = dirs.map((dir) => joinPath(directory, dir, WEB_TOML)) patterns.push(`!${joinPath(directory, NODE_MODULES_EXCLUDE)}`) const paths = await glob(patterns) - return readTomlFilesSafe(paths) + return readTomlFilesCollectingErrors(paths, errors) } -/** - * Read TOML files, skipping any that fail to parse. - * This prevents a malformed inactive config or extension TOML - * from blocking the active config from loading. - */ -async function readTomlFilesSafe(paths: string[]): Promise { - const results = await Promise.all( +async function readTomlFilesCollectingErrors(paths: string[], errors: TomlFileError[]): Promise { + const files: TomlFile[] = [] + await Promise.all( paths.map(async (filePath) => { try { - return await TomlFile.read(filePath) + files.push(await TomlFile.read(filePath)) // eslint-disable-next-line no-catch-all/no-catch-all - } catch { - outputDebug(`Skipping malformed TOML file: ${filePath}`) - return undefined + } catch (err) { + errors.push({path: filePath, message: err instanceof Error ? err.message : `Failed to read ${filePath}`}) } }), ) - return results.filter((file): file is TomlFile => file !== undefined) + return files } /** Discover all .env* files in the project root */ diff --git a/packages/cli-kit/src/public/node/toml/toml-file.ts b/packages/cli-kit/src/public/node/toml/toml-file.ts index 91eba0e891..56e3ea7866 100644 --- a/packages/cli-kit/src/public/node/toml/toml-file.ts +++ b/packages/cli-kit/src/public/node/toml/toml-file.ts @@ -4,6 +4,12 @@ import {updateTomlValues} from '@shopify/toml-patch' type TomlPatchValue = string | number | boolean | undefined | (string | number | boolean)[] +/** An error on a TOML file — missing or malformed. */ +export interface TomlFileError { + readonly path: string + readonly message: string +} + /** * Thrown when a TOML file does not exist at the expected path. */ @@ -62,6 +68,7 @@ export class TomlFile { readonly path: string content: JsonMapType + readonly errors: TomlFileError[] = [] constructor(path: string, content: JsonMapType) { this.path = path From 50263be70b70a003c11ab79455fd4b53bc6fc0c8 Mon Sep 17 00:00:00 2001 From: Ryan Bahan Date: Mon, 30 Mar 2026 15:23:11 -0600 Subject: [PATCH 2/4] Add errors to TomlFile and Project, surface TOML parse failures in validate --json Co-Authored-By: Claude Opus 4.6 (1M context) --- .../cli/commands/app/config/validate.test.ts | 37 +++++++++++++----- .../src/cli/commands/app/config/validate.ts | 34 +++++++++++++++- packages/app/src/cli/models/app/loader.ts | 7 +--- .../cli/models/project/active-config.test.ts | 15 +++---- .../src/cli/models/project/project.test.ts | 34 ++++++++++------ .../app/src/cli/models/project/project.ts | 10 +++-- .../src/public/node/toml/toml-file.test.ts | 16 ++++---- .../cli-kit/src/public/node/toml/toml-file.ts | 39 ++++++------------- 8 files changed, 119 insertions(+), 73 deletions(-) diff --git a/packages/app/src/cli/commands/app/config/validate.test.ts b/packages/app/src/cli/commands/app/config/validate.test.ts index 5ae0a8203e..50fa06e527 100644 --- a/packages/app/src/cli/commands/app/config/validate.test.ts +++ b/packages/app/src/cli/commands/app/config/validate.test.ts @@ -2,48 +2,67 @@ 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() + 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>) 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>) 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>) 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: 'Fix the following error in shopify.app.toml:\nUnexpected character'}], + } 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() + }) }) diff --git a/packages/app/src/cli/commands/app/config/validate.ts b/packages/app/src/cli/commands/app/config/validate.ts index b6ddd0c9ed..07cec6f691 100644 --- a/packages/app/src/cli/commands/app/config/validate.ts +++ b/packages/app/src/cli/commands/app/config/validate.ts @@ -2,9 +2,13 @@ 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 {getAppConfigurationContext} from '../../../models/app/loader.js' +import {selectActiveConfig} from '../../../models/project/active-config.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 +26,34 @@ export default class Validate extends AppLinkedCommand { public async run(): Promise { 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 +67,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 diff --git a/packages/app/src/cli/models/app/loader.ts b/packages/app/src/cli/models/app/loader.ts index 36a887c2e8..798a03f5d6 100644 --- a/packages/app/src/cli/models/app/loader.ts +++ b/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( 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 diff --git a/packages/app/src/cli/models/project/active-config.test.ts b/packages/app/src/cli/models/project/active-config.test.ts index 2f775ed747..3f189f4e48 100644 --- a/packages/app/src/cli/models/project/active-config.test.ts +++ b/packages/app/src/cli/models/project/active-config.test.ts @@ -166,26 +166,27 @@ describe('selectActiveConfig', () => { test('loads with errors when the only app config is malformed', async () => { await inTemporaryDirectory(async (dir) => { - // The only config is broken TOML — Project.load records the error instead of throwing + // The only config is broken TOML — Project.load includes it with errors await writeFile(joinPath(dir, 'shopify.app.toml'), '{{invalid toml') const project = await Project.load(dir) - expect(project.appConfigFiles).toHaveLength(0) + 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) }) }) diff --git a/packages/app/src/cli/models/project/project.test.ts b/packages/app/src/cli/models/project/project.test.ts index 77b0c5a864..2bdba160a6 100644 --- a/packages/app/src/cli/models/project/project.test.ts +++ b/packages/app/src/cli/models/project/project.test.ts @@ -159,20 +159,24 @@ 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"') @@ -180,13 +184,16 @@ describe('Project', () => { 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"]') @@ -194,9 +201,12 @@ describe('Project', () => { 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) }) }) diff --git a/packages/app/src/cli/models/project/project.ts b/packages/app/src/cli/models/project/project.ts index 01c1bb6688..f34ac61bed 100644 --- a/packages/app/src/cli/models/project/project.ts +++ b/packages/app/src/cli/models/project/project.ts @@ -1,5 +1,5 @@ import {configurationFileNames} from '../../constants.js' -import {TomlFile, type TomlFileError} from '@shopify/cli-kit/node/toml/toml-file' +import {TomlFile, TomlFileError} from '@shopify/cli-kit/node/toml/toml-file' import {readAndParseDotEnv, DotEnvFile} from '@shopify/cli-kit/node/dot-env' import {fileExists, glob, findPathUp, readFile} from '@shopify/cli-kit/node/fs' import { @@ -46,7 +46,7 @@ export class Project { // Discover all app config files const appConfigFiles = await discoverAppConfigFiles(directory, errors) - if (appConfigFiles.length === 0 && errors.length === 0) { + if (appConfigFiles.length === 0) { throw new AbortError(`Could not find a Shopify app TOML file in ${directory}`) } @@ -219,7 +219,11 @@ async function readTomlFilesCollectingErrors(paths: string[], errors: TomlFileEr files.push(await TomlFile.read(filePath)) // eslint-disable-next-line no-catch-all/no-catch-all } catch (err) { - errors.push({path: filePath, message: err instanceof Error ? err.message : `Failed to read ${filePath}`}) + const tomlError = err instanceof TomlFileError ? err : new TomlFileError(filePath, `Failed to read ${filePath}`) + const file = new TomlFile(filePath, {}) + file.errors.push(tomlError) + files.push(file) + errors.push(tomlError) } }), ) diff --git a/packages/cli-kit/src/public/node/toml/toml-file.test.ts b/packages/cli-kit/src/public/node/toml/toml-file.test.ts index 05233ddefc..2edcf00fb1 100644 --- a/packages/cli-kit/src/public/node/toml/toml-file.test.ts +++ b/packages/cli-kit/src/public/node/toml/toml-file.test.ts @@ -1,4 +1,4 @@ -import {TomlFile, TomlFileNotFoundError, TomlParseError} from './toml-file.js' +import {TomlFile, TomlFileError} from './toml-file.js' import {writeFile, readFile, inTemporaryDirectory} from '../fs.js' import {joinPath} from '../path.js' import {describe, expect, test} from 'vitest' @@ -28,18 +28,18 @@ describe('TomlFile', () => { }) }) - test('throws TomlParseError with file path on invalid TOML', async () => { + test('throws TomlFileError with file path on invalid TOML', async () => { await inTemporaryDirectory(async (dir) => { const path = joinPath(dir, 'bad.toml') await writeFile(path, 'name = [invalid') - await expect(TomlFile.read(path)).rejects.toThrow(TomlParseError) - await expect(TomlFile.read(path)).rejects.toThrow(/bad\.toml/) + await expect(TomlFile.read(path)).rejects.toThrow(TomlFileError) + await expect(TomlFile.read(path)).rejects.toThrow(/row.*col/) }) }) - test('throws TomlFileNotFoundError if file does not exist', async () => { - await expect(TomlFile.read('/nonexistent/path/test.toml')).rejects.toThrow(TomlFileNotFoundError) + test('throws TomlFileError if file does not exist', async () => { + await expect(TomlFile.read('/nonexistent/path/test.toml')).rejects.toThrow(TomlFileError) }) }) @@ -267,14 +267,14 @@ describe('TomlFile', () => { }) }) - test('throws TomlParseError and does not write to disk when transform produces invalid TOML', async () => { + test('throws TomlFileError and does not write to disk when transform produces invalid TOML', async () => { await inTemporaryDirectory(async (dir) => { const path = joinPath(dir, 'test.toml') const originalContent = 'name = "app"\n' await writeFile(path, originalContent) const file = await TomlFile.read(path) - await expect(file.transformRaw(() => 'name = [invalid')).rejects.toThrow(TomlParseError) + await expect(file.transformRaw(() => 'name = [invalid')).rejects.toThrow(TomlFileError) const raw = await readFile(path) expect(raw).toBe(originalContent) diff --git a/packages/cli-kit/src/public/node/toml/toml-file.ts b/packages/cli-kit/src/public/node/toml/toml-file.ts index 56e3ea7866..2100fd4ea4 100644 --- a/packages/cli-kit/src/public/node/toml/toml-file.ts +++ b/packages/cli-kit/src/public/node/toml/toml-file.ts @@ -4,34 +4,16 @@ import {updateTomlValues} from '@shopify/toml-patch' type TomlPatchValue = string | number | boolean | undefined | (string | number | boolean)[] -/** An error on a TOML file — missing or malformed. */ -export interface TomlFileError { - readonly path: string - readonly message: string -} - -/** - * Thrown when a TOML file does not exist at the expected path. - */ -export class TomlFileNotFoundError extends Error { - readonly path: string - - constructor(path: string) { - super(`TOML file not found: ${path}`) - this.name = 'TomlFileNotFoundError' - this.path = path - } -} - /** - * Thrown when a TOML file cannot be parsed. Includes the file path for context. + * An error on a TOML file — missing or malformed. + * Extends Error so it can be thrown. Carries path and a clean message suitable for JSON output. */ -export class TomlParseError extends Error { +export class TomlFileError extends Error { readonly path: string - constructor(path: string, cause: Error) { - super(`Fix the following error in ${path}:\n${cause.message}`) - this.name = 'TomlParseError' + constructor(path: string, message: string) { + super(message) + this.name = 'TomlFileError' this.path = path } } @@ -50,15 +32,15 @@ export class TomlParseError extends Error { */ export class TomlFile { /** - * Read and parse a TOML file from disk. Throws if the file doesn't exist or contains invalid TOML. - * Parse errors are wrapped in {@link TomlParseError} with the file path for context. + * Read and parse a TOML file from disk. Throws {@link TomlFileError} if the file + * doesn't exist or contains invalid TOML. * * @param path - Absolute path to the TOML file. * @returns A TomlFile instance with parsed content. */ static async read(path: string): Promise { if (!(await fileExists(path))) { - throw new TomlFileNotFoundError(path) + throw new TomlFileError(path, `TOML file not found: ${path}`) } const raw = await readFile(path) const file = new TomlFile(path, {}) @@ -160,7 +142,8 @@ export class TomlFile { // eslint-disable-next-line @typescript-eslint/no-explicit-any } catch (err: any) { if (err.line !== undefined && err.col !== undefined) { - throw new TomlParseError(this.path, err) + const description = String(err.message).split('\n')[0] ?? 'Invalid TOML' + throw new TomlFileError(this.path, `${description} at row ${err.line}, col ${err.col}`) } throw err } From 3b9b24136e619e9adb671a29d84ccc65918b00cb Mon Sep 17 00:00:00 2001 From: Ryan Bahan Date: Mon, 30 Mar 2026 16:16:06 -0600 Subject: [PATCH 3/4] Add errors to TomlFile and Project, surface TOML parse failures in validate --json Co-Authored-By: Claude Opus 4.6 (1M context) --- packages/app/src/cli/commands/app/config/validate.test.ts | 6 ++---- packages/app/src/cli/commands/app/config/validate.ts | 2 -- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/app/src/cli/commands/app/config/validate.test.ts b/packages/app/src/cli/commands/app/config/validate.test.ts index 50fa06e527..7ff92f32e8 100644 --- a/packages/app/src/cli/commands/app/config/validate.test.ts +++ b/packages/app/src/cli/commands/app/config/validate.test.ts @@ -55,14 +55,12 @@ describe('app config validate command', () => { test('outputs JSON issues when project has TOML parse errors', async () => { vi.mocked(Project.load).mockResolvedValue({ - errors: [{path: '/app/shopify.app.toml', message: 'Fix the following error in shopify.app.toml:\nUnexpected character'}], + 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(outputResult).toHaveBeenCalledWith(expect.stringContaining('"valid": false')) expect(linkedAppContext).not.toHaveBeenCalled() }) }) diff --git a/packages/app/src/cli/commands/app/config/validate.ts b/packages/app/src/cli/commands/app/config/validate.ts index 07cec6f691..fffcc2fc0f 100644 --- a/packages/app/src/cli/commands/app/config/validate.ts +++ b/packages/app/src/cli/commands/app/config/validate.ts @@ -2,8 +2,6 @@ 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 {getAppConfigurationContext} from '../../../models/app/loader.js' -import {selectActiveConfig} from '../../../models/project/active-config.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' From eebfb2053d6246bd14b7e6697581c9fb96422e40 Mon Sep 17 00:00:00 2001 From: Ryan Bahan Date: Mon, 30 Mar 2026 16:30:57 -0600 Subject: [PATCH 4/4] Add errors to TomlFile and Project, surface TOML parse failures in validate --json Co-Authored-By: Claude Opus 4.6 (1M context) --- packages/app/src/cli/services/app-context.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/app/src/cli/services/app-context.ts b/packages/app/src/cli/services/app-context.ts index ddd25de060..411e82d064 100644 --- a/packages/app/src/cli/services/app-context.ts +++ b/packages/app/src/cli/services/app-context.ts @@ -85,6 +85,10 @@ export async function linkedAppContext({ let {project, activeConfig} = await getAppConfigurationContext(directory, userProvidedConfigName) let remoteApp: OrganizationApp | undefined + if (activeConfig.file.errors.length > 0) { + throw new AbortError(activeConfig.file.errors.map((err) => err.message).join('\n')) + } + if (!activeConfig.isLinked || forceRelink) { const configName = forceRelink ? undefined : basename(activeConfig.file.path) const result = await link({directory, apiKey: clientId, configName})