Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion pkg/parser/import_field_extractor.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,17 @@ func (acc *importAccumulator) extractAllImportFields(content []byte, item import
// All subsequent field extractions use the pre-parsed result.
// When inputs are present we parse the already-substituted content so that all
// frontmatter fields (runtimes, mcp-servers, etc.) reflect the resolved values.
parsed, err := ExtractFrontmatterFromContent(rawContent)
// For builtin files without inputs, use the process-level cache to avoid redundant
// YAML re-parsing (processIncludedFileWithVisited already populated this cache).
// Builtin files WITH inputs must skip the cache because input substitution modifies
// the content, so the cached (unsubstituted) result would be stale.
var parsed *FrontmatterResult
var err error
if strings.HasPrefix(item.fullPath, BuiltinPathPrefix) && len(item.inputs) == 0 {
parsed, err = ExtractFrontmatterFromBuiltinFile(item.fullPath, content)
} else {
parsed, err = ExtractFrontmatterFromContent(rawContent)
}
var fm map[string]any
if err == nil {
fm = parsed.Frontmatter
Expand Down
83 changes: 83 additions & 0 deletions pkg/parser/import_field_extractor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

// TestComputeImportRelPath verifies that computeImportRelPath produces the correct
Expand Down Expand Up @@ -200,3 +201,85 @@ imports:
assert.Contains(t, importsResult.MergedJobs, "apm", "MergedJobs should contain the 'apm' job")
assert.Contains(t, importsResult.MergedJobs, "ubuntu-slim", "MergedJobs should contain the job runner")
}

// TestExtractAllImportFields_BuiltinCacheHit verifies that extractAllImportFields uses the
// process-level builtin frontmatter cache for builtin files without inputs.
func TestExtractAllImportFields_BuiltinCacheHit(t *testing.T) {
builtinPath := BuiltinPathPrefix + "test/cache-hit.md"
content := []byte(`---
tools:
bash: ["echo"]
engine: claude
---

# Cache Hit Test
`)

// Register the builtin virtual file
RegisterBuiltinVirtualFile(builtinPath, content)

// Warm the cache by parsing once
cachedResult, err := ExtractFrontmatterFromBuiltinFile(builtinPath, content)
require.NoError(t, err, "should parse builtin file without error")
assert.NotNil(t, cachedResult, "cached result should not be nil")

// Verify the cache is populated
cached, ok := GetBuiltinFrontmatterCache(builtinPath)
assert.True(t, ok, "builtin cache should have an entry for the path")
assert.Equal(t, cachedResult, cached, "cached result should match")

// Call extractAllImportFields with no inputs — should hit the cache
acc := newImportAccumulator()
item := importQueueItem{
fullPath: builtinPath,
importPath: "test/cache-hit.md",
sectionName: "",
inputs: nil,
}
visited := map[string]bool{builtinPath: true}

err = acc.extractAllImportFields(content, item, visited)
require.NoError(t, err, "extractAllImportFields should succeed for builtin file without inputs")

// Verify engine was extracted from the cached frontmatter
assert.NotEmpty(t, acc.engines, "engines should be populated from cached builtin file")
assert.Contains(t, acc.engines[0], "claude", "engine should be 'claude' from the builtin file")
}

// TestExtractAllImportFields_BuiltinWithInputsBypassesCache verifies that builtin files
// with inputs bypass the cache and use the substituted content.
func TestExtractAllImportFields_BuiltinWithInputsBypassesCache(t *testing.T) {
builtinPath := BuiltinPathPrefix + "test/cache-bypass.md"
content := []byte(`---
tools:
bash: ["echo"]
engine: copilot
---

# Cache Bypass Test
`)

// Register the builtin virtual file
RegisterBuiltinVirtualFile(builtinPath, content)

// Warm the cache
_, err := ExtractFrontmatterFromBuiltinFile(builtinPath, content)
require.NoError(t, err, "should parse builtin file without error")

// Call extractAllImportFields WITH inputs — should bypass the cache
acc := newImportAccumulator()
item := importQueueItem{
fullPath: builtinPath,
importPath: "test/cache-bypass.md",
sectionName: "",
inputs: map[string]any{"key": "value"},
}
visited := map[string]bool{builtinPath: true}

err = acc.extractAllImportFields(content, item, visited)
require.NoError(t, err, "extractAllImportFields should succeed for builtin file with inputs")

// Verify engine was still extracted (from direct parse, not cache)
assert.NotEmpty(t, acc.engines, "engines should be populated even when bypassing cache")
assert.Contains(t, acc.engines[0], "copilot", "engine should be 'copilot' from the builtin file")
}
92 changes: 76 additions & 16 deletions pkg/parser/schema_compiler.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"fmt"
"os"
"path/filepath"
"reflect"
"regexp"
"sort"
"strings"
Expand Down Expand Up @@ -189,6 +190,73 @@ func GetSafeOutputTypeKeys() ([]string, error) {
return keys, nil
}

// normalizeForJSONSchema recursively returns a normalized copy of v with YAML-native
// Go types converted to JSON-compatible types for JSON schema validation. It does
// not mutate the caller's maps or slices. goccy/go-yaml produces uint64 for
// positive integers and int64 for negative integers, but JSON schema validators
// expect float64 for all numbers (matching encoding/json's unmarshaling behavior).
// goccy/go-yaml may also produce typed slices (e.g. []string) instead of []any;
// the reflection fallback converts these so the schema validator sees []any.
// This avoids the overhead of a json.Marshal + json.Unmarshal roundtrip.
func normalizeForJSONSchema(v any) any {
switch val := v.(type) {
case map[string]any:
normalized := make(map[string]any, len(val))
for k, elem := range val {
normalized[k] = normalizeForJSONSchema(elem)
}
return normalized
case []any:
normalized := make([]any, len(val))
for i, elem := range val {
normalized[i] = normalizeForJSONSchema(elem)
}
return normalized
case int:
return float64(val)
case int8:
return float64(val)
case int16:
return float64(val)
case int32:
return float64(val)
case int64:
return float64(val)
case uint:
return float64(val)
case uint8:
return float64(val)
case uint16:
return float64(val)
case uint32:
return float64(val)
case uint64:
return float64(val)
case float32:
return float64(val)
default:
// Use reflection to handle typed slices (e.g. []string) and typed maps
// that goccy/go-yaml may produce instead of []any / map[string]any.
rv := reflect.ValueOf(v)
switch rv.Kind() {
case reflect.Slice:
normalized := make([]any, rv.Len())
for i := range rv.Len() {
normalized[i] = normalizeForJSONSchema(rv.Index(i).Interface())
}
return normalized
case reflect.Map:
normalized := make(map[string]any, rv.Len())
for _, key := range rv.MapKeys() {
normalized[key.String()] = normalizeForJSONSchema(rv.MapIndex(key).Interface())
}
return normalized
}
// string, bool, float64, nil pass through unchanged
return v
}
}

func validateWithSchema(frontmatter map[string]any, schemaJSON, context string) error {
schemaCompilerLog.Printf("Validating frontmatter against schema for context: %s (%d fields)", context, len(frontmatter))

Expand Down Expand Up @@ -217,27 +285,19 @@ func validateWithSchema(frontmatter map[string]any, schemaJSON, context string)
return fmt.Errorf("schema validation error for %s: %w", context, err)
}

// Convert frontmatter to JSON and back to normalize types for validation
// Handle nil frontmatter as empty object to satisfy schema validation
var frontmatterToValidate map[string]any
// Normalize YAML-native Go types to JSON-compatible types for schema validation.
// goccy/go-yaml produces uint64/int64 for integers, but JSON schema validators
// expect float64 for all numbers (matching encoding/json's behavior).
// This avoids the overhead of a json.Marshal/Unmarshal roundtrip.
var normalized any
if frontmatter == nil {
frontmatterToValidate = make(map[string]any)
normalized = make(map[string]any)
} else {
frontmatterToValidate = frontmatter
}

frontmatterJSON, err := json.Marshal(frontmatterToValidate)
if err != nil {
return fmt.Errorf("schema validation error for %s: failed to marshal frontmatter: %w", context, err)
}

var normalizedFrontmatter any
if err := json.Unmarshal(frontmatterJSON, &normalizedFrontmatter); err != nil {
return fmt.Errorf("schema validation error for %s: failed to unmarshal frontmatter: %w", context, err)
normalized = normalizeForJSONSchema(frontmatter)
}

// Validate the normalized frontmatter
if err := schema.Validate(normalizedFrontmatter); err != nil {
if err := schema.Validate(normalized); err != nil {
schemaCompilerLog.Printf("Schema validation failed for %s: %v", context, err)
Comment on lines 260 to 301
Copy link

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new normalization path is performance-sensitive and changes validation behavior for numeric types; there are existing unit tests for validateWithSchema, but none asserting that YAML-style ints (e.g., int64/uint64) are accepted against number/integer schema types. Please add coverage that exercises normalizeForJSONSchema via validateWithSchema (including nested maps/slices) to prevent regressions.

Copilot uses AI. Check for mistakes.
return err
}
Expand Down
Loading
Loading