Skip to content

Commit 5fcaa9f

Browse files
ryancbahanclaude
andcommitted
Add errors to TomlFile and Project, surface TOML parse failures in validate --json
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 9262a31 commit 5fcaa9f

4 files changed

Lines changed: 69 additions & 15 deletions

File tree

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

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,26 +3,33 @@ import {linkedAppContext} from '../../../services/app-context.js'
33
import {validateApp} from '../../../services/validate.js'
44
import {testAppLinked} from '../../../models/app/app.test-data.js'
55
import {Project} from '../../../models/project/project.js'
6+
import {selectActiveConfig} from '../../../models/project/active-config.js'
7+
import {errorsForConfig} from '../../../models/project/config-selection.js'
68
import {outputResult} from '@shopify/cli-kit/node/output'
9+
import {TomlFile} from '@shopify/cli-kit/node/toml/toml-file'
710
import {describe, expect, test, vi} from 'vitest'
811

912
vi.mock('../../../services/app-context.js')
1013
vi.mock('../../../services/validate.js')
1114
vi.mock('../../../models/project/project.js')
15+
vi.mock('../../../models/project/active-config.js')
16+
vi.mock('../../../models/project/config-selection.js')
1217
vi.mock('@shopify/cli-kit/node/output', async (importOriginal) => {
1318
const actual = await importOriginal<typeof import('@shopify/cli-kit/node/output')>()
1419
return {...actual, outputResult: vi.fn()}
1520
})
1621
vi.mock('@shopify/cli-kit/node/ui')
1722

