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
39 changes: 39 additions & 0 deletions pkg/parser/content_extractor.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,45 @@ func extractToolsFromContent(content string) (string, error) {
return strings.TrimSpace(string(extractedJSON)), nil
}

// extractToolsFromFrontmatter extracts tools and mcp-servers sections from an already-parsed
// frontmatter map as a JSON string. This avoids re-parsing YAML when the frontmatter has
// already been parsed, which is the common case for imported files (both builtin and local).
func extractToolsFromFrontmatter(frontmatter map[string]any) (string, error) {
// Create a map to hold the merged result
extracted := make(map[string]any)

// Helper function to merge a field into extracted map.
// Non-map values (e.g. arrays in custom-agent format) are skipped here;
// schema validation elsewhere will report them as invalid if appropriate.
mergeField := func(fieldName string) {
if fieldValue, exists := frontmatter[fieldName]; exists {
if fieldMap, ok := fieldValue.(map[string]any); ok {
maps.Copy(extracted, fieldMap)
}
}
}

// Extract and merge tools section (tools are stored as tool_name: tool_config)
mergeField("tools")

// Extract and merge mcp-servers section (mcp-servers are stored as server_name: server_config)
mergeField("mcp-servers")

// If nothing was extracted, return empty object
if len(extracted) == 0 {
return "{}", nil
}

// Convert to JSON string. On marshal failure, return an empty object to
// match the behaviour of extractToolsFromContent (defensive, not a hard error).
extractedJSON, err := json.Marshal(extracted)
if err != nil {
return "{}", nil
}
Comment on lines +88 to +91
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

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

extractToolsFromFrontmatter changes the error-handling contract vs extractToolsFromContent: on json.Marshal failure it now returns a non-nil error, which will bubble up via processIncludedFileWithVisited(..., extractTools=true) and can fail compilation where the previous path returned "{}" and nil error (matching the comment in extractToolsFromContent). Consider returning "{}" (and nil error) on marshal failure for behavioral consistency, or ensure callers deliberately handle this new error path. Also, the error text mentions only "tools" even though the output includes both tools and mcp-servers.

Copilot uses AI. Check for mistakes.

return strings.TrimSpace(string(extractedJSON)), nil
}

// extractFrontmatterField extracts a specific field from frontmatter as JSON string
func extractFrontmatterField(content, fieldName, emptyValue string) (string, error) {
contentExtractorLog.Printf("Extracting field: %s", fieldName)
Expand Down
10 changes: 6 additions & 4 deletions pkg/parser/include_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,9 +151,10 @@ func processIncludedFileWithVisited(filePath, sectionName string, extractTools b
// Custom agent files use GitHub Copilot's format where 'tools' is an array, not an object
isAgentFile := isCustomAgentFile(filePath)

// Always try strict validation first (but skip for agent files which have a different schema)
// Always try strict validation first (but skip for agent files which have a different schema,
// and skip for builtin virtual files which are immutable trusted assets validated at development time).
var validationErr error
if !isAgentFile {
if !isAgentFile && !strings.HasPrefix(filePath, BuiltinPathPrefix) {
validationErr = ValidateIncludedFileFrontmatterWithSchemaAndLocation(result.Frontmatter, filePath)
}

Expand Down Expand Up @@ -251,8 +252,9 @@ func processIncludedFileWithVisited(filePath, sectionName string, extractTools b

// Extract tools from frontmatter, using filtered frontmatter for non-workflow files with validation errors
if validationErr == nil || isWorkflowFile {
// If validation passed or it's a workflow file (which must have valid frontmatter), use original extraction
return extractToolsFromContent(string(content))
// If validation passed or it's a workflow file (which must have valid frontmatter),
// use the already-parsed frontmatter to avoid a redundant YAML re-parse.
return extractToolsFromFrontmatter(result.Frontmatter)
} else {
// For non-workflow files with validation errors, only extract tools section
if tools, hasTools := result.Frontmatter["tools"]; hasTools {
Expand Down
5 changes: 3 additions & 2 deletions pkg/workflow/compiler_orchestrator_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,10 +194,11 @@ func (c *Compiler) setupEngineAndImports(result *parser.FrontmatterResult, clean
}

// Validate permissions from imports against top-level permissions
// Extract top-level permissions first
topLevelPermissions := c.extractPermissions(result.Frontmatter)
// Only extract and validate when imports actually contributed permissions — avoids
// the YAML marshaling cost of extractPermissions in the common case of no imports.
if importsResult.MergedPermissions != "" {
orchestratorEngineLog.Printf("Validating included permissions")
topLevelPermissions := c.extractPermissions(result.Frontmatter)
if err := c.ValidateIncludedPermissions(topLevelPermissions, importsResult.MergedPermissions); err != nil {
orchestratorEngineLog.Printf("Included permissions validation failed: %v", err)
return nil, fmt.Errorf("permission validation failed: %w", err)
Expand Down
Loading