From 8f897cf0fb6e82de17a39b4b2244b908a129b9f8 Mon Sep 17 00:00:00 2001 From: Donald Merand Date: Fri, 20 Mar 2026 16:12:34 -0400 Subject: [PATCH 1/3] Expose structured app validation issues in JSON output --- .../app/src/cli/commands/app/validate.test.ts | 69 +++- packages/app/src/cli/commands/app/validate.ts | 89 ++++- .../app/src/cli/services/validate.test.ts | 353 +++++++++++++++++- packages/app/src/cli/services/validate.ts | 49 ++- 4 files changed, 542 insertions(+), 18 deletions(-) diff --git a/packages/app/src/cli/commands/app/validate.test.ts b/packages/app/src/cli/commands/app/validate.test.ts index bcbe7a2661..670f12e2a9 100644 --- a/packages/app/src/cli/commands/app/validate.test.ts +++ b/packages/app/src/cli/commands/app/validate.test.ts @@ -2,10 +2,23 @@ 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 {describe, expect, test, vi} from 'vitest' +import {beforeEach, describe, expect, test, vi} from 'vitest' +import {AbortError} from '@shopify/cli-kit/node/error' +import {outputResult} from '@shopify/cli-kit/node/output' vi.mock('../../services/app-context.js') vi.mock('../../services/validate.js') +vi.mock('@shopify/cli-kit/node/output', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + outputResult: vi.fn(), + } +}) + +beforeEach(() => { + vi.clearAllMocks() +}) describe('app validate command', () => { test('calls validateApp with json: false by default', async () => { @@ -46,4 +59,58 @@ describe('app validate command', () => { // Then expect(validateApp).toHaveBeenCalledWith(app, {json: true}) }) + + test('outputs json issues when app loading aborts before validateApp runs', async () => { + // Given + vi.mocked(linkedAppContext).mockRejectedValue( + new AbortError('Validation errors in /tmp/shopify.app.toml:\n\n• [name]: String is required'), + ) + + // When / Then + await Validate.run(['--json', '--path=/tmp/app'], import.meta.url).catch(() => {}) + expect(outputResult).toHaveBeenCalledWith( + JSON.stringify( + { + valid: false, + issues: [ + { + filePath: '/tmp/shopify.app.toml', + path: ['name'], + pathString: 'name', + message: 'String is required', + }, + ], + }, + null, + 2, + ), + ) + expect(validateApp).not.toHaveBeenCalled() + }) + + test('outputs a root json issue when app loading aborts with a non-structured message', async () => { + // Given + vi.mocked(linkedAppContext).mockRejectedValue(new AbortError("Couldn't find an app toml file at /tmp/app")) + + // When / Then + await Validate.run(['--json', '--path=/tmp/app'], import.meta.url).catch(() => {}) + expect(outputResult).toHaveBeenCalledWith( + JSON.stringify( + { + valid: false, + issues: [ + { + filePath: '/tmp/app', + path: [], + pathString: 'root', + message: "Couldn't find an app toml file at /tmp/app", + }, + ], + }, + null, + 2, + ), + ) + expect(validateApp).not.toHaveBeenCalled() + }) }) diff --git a/packages/app/src/cli/commands/app/validate.ts b/packages/app/src/cli/commands/app/validate.ts index 3e426a2140..d32b4cc5dd 100644 --- a/packages/app/src/cli/commands/app/validate.ts +++ b/packages/app/src/cli/commands/app/validate.ts @@ -1,8 +1,61 @@ import {appFlags} from '../../flags.js' import {validateApp} from '../../services/validate.js' +import type {AppValidationIssue} from '../../models/app/error-parsing.js' import AppLinkedCommand, {AppLinkedCommandOutput} from '../../utilities/app-linked-command.js' import {linkedAppContext} from '../../services/app-context.js' import {globalFlags, jsonFlag} from '@shopify/cli-kit/node/cli' +import {AbortError, AbortSilentError} from '@shopify/cli-kit/node/error' +import {outputResult} from '@shopify/cli-kit/node/output' + +function parseIssuePath(pathString: string): (string | number)[] { + if (pathString === 'root') return [] + + return pathString.split('.').map((segment) => { + const parsed = Number(segment) + return Number.isInteger(parsed) && String(parsed) === segment ? parsed : segment + }) +} + +function toJsonIssuesFromAbortError(error: AbortError, fallbackFilePath: string): AppValidationIssue[] { + const message = error.message.trim() + const match = message.match(/^.*?Validation errors in\s+([^:\n]+):\n\n([\s\S]*)$/) + + if (!match) { + return [ + { + filePath: fallbackFilePath, + path: [], + pathString: 'root', + message, + }, + ] + } + + const filePath = match[1]! + const body = match[2]! + const issues = Array.from(body.matchAll(/^• \[([^\]]+)\]: (.+)$/gm)).map((captures) => { + const pathString = captures[1]! + const issueMessage = captures[2]! + + return { + filePath, + path: parseIssuePath(pathString), + pathString, + message: issueMessage, + } + }) + + if (issues.length > 0) return issues + + return [ + { + filePath, + path: [], + pathString: 'root', + message, + }, + ] +} export default class Validate extends AppLinkedCommand { static summary = 'Validate your app configuration and extensions.' @@ -20,16 +73,34 @@ export default class Validate extends AppLinkedCommand { public async run(): Promise { const {flags} = await this.parse(Validate) - const {app} = await linkedAppContext({ - directory: flags.path, - clientId: flags['client-id'], - forceRelink: flags.reset, - userProvidedConfigName: flags.config, - unsafeReportMode: true, - }) + try { + const {app} = await linkedAppContext({ + directory: flags.path, + clientId: flags['client-id'], + forceRelink: flags.reset, + userProvidedConfigName: flags.config, + unsafeReportMode: true, + }) + + await validateApp(app, {json: flags.json}) - await validateApp(app, {json: flags.json}) + return {app} + } catch (error) { + if (flags.json && error instanceof AbortError) { + outputResult( + JSON.stringify( + { + valid: false, + issues: toJsonIssuesFromAbortError(error, flags.path), + }, + null, + 2, + ), + ) + throw new AbortSilentError() + } - return {app} + throw error + } } } diff --git a/packages/app/src/cli/services/validate.test.ts b/packages/app/src/cli/services/validate.test.ts index c475c4399d..bce9a1a598 100644 --- a/packages/app/src/cli/services/validate.test.ts +++ b/packages/app/src/cli/services/validate.test.ts @@ -1,7 +1,7 @@ import {validateApp} from './validate.js' import {testAppLinked} from '../models/app/app.test-data.js' import {AppErrors} from '../models/app/loader.js' -import {describe, expect, test, vi} from 'vitest' +import {beforeEach, describe, expect, test, vi} from 'vitest' import {outputResult} from '@shopify/cli-kit/node/output' import {renderError, renderSuccess} from '@shopify/cli-kit/node/ui' import {AbortSilentError} from '@shopify/cli-kit/node/error' @@ -15,6 +15,10 @@ vi.mock('@shopify/cli-kit/node/output', async (importOriginal) => { }) vi.mock('@shopify/cli-kit/node/ui') +beforeEach(() => { + vi.clearAllMocks() +}) + describe('validateApp', () => { test('renders success when there are no errors', async () => { // Given @@ -37,7 +41,7 @@ describe('validateApp', () => { await validateApp(app, {json: true}) // Then - expect(outputResult).toHaveBeenCalledWith(JSON.stringify({valid: true, errors: []}, null, 2)) + expect(outputResult).toHaveBeenCalledWith(JSON.stringify({valid: true, issues: []}, null, 2)) expect(renderSuccess).not.toHaveBeenCalled() expect(renderError).not.toHaveBeenCalled() }) @@ -54,16 +58,24 @@ describe('validateApp', () => { await expect(validateApp(app)).rejects.toThrow(AbortSilentError) expect(renderError).toHaveBeenCalledWith({ headline: 'Validation errors found.', - body: expect.stringContaining('client_id is required'), + body: 'client_id is required\n\ninvalid type "unknown"', }) expect(renderSuccess).not.toHaveBeenCalled() expect(outputResult).not.toHaveBeenCalled() }) - test('outputs json errors and throws when --json is enabled and there are validation errors', async () => { + test('outputs structured json issues and throws when --json is enabled and there are validation errors', async () => { // Given const errors = new AppErrors() - errors.addError('/path/to/shopify.app.toml', 'client_id is required') + errors.addError('/path/to/shopify.app.toml', 'App configuration is not valid\nValidation errors in /path/to/shopify.app.toml:\n\n• [client_id]: Required', [ + { + filePath: '/path/to/shopify.app.toml', + path: ['client_id'], + pathString: 'client_id', + message: 'Required', + code: 'invalid_type', + }, + ]) errors.addError('/path/to/extensions/my-ext/shopify.extension.toml', 'invalid type "unknown"') const app = testAppLinked() app.errors = errors @@ -74,7 +86,21 @@ describe('validateApp', () => { JSON.stringify( { valid: false, - errors: ['client_id is required', 'invalid type "unknown"'], + issues: [ + { + filePath: '/path/to/shopify.app.toml', + path: ['client_id'], + pathString: 'client_id', + message: 'Required', + code: 'invalid_type', + }, + { + filePath: '/path/to/extensions/my-ext/shopify.extension.toml', + path: [], + pathString: 'root', + message: 'invalid type "unknown"', + }, + ], }, null, 2, @@ -84,6 +110,321 @@ describe('validateApp', () => { expect(renderSuccess).not.toHaveBeenCalled() }) + test('flattens multiple structured issues across files in json mode', async () => { + // Given + const errors = new AppErrors() + errors.addError( + '/path/to/shopify.app.toml', + 'Validation errors in /path/to/shopify.app.toml:\n\n• [client_id]: Required\n• [name]: Must be set', + [ + { + filePath: '/path/to/shopify.app.toml', + path: ['client_id'], + pathString: 'client_id', + message: 'Required', + code: 'invalid_type', + }, + { + filePath: '/path/to/shopify.app.toml', + path: ['name'], + pathString: 'name', + message: 'Must be set', + code: 'custom', + }, + ], + ) + errors.addError( + '/path/to/extensions/my-ext/shopify.extension.toml', + 'Validation errors in /path/to/extensions/my-ext/shopify.extension.toml:\n\n• [targeting.0.target]: Invalid target', + [ + { + filePath: '/path/to/extensions/my-ext/shopify.extension.toml', + path: ['targeting', 0, 'target'], + pathString: 'targeting.0.target', + message: 'Invalid target', + code: 'custom', + }, + ], + ) + const app = testAppLinked() + app.errors = errors + + // When / Then + await expect(validateApp(app, {json: true})).rejects.toThrow(AbortSilentError) + expect(outputResult).toHaveBeenCalledWith( + JSON.stringify( + { + valid: false, + issues: [ + { + filePath: '/path/to/shopify.app.toml', + path: ['client_id'], + pathString: 'client_id', + message: 'Required', + code: 'invalid_type', + }, + { + filePath: '/path/to/shopify.app.toml', + path: ['name'], + pathString: 'name', + message: 'Must be set', + code: 'custom', + }, + { + filePath: '/path/to/extensions/my-ext/shopify.extension.toml', + path: ['targeting', 0, 'target'], + pathString: 'targeting.0.target', + message: 'Invalid target', + code: 'custom', + }, + ], + }, + null, + 2, + ), + ) + }) + + test('preserves structured issues and appends a root issue for same-file unstructured follow-up errors', async () => { + // Given + const errors = new AppErrors() + errors.addError('tmp/shopify.app.toml', 'Validation errors in tmp/shopify.app.toml:\n\n• [client_id]: Required', [ + { + filePath: 'tmp/shopify.app.toml', + path: ['client_id'], + pathString: 'client_id', + message: 'Required', + code: 'invalid_type', + }, + ]) + errors.addError('tmp/shopify.app.toml', 'Could not infer extension handle from configuration') + const app = testAppLinked() + app.errors = errors + + // When / Then + await expect(validateApp(app, {json: true})).rejects.toThrow(AbortSilentError) + expect(outputResult).toHaveBeenCalledWith( + JSON.stringify( + { + valid: false, + issues: [ + { + filePath: 'tmp/shopify.app.toml', + path: ['client_id'], + pathString: 'client_id', + message: 'Required', + code: 'invalid_type', + }, + { + filePath: 'tmp/shopify.app.toml', + path: [], + pathString: 'root', + message: 'Could not infer extension handle from configuration', + }, + ], + }, + null, + 2, + ), + ) + }) + + test('outputs json success when errors object exists but is empty', async () => { + // Given + const errors = new AppErrors() + const app = testAppLinked() + app.errors = errors + + // When + await validateApp(app, {json: true}) + + // Then + expect(outputResult).toHaveBeenCalledWith(JSON.stringify({valid: true, issues: []}, null, 2)) + expect(renderSuccess).not.toHaveBeenCalled() + expect(renderError).not.toHaveBeenCalled() + }) + + test('uses structured issues without a synthetic root issue when the rendered message is only bullet lines', async () => { + // Given + const errors = new AppErrors() + errors.addError('tmp/shopify.app.toml', '• [client_id]: Required', [ + { + filePath: 'tmp/shopify.app.toml', + path: ['client_id'], + pathString: 'client_id', + message: 'Required', + code: 'invalid_type', + }, + ]) + const app = testAppLinked() + app.errors = errors + + // When / Then + await expect(validateApp(app, {json: true})).rejects.toThrow(AbortSilentError) + expect(outputResult).toHaveBeenCalledWith( + JSON.stringify( + { + valid: false, + issues: [ + { + filePath: 'tmp/shopify.app.toml', + path: ['client_id'], + pathString: 'client_id', + message: 'Required', + code: 'invalid_type', + }, + ], + }, + null, + 2, + ), + ) + }) + + test('does not append a synthetic root issue for repeated same-file structured errors', async () => { + // Given + const errors = new AppErrors() + errors.addError('tmp/shopify.app.toml', 'Validation errors in tmp/shopify.app.toml:\n\n• [client_id]: Required', [ + { + filePath: 'tmp/shopify.app.toml', + path: ['client_id'], + pathString: 'client_id', + message: 'Required', + code: 'invalid_type', + }, + ]) + errors.addError('tmp/shopify.app.toml', 'Validation errors in tmp/shopify.app.toml:\n\n• [name]: Must be set', [ + { + filePath: 'tmp/shopify.app.toml', + path: ['name'], + pathString: 'name', + message: 'Must be set', + code: 'custom', + }, + ]) + const app = testAppLinked() + app.errors = errors + + // When / Then + await expect(validateApp(app, {json: true})).rejects.toThrow(AbortSilentError) + expect(outputResult).toHaveBeenCalledWith( + JSON.stringify( + { + valid: false, + issues: [ + { + filePath: 'tmp/shopify.app.toml', + path: ['client_id'], + pathString: 'client_id', + message: 'Required', + code: 'invalid_type', + }, + { + filePath: 'tmp/shopify.app.toml', + path: ['name'], + pathString: 'name', + message: 'Must be set', + code: 'custom', + }, + ], + }, + null, + 2, + ), + ) + }) + + test('preserves a root issue when structured bullets are prefixed by meaningful context', async () => { + // Given + const errors = new AppErrors() + errors.addError('tmp/shopify.app.toml', 'Could not infer extension handle\n\n• [client_id]: Required', [ + { + filePath: 'tmp/shopify.app.toml', + path: ['client_id'], + pathString: 'client_id', + message: 'Required', + code: 'invalid_type', + }, + ]) + const app = testAppLinked() + app.errors = errors + + // When / Then + await expect(validateApp(app, {json: true})).rejects.toThrow(AbortSilentError) + expect(outputResult).toHaveBeenCalledWith( + JSON.stringify( + { + valid: false, + issues: [ + { + filePath: 'tmp/shopify.app.toml', + path: ['client_id'], + pathString: 'client_id', + message: 'Required', + code: 'invalid_type', + }, + { + filePath: 'tmp/shopify.app.toml', + path: [], + pathString: 'root', + message: 'Could not infer extension handle\n\n• [client_id]: Required', + }, + ], + }, + null, + 2, + ), + ) + }) + + test('preserves a root issue when contextual text precedes a validation wrapper', async () => { + // Given + const errors = new AppErrors() + errors.addError( + 'tmp/shopify.app.toml', + 'Could not infer extension handle\n\nValidation errors in tmp/shopify.app.toml:\n\n• [client_id]: Required', + [ + { + filePath: 'tmp/shopify.app.toml', + path: ['client_id'], + pathString: 'client_id', + message: 'Required', + code: 'invalid_type', + }, + ], + ) + const app = testAppLinked() + app.errors = errors + + // When / Then + await expect(validateApp(app, {json: true})).rejects.toThrow(AbortSilentError) + expect(outputResult).toHaveBeenCalledWith( + JSON.stringify( + { + valid: false, + issues: [ + { + filePath: 'tmp/shopify.app.toml', + path: ['client_id'], + pathString: 'client_id', + message: 'Required', + code: 'invalid_type', + }, + { + filePath: 'tmp/shopify.app.toml', + path: [], + pathString: 'root', + message: + 'Could not infer extension handle\n\nValidation errors in tmp/shopify.app.toml:\n\n• [client_id]: Required', + }, + ], + }, + null, + 2, + ), + ) + }) + test('renders success when errors object exists but is empty', async () => { // Given const errors = new AppErrors() diff --git a/packages/app/src/cli/services/validate.ts b/packages/app/src/cli/services/validate.ts index d27efebeea..465b868937 100644 --- a/packages/app/src/cli/services/validate.ts +++ b/packages/app/src/cli/services/validate.ts @@ -1,4 +1,5 @@ import {AppLinkedInterface} from '../models/app/app.js' +import type {AppValidationIssue} from '../models/app/error-parsing.js' import {outputResult, stringifyMessage} from '@shopify/cli-kit/node/output' import {renderError, renderSuccess} from '@shopify/cli-kit/node/ui' import {AbortSilentError} from '@shopify/cli-kit/node/error' @@ -7,12 +8,56 @@ interface ValidateAppOptions { json: boolean } +function toRootIssue(filePath: string, message: string): AppValidationIssue { + return { + filePath, + path: [], + pathString: 'root', + message, + } +} + +function isStructuredMessageEquivalent(message: string, issues: AppValidationIssue[]): boolean { + if (issues.length === 0) return false + + const normalizedMessage = message.trim().replace(/^App configuration is not valid\n/, '') + + for (let startIndex = 0; startIndex < issues.length; startIndex++) { + const issueLines = issues + .slice(startIndex) + .map((issue) => `• [${issue.pathString}]: ${issue.message}`) + .join('\n') + + if (normalizedMessage === issueLines) { + return true + } + + const isValidationWrapper = normalizedMessage.startsWith('Validation errors in ') + if (isValidationWrapper && normalizedMessage.endsWith(`\n\n${issueLines}`)) { + return true + } + } + + return false +} + +function toPublicIssues(app: AppLinkedInterface): AppValidationIssue[] { + const structuredErrors = app.errors?.toStructuredJSON() ?? [] + + return structuredErrors.flatMap(({filePath, message, issues}) => { + const renderedMessage = stringifyMessage(message).trim() + if (issues.length === 0) return [toRootIssue(filePath, renderedMessage)] + if (isStructuredMessageEquivalent(renderedMessage, issues)) return issues + return [...issues, toRootIssue(filePath, renderedMessage)] + }) +} + export async function validateApp(app: AppLinkedInterface, options: ValidateAppOptions = {json: false}): Promise { const errors = app.errors if (!errors || errors.isEmpty()) { if (options.json) { - outputResult(JSON.stringify({valid: true, errors: []}, null, 2)) + outputResult(JSON.stringify({valid: true, issues: []}, null, 2)) return } @@ -23,7 +68,7 @@ export async function validateApp(app: AppLinkedInterface, options: ValidateAppO const errorMessages = errors.toJSON().map((error) => stringifyMessage(error).trim()) if (options.json) { - outputResult(JSON.stringify({valid: false, errors: errorMessages}, null, 2)) + outputResult(JSON.stringify({valid: false, issues: toPublicIssues(app)}, null, 2)) throw new AbortSilentError() } From a186dde1d9b795ef287003bc94e10537c4cf3f0a Mon Sep 17 00:00:00 2001 From: Donald Merand Date: Fri, 20 Mar 2026 16:20:27 -0400 Subject: [PATCH 2/3] Strip ANSI escape codes from app validate JSON errors --- .../app/src/cli/commands/app/validate.test.ts | 36 ++++++++++++++++--- packages/app/src/cli/commands/app/validate.ts | 7 ++-- 2 files changed, 35 insertions(+), 8 deletions(-) diff --git a/packages/app/src/cli/commands/app/validate.test.ts b/packages/app/src/cli/commands/app/validate.test.ts index 670f12e2a9..ac289be806 100644 --- a/packages/app/src/cli/commands/app/validate.test.ts +++ b/packages/app/src/cli/commands/app/validate.test.ts @@ -2,7 +2,7 @@ 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 {beforeEach, describe, expect, test, vi} from 'vitest' +import {describe, expect, test, vi} from 'vitest' import {AbortError} from '@shopify/cli-kit/node/error' import {outputResult} from '@shopify/cli-kit/node/output' @@ -16,10 +16,6 @@ vi.mock('@shopify/cli-kit/node/output', async (importOriginal) => { } }) -beforeEach(() => { - vi.clearAllMocks() -}) - describe('app validate command', () => { test('calls validateApp with json: false by default', async () => { // Given @@ -88,6 +84,36 @@ describe('app validate command', () => { expect(validateApp).not.toHaveBeenCalled() }) + test('outputs json issues when app loading aborts with ansi-colored structured text', async () => { + // Given + vi.mocked(linkedAppContext).mockRejectedValue( + new AbortError( + '\u001b[1m\u001b[91mValidation errors\u001b[39m\u001b[22m in /tmp/shopify.app.toml:\n\n• [name]: String is required', + ), + ) + + // When / Then + await Validate.run(['--json', '--path=/tmp/app'], import.meta.url).catch(() => {}) + expect(outputResult).toHaveBeenCalledWith( + JSON.stringify( + { + valid: false, + issues: [ + { + filePath: '/tmp/shopify.app.toml', + path: ['name'], + pathString: 'name', + message: 'String is required', + }, + ], + }, + null, + 2, + ), + ) + expect(validateApp).not.toHaveBeenCalled() + }) + test('outputs a root json issue when app loading aborts with a non-structured message', async () => { // Given vi.mocked(linkedAppContext).mockRejectedValue(new AbortError("Couldn't find an app toml file at /tmp/app")) diff --git a/packages/app/src/cli/commands/app/validate.ts b/packages/app/src/cli/commands/app/validate.ts index d32b4cc5dd..b7abb1d0a2 100644 --- a/packages/app/src/cli/commands/app/validate.ts +++ b/packages/app/src/cli/commands/app/validate.ts @@ -1,11 +1,12 @@ import {appFlags} from '../../flags.js' import {validateApp} from '../../services/validate.js' -import type {AppValidationIssue} from '../../models/app/error-parsing.js' import AppLinkedCommand, {AppLinkedCommandOutput} from '../../utilities/app-linked-command.js' import {linkedAppContext} from '../../services/app-context.js' import {globalFlags, jsonFlag} from '@shopify/cli-kit/node/cli' import {AbortError, AbortSilentError} from '@shopify/cli-kit/node/error' -import {outputResult} from '@shopify/cli-kit/node/output' +import {outputResult, unstyled} from '@shopify/cli-kit/node/output' + +import type {AppValidationIssue} from '../../models/app/error-parsing.js' function parseIssuePath(pathString: string): (string | number)[] { if (pathString === 'root') return [] @@ -17,7 +18,7 @@ function parseIssuePath(pathString: string): (string | number)[] { } function toJsonIssuesFromAbortError(error: AbortError, fallbackFilePath: string): AppValidationIssue[] { - const message = error.message.trim() + const message = unstyled(error.message).trim() const match = message.match(/^.*?Validation errors in\s+([^:\n]+):\n\n([\s\S]*)$/) if (!match) { From 9b0ba081d0f127063b743cf729767b9a50fd43ce Mon Sep 17 00:00:00 2001 From: Donald Merand Date: Fri, 20 Mar 2026 16:51:44 -0400 Subject: [PATCH 3/3] Preserve context in app validate JSON fallback --- .../app/src/cli/commands/app/validate.test.ts | 65 +++++++++++++++++++ packages/app/src/cli/commands/app/validate.ts | 57 +++++++++------- 2 files changed, 99 insertions(+), 23 deletions(-) diff --git a/packages/app/src/cli/commands/app/validate.test.ts b/packages/app/src/cli/commands/app/validate.test.ts index ac289be806..500a31d39f 100644 --- a/packages/app/src/cli/commands/app/validate.test.ts +++ b/packages/app/src/cli/commands/app/validate.test.ts @@ -114,6 +114,71 @@ describe('app validate command', () => { expect(validateApp).not.toHaveBeenCalled() }) + test('preserves a root json issue when contextual text precedes structured validation errors', async () => { + // Given + vi.mocked(linkedAppContext).mockRejectedValue( + new AbortError( + 'Could not infer extension handle\n\nValidation errors in /tmp/shopify.app.toml:\n\n• [name]: String is required', + ), + ) + + // When / Then + await Validate.run(['--json', '--path=/tmp/app'], import.meta.url).catch(() => {}) + expect(outputResult).toHaveBeenCalledWith( + JSON.stringify( + { + valid: false, + issues: [ + { + filePath: '/tmp/shopify.app.toml', + path: ['name'], + pathString: 'name', + message: 'String is required', + }, + { + filePath: '/tmp/shopify.app.toml', + path: [], + pathString: 'root', + message: + 'Could not infer extension handle\n\nValidation errors in /tmp/shopify.app.toml:\n\n• [name]: String is required', + }, + ], + }, + null, + 2, + ), + ) + expect(validateApp).not.toHaveBeenCalled() + }) + + test('parses structured validation errors for windows-style paths', async () => { + // Given + vi.mocked(linkedAppContext).mockRejectedValue( + new AbortError('Validation errors in C:\\tmp\\shopify.app.toml:\n\n• [name]: String is required'), + ) + + // When / Then + await Validate.run(['--json', '--path=/tmp/app'], import.meta.url).catch(() => {}) + expect(outputResult).toHaveBeenCalledWith( + JSON.stringify( + { + valid: false, + issues: [ + { + filePath: 'C:\\tmp\\shopify.app.toml', + path: ['name'], + pathString: 'name', + message: 'String is required', + }, + ], + }, + null, + 2, + ), + ) + expect(validateApp).not.toHaveBeenCalled() + }) + test('outputs a root json issue when app loading aborts with a non-structured message', async () => { // Given vi.mocked(linkedAppContext).mockRejectedValue(new AbortError("Couldn't find an app toml file at /tmp/app")) diff --git a/packages/app/src/cli/commands/app/validate.ts b/packages/app/src/cli/commands/app/validate.ts index b7abb1d0a2..4e10445c09 100644 --- a/packages/app/src/cli/commands/app/validate.ts +++ b/packages/app/src/cli/commands/app/validate.ts @@ -17,23 +17,41 @@ function parseIssuePath(pathString: string): (string | number)[] { }) } +function toRootIssue(filePath: string, message: string): AppValidationIssue { + return { + filePath, + path: [], + pathString: 'root', + message, + } +} + +function hasMeaningfulPrefix(prefix: string): boolean { + const normalizedPrefix = prefix.trim() + return normalizedPrefix !== '' && normalizedPrefix !== 'App configuration is not valid' +} + function toJsonIssuesFromAbortError(error: AbortError, fallbackFilePath: string): AppValidationIssue[] { const message = unstyled(error.message).trim() - const match = message.match(/^.*?Validation errors in\s+([^:\n]+):\n\n([\s\S]*)$/) - - if (!match) { - return [ - { - filePath: fallbackFilePath, - path: [], - pathString: 'root', - message, - }, - ] + const marker = 'Validation errors in ' + const markerIndex = message.indexOf(marker) + + if (markerIndex === -1) { + return [toRootIssue(fallbackFilePath, message)] } - const filePath = match[1]! - const body = match[2]! + const bodyStartIndex = message.indexOf('\n\n', markerIndex) + if (bodyStartIndex === -1) { + return [toRootIssue(fallbackFilePath, message)] + } + + const filePathLine = message.slice(markerIndex + marker.length, bodyStartIndex) + if (!filePathLine.endsWith(':')) { + return [toRootIssue(fallbackFilePath, message)] + } + + const filePath = filePathLine.slice(0, -1) + const body = message.slice(bodyStartIndex + 2) const issues = Array.from(body.matchAll(/^• \[([^\]]+)\]: (.+)$/gm)).map((captures) => { const pathString = captures[1]! const issueMessage = captures[2]! @@ -46,16 +64,9 @@ function toJsonIssuesFromAbortError(error: AbortError, fallbackFilePath: string) } }) - if (issues.length > 0) return issues - - return [ - { - filePath, - path: [], - pathString: 'root', - message, - }, - ] + if (issues.length === 0) return [toRootIssue(filePath, message)] + if (hasMeaningfulPrefix(message.slice(0, markerIndex))) return [...issues, toRootIssue(filePath, message)] + return issues } export default class Validate extends AppLinkedCommand {