Enhance union validation error messages with smart variant detection

lunataoo · delongGao · lunataoo · commit 48a361383dc5 · 2025-08-13T18:04:54.000-04:00
Co-authored-by: Delong Gao &lt;delong.gao@shopify.com&gt;
diff --git a/packages/app/src/cli/models/app/loader.test.ts b/packages/app/src/cli/models/app/loader.test.ts
@@ -2814,7 +2814,29 @@ describe('parseConfigurationObject', () => {
       roles: 1,
     }
 
-    const errorObject = [
+    const abortOrReport = vi.fn()
+    await parseConfigurationObject(WebConfigurationSchema, 'tmp', configurationObject, abortOrReport)
+
+    // Verify the function was called and capture the actual error structure
+    expect(abortOrReport).toHaveBeenCalledOnce()
+    const callArgs = abortOrReport.mock.calls[0]!
+    const actualErrorMessage = callArgs[0]
+
+    // Convert TokenizedString to regular string for testing
+    const errorString = actualErrorMessage.value
+
+    // The enhanced union handling should show only the most relevant errors
+    // instead of showing all variants, making it much more user-friendly
+    expect(errorString).toContain('[roles]: Expected array, received number')
+
+    // Should NOT show the confusing union variant breakdown
+    expect(errorString).not.toContain('Union validation failed')
+    expect(errorString).not.toContain('Variant 1:')
+    expect(errorString).not.toContain('Variant 2:')
+  })
+
+  test('parseHumanReadableError formats union errors with smart variant detection', () => {
+    const unionErrorObject = [
       {
         code: 'invalid_union',
         unionErrors: [
@@ -2824,36 +2846,226 @@ describe('parseConfigurationObject', () => {
                 code: 'invalid_type',
                 expected: 'array',
                 received: 'number',
-                path: ['roles'],
+                path: ['web', 'roles'],
                 message: 'Expected array, received number',
               },
+              {
+                code: 'invalid_type',
+                expected: 'string',
+                received: 'undefined',
+                path: ['web', 'commands', 'build'],
+                message: 'Required',
+              },
             ],
             name: 'ZodError',
           },
           {
             issues: [
               {
-                expected: "'frontend' | 'backend' | 'background'",
+                code: 'invalid_literal',
+                expected: "'frontend'",
                 received: 'number',
-                code: 'invalid_type',
-                path: ['type'],
-                message: "Expected 'frontend' | 'backend' | 'background', received number",
+                path: ['web', 'type'],
+                message: "Invalid literal value, expected 'frontend'",
               },
             ],
             name: 'ZodError',
           },
         ],
+        path: ['web'],
+        message: 'Invalid input',
+      },
+    ]
+
+    const result = parseHumanReadableError(unionErrorObject)
+
+    // Verify the enhanced format shows only the best matching variant's errors
+    // (Variant 1 has both missing field + type error, so it's likely the intended one)
+    expect(result).toContain('[web.roles]: Expected array, received number')
+    expect(result).toContain('[web.commands.build]: Required')
+
+    // Should NOT show confusing union variant breakdown
+    expect(result).not.toContain('Union validation failed')
+    expect(result).not.toContain('Variant 1:')
+    expect(result).not.toContain('Variant 2:')
+
+    // Should NOT show errors from the less likely variant 2
+    expect(result).not.toContain("[web.type]: Invalid literal value, expected 'frontend'")
+  })
+
+  test('parseHumanReadableError handles regular non-union errors', () => {
+    const regularErrorObject = [
+      {
+        path: ['name'],
+        message: 'Required field is missing',
+      },
+      {
+        path: ['version'],
+        message: 'Must be a valid semver string',
+      },
+    ]
+
+    const result = parseHumanReadableError(regularErrorObject)
+
+    // Verify regular errors still work as expected
+    expect(result).toBe('• [name]: Required field is missing\n• [version]: Must be a valid semver string\n')
+    expect(result).not.toContain('Union validation failed')
+  })
+
+  test('parseHumanReadableError handles edge cases for union error detection', () => {
+    // Test case 1: Union error with no unionErrors array
+    const noUnionErrors = [
+      {
+        code: 'invalid_union',
+        path: ['root'],
+        message: 'Invalid input',
+      },
+    ]
+
+    const result1 = parseHumanReadableError(noUnionErrors)
+    expect(result1).toBe('• [root]: Invalid input\n')
+
+    // Test case 2: Union error with empty unionErrors array - should fall back to showing full union error
+    const emptyUnionErrors = [
+      {
+        code: 'invalid_union',
+        unionErrors: [],
+        path: ['root'],
+        message: 'Invalid input',
+      },
+    ]
+
+    const result2 = parseHumanReadableError(emptyUnionErrors)
+    expect(result2).toContain('Union validation failed. None of the possible variants matched:')
+
+    // Test case 3: Union errors with variants that have no issues - results in empty string
+    const noIssuesVariants = [
+      {
+        code: 'invalid_union',
+        unionErrors: [
+          {issues: [], name: 'ZodError'},
+          {issues: [], name: 'ZodError'},
+        ],
+        path: ['root'],
+        message: 'Invalid input',
+      },
+    ]
+
+    const result3 = parseHumanReadableError(noIssuesVariants)
+    // When all variants have no issues, the best variant selection returns null issues
+    // resulting in no output, which falls back to the union error display
+    expect(result3).toContain('Union validation failed. None of the possible variants matched:')
+  })
+
+  test('findBestMatchingVariant scoring logic works correctly', () => {
+    // Test various scoring scenarios by creating mock union errors
+    const scenarioWithMissingFields = [
+      {
+        code: 'invalid_union',
+        unionErrors: [
+          {
+            // Variant with missing fields - should score highest
+            issues: [
+              {path: ['supports_moto'], message: 'Required'},
+              {path: ['merchant_label'], message: 'Required'},
+            ],
+            name: 'ZodError',
+          },
+          {
+            // Variant with only type errors - should score lower
+            issues: [
+              {path: ['type'], message: 'Expected string, received number'},
+              {path: ['id'], message: 'Expected number, received string'},
+            ],
+            name: 'ZodError',
+          },
+          {
+            // Variant with other errors - should score lowest
+            issues: [{path: ['url'], message: 'Invalid URL format'}],
+            name: 'ZodError',
+          },
+        ],
         path: [],
         message: 'Invalid input',
       },
     ]
-    const expectedFormatted = outputContent`\n${outputToken.errorText(
-      'Validation errors',
-    )} in tmp:\n\n${parseHumanReadableError(errorObject)}`
-    const abortOrReport = vi.fn()
-    await parseConfigurationObject(WebConfigurationSchema, 'tmp', configurationObject, abortOrReport)
 
-    expect(abortOrReport).toHaveBeenCalledWith(expectedFormatted, {}, 'tmp')
+    const result = parseHumanReadableError(scenarioWithMissingFields)
+
+    // Should show only the variant with missing fields (highest score)
+    expect(result).toContain('[supports_moto]: Required')
+    expect(result).toContain('[merchant_label]: Required')
+
+    // Should NOT show errors from other variants
+    expect(result).not.toContain('Expected string, received number')
+    expect(result).not.toContain('Invalid URL format')
+    expect(result).not.toContain('Union validation failed')
+  })
+
+  test('parseHumanReadableError handles undefined messages gracefully', () => {
+    const undefinedMessageError = [
+      {
+        path: ['field'],
+        message: undefined,
+      },
+      {
+        path: [],
+        // message is completely missing
+      },
+    ]
+
+    const result = parseHumanReadableError(undefinedMessageError)
+
+    expect(result).toBe('• [field]: Unknown error\n• [root]: Unknown error\n')
+  })
+
+  test('parseHumanReadableError handles mixed scoring scenarios', () => {
+    // Test scenario where we need to pick between variants with different error combinations
+    const mixedScenario = [
+      {
+        code: 'invalid_union',
+        unionErrors: [
+          {
+            // Mix of missing fields and type errors - this should win due to missing fields
+            issues: [
+              {path: ['required_field'], message: 'Required'},
+              {path: ['wrong_type'], message: 'Expected string, received number'},
+            ],
+            name: 'ZodError',
+          },
+          {
+            // Only type errors - should lose
+            issues: [
+              {path: ['field1'], message: 'Expected boolean, received string'},
+              {path: ['field2'], message: 'Expected array, received object'},
+              {path: ['field3'], message: 'Expected number, received string'},
+            ],
+            name: 'ZodError',
+          },
+          {
+            // Only other validation errors - should score lowest
+            issues: [
+              {path: ['url'], message: 'Must be valid URL'},
+              {path: ['email'], message: 'Must be valid email'},
+            ],
+            name: 'ZodError',
+          },
+        ],
+        path: [],
+        message: 'Invalid input',
+      },
+    ]
+
+    const result = parseHumanReadableError(mixedScenario)
+
+    // Should pick the variant with missing field (even though it has fewer total errors)
+    expect(result).toContain('[required_field]: Required')
+    expect(result).toContain('[wrong_type]: Expected string, received number')
+
+    // Should not show errors from other variants
+    expect(result).not.toContain('Expected boolean, received string')
+    expect(result).not.toContain('Must be valid URL')
+    expect(result).not.toContain('Union validation failed')
   })
 })
 
