From f1022bdeda5f578adf5e6ff750fa88c70f6eb9c5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Oct 2025 09:08:10 +0000 Subject: [PATCH 1/4] Initial plan From 013ff08da341e33f9cc066c16b68ff8aeb75551c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Oct 2025 09:22:53 +0000 Subject: [PATCH 2/4] Add imports field to frontmatter schema and implement processing Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/add_command.go | 10 ++ pkg/cli/imports.go | 94 +++++++++++++++- pkg/parser/frontmatter.go | 80 +++++++++++++ pkg/parser/frontmatter_test.go | 112 +++++++++++++++++++ pkg/parser/schemas/main_workflow_schema.json | 8 ++ pkg/workflow/compiler.go | 23 +++- 6 files changed, 324 insertions(+), 3 deletions(-) diff --git a/pkg/cli/add_command.go b/pkg/cli/add_command.go index be40615acbf..0eb74a49ee6 100644 --- a/pkg/cli/add_command.go +++ b/pkg/cli/add_command.go @@ -455,6 +455,16 @@ func addWorkflowWithTracking(workflow *WorkflowSpec, number int, verbose bool, e content = updatedContent } + // Process imports field and replace with workflowspec + processedImportsContent, err := processImportsWithWorkflowSpec(content, workflow, sourceInfo.CommitSHA, verbose) + if err != nil { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to process imports: %v", err))) + } + } else { + content = processedImportsContent + } + // Process @include directives and replace with workflowspec processedContent, err := processIncludesWithWorkflowSpec(content, workflow, sourceInfo.CommitSHA, sourceInfo.PackagePath, verbose) if err != nil { diff --git a/pkg/cli/imports.go b/pkg/cli/imports.go index f83f20e4ca0..0eacf1d5fe8 100644 --- a/pkg/cli/imports.go +++ b/pkg/cli/imports.go @@ -9,8 +9,88 @@ import ( "github.com/githubnext/gh-aw/pkg/console" "github.com/githubnext/gh-aw/pkg/parser" + "github.com/goccy/go-yaml" ) +// processImportsWithWorkflowSpec processes imports field in frontmatter and replaces local file references +// with workflowspec format (owner/repo/path@sha) for all imports found +func processImportsWithWorkflowSpec(content string, workflow *WorkflowSpec, commitSHA string, verbose bool) (string, error) { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatVerboseMessage("Processing imports field to replace with workflowspec")) + } + + // Extract frontmatter from content + result, err := parser.ExtractFrontmatterFromContent(content) + if err != nil { + return content, nil // Return original content if no frontmatter + } + + // Check if imports field exists + importsField, exists := result.Frontmatter["imports"] + if !exists { + return content, nil // No imports field, return original content + } + + // Convert imports to array of strings + var imports []string + switch v := importsField.(type) { + case []any: + for _, item := range v { + if str, ok := item.(string); ok { + imports = append(imports, str) + } + } + case []string: + imports = v + default: + return content, nil // Invalid imports field, skip processing + } + + // Process each import and replace with workflowspec format + processedImports := make([]string, 0, len(imports)) + for _, importPath := range imports { + // Skip if already a workflowspec + if isWorkflowSpecFormat(importPath) { + processedImports = append(processedImports, importPath) + continue + } + + // Build workflowspec for this import + // Format: owner/repo/path@sha + workflowSpec := workflow.Repo + "/" + importPath + if commitSHA != "" { + workflowSpec += "@" + commitSHA + } else if workflow.Version != "" { + workflowSpec += "@" + workflow.Version + } + + processedImports = append(processedImports, workflowSpec) + } + + // Update frontmatter with processed imports + result.Frontmatter["imports"] = processedImports + + // Convert frontmatter back to YAML + frontmatterYAML, err := yaml.Marshal(result.Frontmatter) + if err != nil { + return content, fmt.Errorf("failed to marshal frontmatter: %w", err) + } + + // Reconstruct the workflow file + var lines []string + lines = append(lines, "---") + frontmatterStr := strings.TrimSuffix(string(frontmatterYAML), "\n") + if frontmatterStr != "" { + lines = append(lines, strings.Split(frontmatterStr, "\n")...) + } + lines = append(lines, "---") + if result.Markdown != "" { + lines = append(lines, result.Markdown) + } + + return strings.Join(lines, "\n"), nil +} + // processIncludesWithWorkflowSpec processes @include directives in content and replaces local file references // with workflowspec format (owner/repo/path@sha) for all includes found in the package func processIncludesWithWorkflowSpec(content string, workflow *WorkflowSpec, commitSHA, packagePath string, verbose bool) (string, error) { @@ -157,8 +237,20 @@ func processIncludesWithWorkflowSpec(content string, workflow *WorkflowSpec, com } // processIncludesInContent processes @include directives in workflow content for update command +// and also processes imports field in frontmatter func processIncludesInContent(content string, workflow *WorkflowSpec, commitSHA string, verbose bool) (string, error) { - scanner := bufio.NewScanner(strings.NewReader(content)) + // First process imports field in frontmatter + processedImportsContent, err := processImportsWithWorkflowSpec(content, workflow, commitSHA, verbose) + if err != nil { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to process imports: %v", err))) + } + // Continue with original content on error + processedImportsContent = content + } + + // Then process @include directives in markdown + scanner := bufio.NewScanner(strings.NewReader(processedImportsContent)) var result strings.Builder for scanner.Scan() { diff --git a/pkg/parser/frontmatter.go b/pkg/parser/frontmatter.go index c5e4ecb9906..4998b042572 100644 --- a/pkg/parser/frontmatter.go +++ b/pkg/parser/frontmatter.go @@ -288,6 +288,86 @@ func ExtractMarkdown(filePath string) (string, error) { return ExtractMarkdownContent(string(content)) } +// ProcessImportsFromFrontmatter processes imports field from frontmatter +// Returns merged tools and engines from imported files +func ProcessImportsFromFrontmatter(frontmatter map[string]any, baseDir string) (mergedTools string, mergedEngines []string, err error) { + // Check if imports field exists + importsField, exists := frontmatter["imports"] + if !exists { + return "", nil, nil + } + + // Convert to array of strings + var imports []string + switch v := importsField.(type) { + case []any: + for _, item := range v { + if str, ok := item.(string); ok { + imports = append(imports, str) + } + } + case []string: + imports = v + default: + return "", nil, fmt.Errorf("imports field must be an array of strings") + } + + if len(imports) == 0 { + return "", nil, nil + } + + // Track visited to prevent cycles + visited := make(map[string]bool) + + // Process each import + var toolsBuilder strings.Builder + var engines []string + + for _, importPath := range imports { + // Handle section references (file.md#Section) + var filePath, sectionName string + if strings.Contains(importPath, "#") { + parts := strings.SplitN(importPath, "#", 2) + filePath = parts[0] + sectionName = parts[1] + } else { + filePath = importPath + } + + // Resolve import path (supports workflowspec format) + fullPath, err := resolveIncludePath(filePath, baseDir) + if err != nil { + return "", nil, fmt.Errorf("failed to resolve import '%s': %w", filePath, err) + } + + // Check for cycles + if visited[fullPath] { + continue + } + visited[fullPath] = true + + // Extract tools from imported file + toolsContent, err := processIncludedFileWithVisited(fullPath, sectionName, true, baseDir, visited) + if err != nil { + return "", nil, fmt.Errorf("failed to process imported file '%s': %w", fullPath, err) + } + toolsBuilder.WriteString(toolsContent + "\n") + + // Extract engines from imported file + content, err := os.ReadFile(fullPath) + if err != nil { + return "", nil, fmt.Errorf("failed to read imported file '%s': %w", fullPath, err) + } + + engineContent, err := extractEngineFromContent(string(content)) + if err == nil && engineContent != "" { + engines = append(engines, engineContent) + } + } + + return toolsBuilder.String(), engines, nil +} + // ProcessIncludes processes @include and @import directives in markdown content // This matches the bash process_includes function behavior func ProcessIncludes(content, baseDir string, extractTools bool) (string, error) { diff --git a/pkg/parser/frontmatter_test.go b/pkg/parser/frontmatter_test.go index 09f5c380186..37237e97ce0 100644 --- a/pkg/parser/frontmatter_test.go +++ b/pkg/parser/frontmatter_test.go @@ -1914,3 +1914,115 @@ func TestProcessIncludesWithCycleDetection(t *testing.T) { t.Errorf("ProcessIncludes result should contain File B content") } } + +func TestProcessImportsFromFrontmatter(t *testing.T) { + // Create temp directory for test files + tempDir := t.TempDir() + + // Create a test include file + includeFile := filepath.Join(tempDir, "include.md") + includeContent := `--- +tools: + bash: + allowed: + - ls + - cat +--- +# Include Content +This is an included file.` + if err := os.WriteFile(includeFile, []byte(includeContent), 0644); err != nil { + t.Fatalf("Failed to write include file: %v", err) + } + + tests := []struct { + name string + frontmatter map[string]any + wantToolsJSON bool + wantEngines bool + wantErr bool + }{ + { + name: "no imports field", + frontmatter: map[string]any{ + "on": "push", + }, + wantToolsJSON: false, + wantEngines: false, + wantErr: false, + }, + { + name: "empty imports array", + frontmatter: map[string]any{ + "on": "push", + "imports": []string{}, + }, + wantToolsJSON: false, + wantEngines: false, + wantErr: false, + }, + { + name: "valid imports", + frontmatter: map[string]any{ + "on": "push", + "imports": []string{"include.md"}, + }, + wantToolsJSON: true, + wantEngines: false, + wantErr: false, + }, + { + name: "invalid imports type", + frontmatter: map[string]any{ + "on": "push", + "imports": "not-an-array", + }, + wantToolsJSON: false, + wantEngines: false, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tools, engines, err := ProcessImportsFromFrontmatter(tt.frontmatter, tempDir) + + if tt.wantErr { + if err == nil { + t.Errorf("ProcessImportsFromFrontmatter() expected error but got none") + } + return + } + + if err != nil { + t.Errorf("ProcessImportsFromFrontmatter() unexpected error: %v", err) + return + } + + if tt.wantToolsJSON { + if tools == "" { + t.Errorf("ProcessImportsFromFrontmatter() expected tools JSON but got empty string") + } + // Verify it's valid JSON + var toolsMap map[string]any + if err := json.Unmarshal([]byte(tools), &toolsMap); err != nil { + t.Errorf("ProcessImportsFromFrontmatter() tools not valid JSON: %v", err) + } + } else { + if tools != "" { + t.Errorf("ProcessImportsFromFrontmatter() expected no tools but got: %s", tools) + } + } + + if tt.wantEngines { + if len(engines) == 0 { + t.Errorf("ProcessImportsFromFrontmatter() expected engines but got none") + } + } else { + if len(engines) != 0 { + t.Errorf("ProcessImportsFromFrontmatter() expected no engines but got: %v", engines) + } + } + }) + } +} + diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index ab08801413f..57d857f1c17 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -14,6 +14,14 @@ "type": "string", "description": "Optional source reference indicating where this workflow was added from. Format: owner/repo/path@ref (e.g., githubnext/agentics/workflows/ci-doctor.md@v1.0.0). Rendered as a comment in the generated lock file." }, + "imports": { + "type": "array", + "description": "Optional array of workflow specifications to import (similar to @include directives but defined in frontmatter). Format: owner/repo/path@ref (e.g., githubnext/agentics/workflows/shared/common.md@v1.0.0).", + "items": { + "type": "string", + "description": "Workflow specification in format owner/repo/path@ref" + } + }, "on": { "description": "Workflow triggers that define when the agentic workflow should run. Supports standard GitHub Actions trigger events plus special command triggers for /commands (required)", "oneOf": [ diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go index 6943a4479fc..9c29428022a 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -478,14 +478,23 @@ func (c *Compiler) ParseWorkflowFile(markdownPath string) (*WorkflowData, error) engineSetting = c.engineOverride } + // Process imports from frontmatter first (before @include directives) + importedTools, importedEngines, err := parser.ProcessImportsFromFrontmatter(result.Frontmatter, markdownDir) + if err != nil { + return nil, fmt.Errorf("failed to process imports from frontmatter: %w", err) + } + // Process @include directives to extract engine configurations and check for conflicts includedEngines, err := parser.ExpandIncludesForEngines(result.Markdown, markdownDir) if err != nil { return nil, fmt.Errorf("failed to expand includes for engines: %w", err) } + // Combine imported engines with included engines + allEngines := append(importedEngines, includedEngines...) + // Validate that only one engine field exists across all files - finalEngineSetting, err := c.validateSingleEngineSpecification(engineSetting, includedEngines) + finalEngineSetting, err := c.validateSingleEngineSpecification(engineSetting, allEngines) if err != nil { return nil, err } @@ -538,8 +547,18 @@ func (c *Compiler) ParseWorkflowFile(markdownPath string) (*WorkflowData, error) return nil, fmt.Errorf("failed to expand includes for tools: %w", err) } + // Combine imported tools with included tools + var allIncludedTools string + if importedTools != "" && includedTools != "" { + allIncludedTools = importedTools + "\n" + includedTools + } else if importedTools != "" { + allIncludedTools = importedTools + } else { + allIncludedTools = includedTools + } + // Merge tools including mcp-servers - tools, err = c.mergeToolsAndMCPServers(topTools, mcpServers, includedTools) + tools, err = c.mergeToolsAndMCPServers(topTools, mcpServers, allIncludedTools) if err != nil { return nil, fmt.Errorf("failed to merge tools: %w", err) From f80ce302f97789b8553028ee8300f072931d277f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Oct 2025 09:26:25 +0000 Subject: [PATCH 3/4] Complete imports field implementation - all tests and validations pass Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/parser/frontmatter.go | 6 +++--- pkg/parser/frontmatter_test.go | 1 - 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/pkg/parser/frontmatter.go b/pkg/parser/frontmatter.go index 4998b042572..583f9362042 100644 --- a/pkg/parser/frontmatter.go +++ b/pkg/parser/frontmatter.go @@ -318,11 +318,11 @@ func ProcessImportsFromFrontmatter(frontmatter map[string]any, baseDir string) ( // Track visited to prevent cycles visited := make(map[string]bool) - + // Process each import var toolsBuilder strings.Builder var engines []string - + for _, importPath := range imports { // Handle section references (file.md#Section) var filePath, sectionName string @@ -358,7 +358,7 @@ func ProcessImportsFromFrontmatter(frontmatter map[string]any, baseDir string) ( if err != nil { return "", nil, fmt.Errorf("failed to read imported file '%s': %w", fullPath, err) } - + engineContent, err := extractEngineFromContent(string(content)) if err == nil && engineContent != "" { engines = append(engines, engineContent) diff --git a/pkg/parser/frontmatter_test.go b/pkg/parser/frontmatter_test.go index 37237e97ce0..4555a0977f6 100644 --- a/pkg/parser/frontmatter_test.go +++ b/pkg/parser/frontmatter_test.go @@ -2025,4 +2025,3 @@ This is an included file.` }) } } - From e4f20842febba085d477b66fce9b1098764cfb35 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Oct 2025 14:53:40 +0000 Subject: [PATCH 4/4] Refactor to use YAML marshal helpers and add imports to field priority list Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/imports.go | 28 ++++++++++++++++++++-------- pkg/constants/constants.go | 2 +- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/pkg/cli/imports.go b/pkg/cli/imports.go index 0eacf1d5fe8..b8f965a0e30 100644 --- a/pkg/cli/imports.go +++ b/pkg/cli/imports.go @@ -8,8 +8,9 @@ import ( "strings" "github.com/githubnext/gh-aw/pkg/console" + "github.com/githubnext/gh-aw/pkg/constants" "github.com/githubnext/gh-aw/pkg/parser" - "github.com/goccy/go-yaml" + "github.com/githubnext/gh-aw/pkg/workflow" ) // processImportsWithWorkflowSpec processes imports field in frontmatter and replaces local file references @@ -70,22 +71,33 @@ func processImportsWithWorkflowSpec(content string, workflow *WorkflowSpec, comm // Update frontmatter with processed imports result.Frontmatter["imports"] = processedImports - // Convert frontmatter back to YAML - frontmatterYAML, err := yaml.Marshal(result.Frontmatter) + // Use helper function to reconstruct workflow file with proper field ordering + return reconstructWorkflowFileFromMap(result.Frontmatter, result.Markdown) +} + +// reconstructWorkflowFileFromMap reconstructs a workflow file from frontmatter map and markdown +// using proper field ordering and YAML helpers +func reconstructWorkflowFileFromMap(frontmatter map[string]any, markdown string) (string, error) { + // Convert frontmatter to YAML with proper field ordering + // Use PriorityWorkflowFields to ensure consistent ordering of top-level fields + updatedFrontmatter, err := workflow.MarshalWithFieldOrder(frontmatter, constants.PriorityWorkflowFields) if err != nil { - return content, fmt.Errorf("failed to marshal frontmatter: %w", err) + return "", fmt.Errorf("failed to marshal frontmatter: %w", err) } - // Reconstruct the workflow file + // Clean up the YAML - remove trailing newline and unquote the "on" key + frontmatterStr := strings.TrimSuffix(string(updatedFrontmatter), "\n") + frontmatterStr = workflow.UnquoteYAMLKey(frontmatterStr, "on") + + // Reconstruct the file var lines []string lines = append(lines, "---") - frontmatterStr := strings.TrimSuffix(string(frontmatterYAML), "\n") if frontmatterStr != "" { lines = append(lines, strings.Split(frontmatterStr, "\n")...) } lines = append(lines, "---") - if result.Markdown != "" { - lines = append(lines, result.Markdown) + if markdown != "" { + lines = append(lines, markdown) } return strings.Join(lines, "\n"), nil diff --git a/pkg/constants/constants.go b/pkg/constants/constants.go index ff44cc4a910..bafc49f5a60 100644 --- a/pkg/constants/constants.go +++ b/pkg/constants/constants.go @@ -169,4 +169,4 @@ var PriorityJobFields = []string{"name", "runs-on", "needs", "if", "permissions" // PriorityWorkflowFields defines the conventional field order for top-level GitHub Actions workflow frontmatter // Fields appear in this order first, followed by remaining fields alphabetically -var PriorityWorkflowFields = []string{"on", "permissions", "if", "network", "safe-outputs", "steps"} +var PriorityWorkflowFields = []string{"on", "permissions", "if", "network", "imports", "safe-outputs", "steps"}