18-
function mockProjectWithNoErrors() {
23+
function mockHealthyProject() {
1924
vi.mocked(Project.load).mockResolvedValue({errors: []} as unknown as Project)
25+
vi.mocked(selectActiveConfig).mockResolvedValue({file: new TomlFile('shopify.app.toml', {})} as any)
26+
vi.mocked(errorsForConfig).mockReturnValue([])
2027
}
2128

2229
describe('app config validate command', () => {
2330
test('calls validateApp with json: false by default', async () => {
2431
const app = testAppLinked()
25-
mockProjectWithNoErrors()
32+
mockHealthyProject()
2633
vi.mocked(linkedAppContext).mockResolvedValue({app} as Awaited<ReturnType<typeof linkedAppContext>>)
2734
vi.mocked(validateApp).mockResolvedValue()
2835

@@ -33,7 +40,7 @@ describe('app config validate command', () => {
3340

3441
test('calls validateApp with json: true when --json flag is passed', async () => {
3542
const app = testAppLinked()
36-
mockProjectWithNoErrors()
43+
mockHealthyProject()
3744
vi.mocked(linkedAppContext).mockResolvedValue({app} as Awaited<ReturnType<typeof linkedAppContext>>)
3845
vi.mocked(validateApp).mockResolvedValue()
3946

@@ -44,7 +51,7 @@ describe('app config validate command', () => {
4451

4552
test('calls validateApp with json: true when -j flag is passed', async () => {
4653
const app = testAppLinked()
47-
mockProjectWithNoErrors()
54+
mockHealthyProject()
4855
vi.mocked(linkedAppContext).mockResolvedValue({app} as Awaited<ReturnType<typeof linkedAppContext>>)
4956
vi.mocked(validateApp).mockResolvedValue()
5057

@@ -53,10 +60,12 @@ describe('app config validate command', () => {
5360
expect(validateApp).toHaveBeenCalledWith(app, {json: true})
5461
})
5562

56-
test('outputs JSON issues when project has TOML parse errors', async () => {
57-
vi.mocked(Project.load).mockResolvedValue({
58-
errors: [{path: '/app/shopify.app.toml', message: 'Unexpected character at row 1, col 5'}],
59-
} as unknown as Project)
63+
test('outputs JSON issues when active config has TOML parse errors', async () => {
64+
vi.mocked(Project.load).mockResolvedValue({errors: []} as unknown as Project)
65+
vi.mocked(selectActiveConfig).mockResolvedValue({file: new TomlFile('shopify.app.toml', {})} as any)
66+
vi.mocked(errorsForConfig).mockReturnValue([
67+
{path: '/app/shopify.app.toml', message: 'Unexpected character at row 1, col 5'} as any,
68+
])
6069

6170
await expect(Validate.run(['--json'], import.meta.url)).rejects.toThrow()
6271

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

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ import {appFlags} from '../../../flags.js'
22
import {validateApp} from '../../../services/validate.js'
33
import AppLinkedCommand, {AppLinkedCommandOutput} from '../../../utilities/app-linked-command.js'
44
import {linkedAppContext} from '../../../services/app-context.js'
5+
import {selectActiveConfig} from '../../../models/project/active-config.js'
6+
import {errorsForConfig} from '../../../models/project/config-selection.js'
57
import {Project} from '../../../models/project/project.js'
68
import {globalFlags, jsonFlag} from '@shopify/cli-kit/node/cli'
79
import {AbortError, AbortSilentError} from '@shopify/cli-kit/node/error'
@@ -24,9 +26,7 @@ export default class Validate extends AppLinkedCommand {
2426
public async run(): Promise<AppLinkedCommandOutput> {
2527
const {flags} = await this.parse(Validate)
2628

27-
// Check for TOML parse errors before attempting to link/load.
28-
// If the active config has parse errors, report them directly
29-
// instead of falling into the linking flow.
29+
// Stage 1: Load project
3030
let project: Project
3131
try {
3232
project = await Project.load(flags.path)
@@ -39,8 +39,22 @@ export default class Validate extends AppLinkedCommand {
3939
throw err
4040
}
4141

42-
if (project.errors.length > 0) {
43-
const issues = project.errors.map((err) => ({file: err.path, message: err.message}))
42+
// Stage 2: Select active config and check for TOML parse errors scoped to it
43+
let activeConfig
44+
try {
45+
activeConfig = await selectActiveConfig(project, flags.config)
46+
} catch (err) {
47+
if (err instanceof AbortError && flags.json) {
48+
const message = unstyled(stringifyMessage(err.message)).trim()
49+
outputResult(JSON.stringify({valid: false, issues: [{message}]}, null, 2))
50+
throw new AbortSilentError()
51+
}
52+
throw err
53+
}
54+
55+
const configErrors = errorsForConfig(project, activeConfig.file)
56+
if (configErrors.length > 0) {
57+
const issues = configErrors.map((err) => ({file: err.path, message: err.message}))
4458
if (flags.json) {
4559
outputResult(JSON.stringify({valid: false, issues}, null, 2))
4660
throw new AbortSilentError()
@@ -52,6 +66,7 @@ export default class Validate extends AppLinkedCommand {
5266
throw new AbortSilentError()
5367
}
5468

69+
// Stage 3: Load app (link + remote fetch + schema validation)
5570
let app
5671
try {
5772
const context = await linkedAppContext({
@@ -65,7 +80,6 @@ export default class Validate extends AppLinkedCommand {
6580
} catch (err) {
6681
// Only catch config validation errors for JSON output. Auth/linking/remote
6782
// failures should propagate normally — they aren't validation results.
68-
// This is a workaround while we consider more granular error types -- today most everything is AbortError.
6983
const message = err instanceof AbortError ? unstyled(stringifyMessage(err.message)).trim() : ''
7084
const isValidationError = message.startsWith('Validation errors in ')
7185
if (isValidationError && flags.json) {

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

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import {getAppConfigurationShorthand} from '../app/loader.js'
44
import {dotEnvFileNames} from '../../constants.js'
55
import {patchAppHiddenConfigFile} from '../../services/app/patch-app-configuration-file.js'
66
import {getOrCreateAppConfigHiddenPath} from '../../utilities/app/config/hidden-app-config.js'
7-
import {TomlFile} from '@shopify/cli-kit/node/toml/toml-file'
7+
import {TomlFile, TomlFileError} from '@shopify/cli-kit/node/toml/toml-file'
88
import {DotEnvFile} from '@shopify/cli-kit/node/dot-env'
99
import {matchGlob} from '@shopify/cli-kit/node/fs'
1010
import {relativePath} from '@shopify/cli-kit/node/path'
@@ -112,3 +112,29 @@ export function webFilesForConfig(project: Project, activeConfig: TomlFile): Tom
112112
return globPatterns.some((pattern) => matchGlob(relPath, pattern))
113113
})
114114
}
115+
116+
/**
117+
* Filter project.errors to only those relevant to the active config:
118+
* the active config file itself, plus extension/web TOMLs in its directories.
119+
* @public
120+
*/
121+
export function errorsForConfig(project: Project, activeConfig: TomlFile): TomlFileError[] {
122+
const extDirs = activeConfig.content.extension_directories
123+
const extPatterns = (Array.isArray(extDirs) && extDirs.length > 0 ? (extDirs as string[]) : ['extensions/*']).map(
124+
(dir) => `${dir}/*.extension.toml`,
125+
)
126+
127+
const webDirs = activeConfig.content.web_directories
128+
const webPatterns =
129+
Array.isArray(webDirs) && webDirs.length > 0
130+
? (webDirs as string[]).map((dir) => `${dir}/shopify.web.toml`)
131+
: ['**/shopify.web.toml']
132+
133+
const allPatterns = [...extPatterns, ...webPatterns]
134+
135+
return project.errors.filter((err) => {
136+
if (err.path === activeConfig.path) return true
137+
const relPath = relativePath(project.directory, err.path).replace(/\\/g, '/')
138+
return allPatterns.some((pattern) => matchGlob(relPath, pattern))
139+
})
140+
}

packages/app/src/cli/services/app-context.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,11 @@ export async function localAppContext({
177177
userProvidedConfigName,
178178
}: LocalAppContextOptions): Promise<LocalAppContextOutput> {
179179
const {project, activeConfig} = await getAppConfigurationContext(directory, userProvidedConfigName)
180+
181+
if (activeConfig.file.errors.length > 0) {
182+
throw new AbortError(activeConfig.file.errors.map((err) => err.message).join('\n'))
183+
}
184+
180185
const specifications = await loadLocalExtensionsSpecifications()
181186
const app = await loadAppFromContext({project, activeConfig, specifications, ignoreUnknownExtensions: true})
182187

0 commit comments

Comments
 (0)