diff --git a/pkg/parser/schema_suggestions.go b/pkg/parser/schema_suggestions.go index 948c990c715..a35e1cb4928 100644 --- a/pkg/parser/schema_suggestions.go +++ b/pkg/parser/schema_suggestions.go @@ -46,9 +46,13 @@ func generateSchemaBasedSuggestions(schemaJSON, errorMessage, jsonPath, frontmat if strings.Contains(strings.ToLower(errorMessage), "value must be one of") { schemaSuggestionsLog.Print("Detected enum constraint violation") enumValues := extractEnumValuesFromError(errorMessage) - userValue := extractYAMLValueAtPath(frontmatterContent, jsonPath) + // For oneOf errors, the path points to the container (e.g., "/permissions") but + // the enum constraint is on a nested field (e.g., "/permissions/contents"). + // Try to extract the actual sub-path from the message. + actualPath := extractEnumConstraintPath(errorMessage, jsonPath) + userValue := extractYAMLValueAtPath(frontmatterContent, actualPath) if userValue != "" && len(enumValues) > 0 { - closest := FindClosestMatches(userValue, enumValues, maxClosestMatches) + closest := sliceutil.Deduplicate(FindClosestMatches(userValue, enumValues, maxClosestMatches)) if len(closest) == 1 { return fmt.Sprintf("Did you mean '%s'?", closest[0]) } else if len(closest) > 1 { @@ -468,38 +472,129 @@ func extractEnumValuesFromError(errorMessage string) []string { return values } -// extractYAMLValueAtPath extracts the scalar value at a simple top-level JSON path -// (e.g., "/engine") from raw YAML frontmatter content. -// Only top-level paths are supported; nested paths return an empty string. +// extractYAMLValueAtPath extracts the scalar value at a JSON path from raw YAML frontmatter. +// Supports top-level paths ("/field") and two-level nested paths ("/parent/child"). +// Deeper paths return an empty string. func extractYAMLValueAtPath(yamlContent, jsonPath string) string { if yamlContent == "" || jsonPath == "" { return "" } - // Only handle simple top-level paths like "/engine" (one slash, one segment) - if strings.Count(jsonPath, "/") != 1 { + segments := strings.SplitN(strings.TrimPrefix(jsonPath, "/"), "/", 3) + switch len(segments) { + case 1: + return extractTopLevelYAMLValue(yamlContent, segments[0]) + case 2: + return extractNestedYAMLValue(yamlContent, segments[0], segments[1]) + default: return "" } - fieldName := strings.TrimPrefix(jsonPath, "/") +} + +// extractTopLevelYAMLValue extracts the scalar value of a top-level key from raw YAML. +// Uses horizontal-only whitespace between the colon and value to avoid matching multi-line blocks. +// Only keys at column 0 (no indentation) are matched, preventing false matches against +// nested keys with the same name. +func extractTopLevelYAMLValue(yamlContent, fieldName string) string { escapedField := regexp.QuoteMeta(fieldName) - // Try single-quoted value: field: 'value' - reSingle := regexp.MustCompile(`(?m)^\s*` + escapedField + `\s*:\s*'([^'\n]+)'`) + // Try single-quoted value: field: 'value' (anchored to column 0, no leading whitespace) + reSingle := regexp.MustCompile(`(?m)^` + escapedField + `[ \t]*:[ \t]*'([^'\n]+)'`) if match := reSingle.FindStringSubmatch(yamlContent); len(match) >= 2 { return strings.TrimSpace(match[1]) } // Try double-quoted value: field: "value" - reDouble := regexp.MustCompile(`(?m)^\s*` + escapedField + `\s*:\s*"([^"\n]+)"`) + reDouble := regexp.MustCompile(`(?m)^` + escapedField + `[ \t]*:[ \t]*"([^"\n]+)"`) if match := reDouble.FindStringSubmatch(yamlContent); len(match) >= 2 { return strings.TrimSpace(match[1]) } // Try unquoted value: field: value - reUnquoted := regexp.MustCompile(`(?m)^\s*` + escapedField + `\s*:\s*([^'"\n#][^\n#]*?)(?:\s*#.*)?$`) + reUnquoted := regexp.MustCompile(`(?m)^` + escapedField + `[ \t]*:[ \t]*([^'"\n#][^\n#]*?)(?:[ \t]*#.*)?$`) if match := reUnquoted.FindStringSubmatch(yamlContent); len(match) >= 2 { return strings.TrimSpace(match[1]) } return "" } +// extractNestedYAMLValue extracts the scalar value of a direct child key under a parent key in raw YAML. +// It finds the parent key's block (by indentation), determines the direct-child indent level from +// the first non-blank line inside the block, and only matches keys at that exact indent level. +// This prevents false matches against grandchildren that share the same key name. +func extractNestedYAMLValue(yamlContent, parentKey, childKey string) string { + lines := strings.Split(yamlContent, "\n") + + escapedParent := regexp.QuoteMeta(parentKey) + parentPattern := regexp.MustCompile(`^(\s*)` + escapedParent + `[ \t]*:`) + escapedChild := regexp.QuoteMeta(childKey) + + parentIndent := -1 + childIndent := -1 // indent of direct children (set on first non-blank line inside the block) + inParentBlock := false + + for _, line := range lines { + if !inParentBlock { + if match := parentPattern.FindStringSubmatch(line); match != nil { + parentIndent = len(match[1]) + inParentBlock = true + } + continue + } + + // Inside parent block: skip blank lines + if strings.TrimSpace(line) == "" { + continue + } + lineIndent := len(line) - len(strings.TrimLeft(line, " \t")) + + // Left parent block if indentation returned to parent level or less + if lineIndent <= parentIndent { + break + } + + // Establish the direct-child indentation from the first non-blank child line + if childIndent == -1 { + childIndent = lineIndent + } + + // Only match keys at the direct-child indent level (not grandchildren deeper) + if lineIndent != childIndent { + continue + } + + // Try to match child key with its value (single-quoted, double-quoted, unquoted). + childPrefix := `^\s+` + escapedChild + `[ \t]*:[ \t]*` + reSingle := regexp.MustCompile(childPrefix + `'([^'\n]+)'`) + if match := reSingle.FindStringSubmatch(line); len(match) >= 2 { + return strings.TrimSpace(match[1]) + } + reDouble := regexp.MustCompile(childPrefix + `"([^"\n]+)"`) + if match := reDouble.FindStringSubmatch(line); len(match) >= 2 { + return strings.TrimSpace(match[1]) + } + reUnquoted := regexp.MustCompile(childPrefix + `([^'"\n#][^\n#]*?)(?:[ \t]*#.*)?$`) + if match := reUnquoted.FindStringSubmatch(line); len(match) >= 2 { + return strings.TrimSpace(match[1]) + } + } + + return "" +} + +// extractEnumConstraintPath finds the JSON path of an enum constraint violation in an error message. +// For simple errors like "value must be one of 'a', 'b'", it returns the provided fallbackPath. +// For oneOf errors that contain a nested sub-path such as: +// +// "- at '/permissions/contents': value must be one of 'read', 'write', 'none'" +// +// it extracts "/permissions/contents" as the actual constraint path. +var enumConstraintPathPattern = regexp.MustCompile(`at '(/[^']+)':\s*value must be one of`) + +func extractEnumConstraintPath(errorMessage, fallbackPath string) string { + if match := enumConstraintPathPattern.FindStringSubmatch(errorMessage); len(match) >= 2 { + return match[1] + } + return fallbackPath +} + // collectSchemaPropertyPaths recursively collects all (fieldName, parentPath) pairs from a JSON schema document. // It traverses properties, oneOf/anyOf/allOf, and items to build a complete picture of valid fields across the schema. func collectSchemaPropertyPaths(schemaDoc any, currentPath string, depth int) []schemaFieldLocation { diff --git a/pkg/parser/schema_suggestions_test.go b/pkg/parser/schema_suggestions_test.go index e8e4dee82c3..8a140adbfc1 100644 --- a/pkg/parser/schema_suggestions_test.go +++ b/pkg/parser/schema_suggestions_test.go @@ -89,6 +89,15 @@ func TestGenerateSchemaBasedSuggestions(t *testing.T) { frontmatterContent: "engine: xyz123\n", wantEmpty: true, }, + { + // Full end-to-end: path is the oneOf container, message contains nested path, + // frontmatter has a permission level typo. + name: "nested oneOf enum violation extracts sub-path and suggests Did you mean", + errorMessage: "'oneOf' failed, none matched\n - at '/permissions': got object, want string\n - at '/permissions/contents': value must be one of 'read', 'write', 'none'", + jsonPath: "/permissions", + frontmatterContent: "permissions:\n contents: raed\n", + wantContains: []string{"Did you mean", "read"}, + }, } for _, tt := range tests { @@ -409,11 +418,41 @@ func TestExtractYAMLValueAtPath(t *testing.T) { wantValue: "copilot", }, { - name: "nested path returns empty", + name: "nested path - child not in yaml returns empty", yaml: "engine: copilot\n", path: "/permissions/issues", wantValue: "", }, + { + name: "nested path - extracts value under parent key", + yaml: "permissions:\n contents: raed\n issues: write\n", + path: "/permissions/contents", + wantValue: "raed", + }, + { + name: "nested path - second child key", + yaml: "permissions:\n contents: read\n issues: neno\n", + path: "/permissions/issues", + wantValue: "neno", + }, + { + name: "nested path - single-quoted value", + yaml: "permissions:\n contents: 'raed'\n", + path: "/permissions/contents", + wantValue: "raed", + }, + { + name: "nested path - double-quoted value", + yaml: "permissions:\n contents: \"raed\"\n", + path: "/permissions/contents", + wantValue: "raed", + }, + { + name: "three-level path returns empty", + yaml: "a:\n b:\n c: value\n", + path: "/a/b/c", + wantValue: "", + }, { name: "empty yaml returns empty", yaml: "", @@ -426,6 +465,27 @@ func TestExtractYAMLValueAtPath(t *testing.T) { path: "/timeout-minutes", wantValue: "", }, + { + name: "top-level key with only nested block returns empty - no inline scalar value", + yaml: "permissions:\n contents: raed\n", + path: "/permissions", + wantValue: "", + }, + { + // Ensures column-0 anchoring: a nested key with the same name must not + // satisfy a top-level path request. + name: "indented key with same name does not match top-level path", + yaml: "parent:\n engine: nested-value\nengine: top-value\n", + path: "/engine", + wantValue: "top-value", + }, + { + // Grandchild key must not be returned for a direct-child path. + name: "nested path - grandchild key not returned for child path", + yaml: "permissions:\n nested:\n contents: grandchild\n contents: direct\n", + path: "/permissions/contents", + wantValue: "direct", + }, } for _, tt := range tests { @@ -438,7 +498,51 @@ func TestExtractYAMLValueAtPath(t *testing.T) { } } -// TestGenerateExampleFromSchemaWithExamples tests that schema examples array is preferred over generic fallback +// TestExtractEnumConstraintPath tests that the correct JSON path is extracted from +// enum constraint messages, including nested paths embedded in oneOf error messages. +func TestExtractEnumConstraintPath(t *testing.T) { + tests := []struct { + name string + errorMessage string + fallbackPath string + wantPath string + }{ + { + name: "simple enum error uses fallback path", + errorMessage: "value must be one of 'claude', 'copilot'", + fallbackPath: "/engine", + wantPath: "/engine", + }, + { + name: "nested enum constraint extracted from oneOf message", + errorMessage: "'oneOf' failed, none matched\n - at '/permissions': got object, want string\n - at '/permissions/contents': value must be one of 'read', 'write', 'none'", + fallbackPath: "/permissions", + wantPath: "/permissions/contents", + }, + { + name: "nested path with issues scope", + errorMessage: " - at '/permissions/issues': value must be one of 'read', 'write', 'none'", + fallbackPath: "/permissions", + wantPath: "/permissions/issues", + }, + { + name: "no enum path pattern uses fallback", + errorMessage: "got object, want string", + fallbackPath: "/engine", + wantPath: "/engine", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := extractEnumConstraintPath(tt.errorMessage, tt.fallbackPath) + if result != tt.wantPath { + t.Errorf("extractEnumConstraintPath() = %q, want %q", result, tt.wantPath) + } + }) + } +} + func TestGenerateExampleFromSchemaWithExamples(t *testing.T) { tests := []struct { name string