diff --git a/packages/app/src/cli/models/app/loader.ts b/packages/app/src/cli/models/app/loader.ts
@@ -126,12 +126,107 @@ export async function parseConfigurationFile<TSchema extends zod.ZodType>(
   return {...configuration, path: filepath}
 }
 
-export function parseHumanReadableError(issues: Pick<zod.ZodIssueBase, 'path' | 'message'>[]) {
+interface UnionError {
+  issues?: {path?: (string | number)[]; message: string}[]
+  name: string
+}
+
+interface ExtendedZodIssue {
+  path?: (string | number)[]
+  message?: string
+  code?: string
+  unionErrors?: UnionError[]
+}
+
+/**
+ * Finds the best matching variant from a union error by scoring each variant
+ * based on how close it is to the user's likely intent.
+ */
+function findBestMatchingVariant(unionErrors: UnionError[]): UnionError | null {
+  if (!unionErrors || unionErrors.length === 0) return null
+
+  let bestVariant: UnionError | null = null
+  let bestScore = -1
+
+  for (const variant of unionErrors) {
+    if (!variant.issues) continue
+
+    // Score based on error types - prioritize variants with fewer missing fields
+    let score = 0
+    let missingFieldCount = 0
+    let typeErrorCount = 0
+    let otherErrorCount = 0
+
+    for (const issue of variant.issues) {
+      if (issue.message?.includes('Required') || issue.message?.includes('required')) {
+        missingFieldCount++
+      } else if (issue.message?.includes('Expected') && issue.message?.includes('received')) {
+        typeErrorCount++
+      } else {
+        otherErrorCount++
+      }
+    }
+
+    // Scoring logic: prefer variants with fewer missing fields
+    // Missing fields are more likely to indicate the intended variant
+    // than type mismatches (which could indicate wrong variant entirely)
+    if (missingFieldCount > 0) {
+      // If there are missing fields, this could be the intended variant
+      score = 1000 - missingFieldCount * 10 - typeErrorCount - otherErrorCount
+    } else if (typeErrorCount > 0) {
+      // If only type errors, less likely to be the intended variant
+      score = 100 - typeErrorCount * 5 - otherErrorCount
+    } else {
+      // Other errors
+      score = 50 - otherErrorCount
+    }
+
+    if (score > bestScore) {
+      bestScore = score
+      bestVariant = variant
+    }
+  }
+
+  return bestVariant
+}
+
+export function parseHumanReadableError(issues: ExtendedZodIssue[]) {
   let humanReadableError = ''
+
   issues.forEach((issue) => {
-    const path = issue.path ? issue?.path.join('.') : 'n/a'
-    humanReadableError += `• [${path}]: ${issue.message}\n`
+    // Handle union errors with smart variant detection
+    if (issue.code === 'invalid_union' && issue.unionErrors) {
+      // Find the variant that's most likely the intended one
+      const bestVariant = findBestMatchingVariant(issue.unionErrors)
+
+      if (bestVariant && bestVariant.issues && bestVariant.issues.length > 0) {
+        // Show errors only for the best matching variant
+        bestVariant.issues.forEach((nestedIssue) => {
+          const path = nestedIssue.path && nestedIssue.path.length > 0 ? nestedIssue.path.map(String).join('.') : 'root'
+          humanReadableError += `• [${path}]: ${nestedIssue.message}\n`
+        })
+      } else {
+        // Fallback to showing all variants if we can't determine the best one
+        humanReadableError += `• Union validation failed. None of the possible variants matched:\n`
+        issue.unionErrors.forEach((unionError, index: number) => {
+          humanReadableError += `  Variant ${index + 1}:\n`
+          if (unionError.issues) {
+            unionError.issues.forEach((nestedIssue) => {
+              const path =
+                nestedIssue.path && nestedIssue.path.length > 0 ? nestedIssue.path.map(String).join('.') : 'root'
+              humanReadableError += `    • [${path}]: ${nestedIssue.message}\n`
+            })
+          }
+        })
+      }
+    } else {
+      // Handle regular issues
+      const path = issue.path && issue.path.length > 0 ? issue.path.map(String).join('.') : 'root'
+      const message = issue.message ?? 'Unknown error'
+      humanReadableError += `• [${path}]: ${message}\n`
+    }
   })
+
   return humanReadableError
 }
 
