diff --git a/pkg/cli/compile_command.go b/pkg/cli/compile_command.go index 33527a92e7d..afdba352eea 100644 --- a/pkg/cli/compile_command.go +++ b/pkg/cli/compile_command.go @@ -789,6 +789,19 @@ func watchAndCompileWorkflows(markdownFile string, compiler *workflow.Compiler, } } + // Build dependency graph for intelligent recompilation + depGraph := NewDependencyGraph(workflowsDir) + compileLog.Print("Building dependency graph for watch mode...") + if err := depGraph.BuildGraph(compiler); err != nil { + compileLog.Printf("Warning: failed to build dependency graph: %v", err) + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to build dependency graph: %v", err))) + } else { + compileLog.Printf("Dependency graph built successfully: %d workflows", len(depGraph.nodes)) + if verbose { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Dependency graph built: %d workflows", len(depGraph.nodes)))) + } + } + // Set up file system watcher with buffered events for better handling of burst activity watcher, err := fsnotify.NewBufferedWatcher(100) if err != nil { @@ -919,6 +932,8 @@ func watchAndCompileWorkflows(markdownFile string, compiler *workflow.Compiler, case event.Has(fsnotify.Remove): // Handle file deletion handleFileDeleted(event.Name, verbose) + // Remove from dependency graph + depGraph.RemoveWorkflow(event.Name) case event.Has(fsnotify.Write) || event.Has(fsnotify.Create): // Handle file modification or creation - add to debounced compilation modifiedFiles[event.Name] = struct{}{} @@ -935,8 +950,8 @@ func watchAndCompileWorkflows(markdownFile string, compiler *workflow.Compiler, // Clear the modifiedFiles map modifiedFiles = make(map[string]struct{}) - // Compile the modified files - compileModifiedFiles(compiler, filesToCompile, verbose) + // Compile the modified files using dependency graph + compileModifiedFilesWithDependencies(compiler, depGraph, filesToCompile, verbose) }) } @@ -1118,6 +1133,90 @@ func compileModifiedFiles(compiler *workflow.Compiler, files []string, verbose b printCompilationSummary(stats) } +// compileModifiedFilesWithDependencies compiles modified files and their dependencies using the dependency graph +func compileModifiedFilesWithDependencies(compiler *workflow.Compiler, depGraph *DependencyGraph, files []string, verbose bool) { + if len(files) == 0 { + return + } + + // Clear screen before emitting new output in watch mode + console.ClearScreen() + + // Use dependency graph to determine what needs to be recompiled + var workflowsToCompile []string + uniqueWorkflows := make(map[string]bool) + + for _, modifiedFile := range files { + compileLog.Printf("Processing modified file: %s", modifiedFile) + + // Update the workflow in the dependency graph + if err := depGraph.UpdateWorkflow(modifiedFile, compiler); err != nil { + compileLog.Printf("Warning: failed to update workflow in dependency graph: %v", err) + } + + // Get affected workflows from dependency graph + affected := depGraph.GetAffectedWorkflows(modifiedFile) + compileLog.Printf("File %s affects %d workflow(s)", modifiedFile, len(affected)) + + // Add to unique set + for _, workflow := range affected { + if !uniqueWorkflows[workflow] { + uniqueWorkflows[workflow] = true + workflowsToCompile = append(workflowsToCompile, workflow) + } + } + } + + fmt.Fprintln(os.Stderr, "Watching for file changes") + if verbose { + fmt.Fprintln(os.Stderr, console.FormatProgressMessage(fmt.Sprintf("Recompiling %d workflow(s) affected by %d change(s)...", len(workflowsToCompile), len(files)))) + } + + // Reset warning count before compilation + compiler.ResetWarningCount() + + // Track compilation statistics + stats := &CompilationStats{} + + for _, file := range workflowsToCompile { + compileSingleFile(compiler, file, stats, verbose, true) + } + + // Get warning count from compiler + stats.Warnings = compiler.GetWarningCount() + + // Save the action cache after compilations + actionCache := compiler.GetSharedActionCache() + hasActionCacheEntries := actionCache != nil && len(actionCache.Entries) > 0 + successCount := stats.Total - stats.Errors + + if actionCache != nil { + if err := actionCache.Save(); err != nil { + compileLog.Printf("Failed to save action cache: %v", err) + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to save action cache: %v", err))) + } + } else { + compileLog.Print("Action cache saved successfully") + } + } + + // Ensure .gitattributes marks .lock.yml files as generated + // Only update if we successfully compiled workflows or have action cache entries + if successCount > 0 || hasActionCacheEntries { + if err := ensureGitAttributes(); err != nil { + if verbose { + fmt.Printf("⚠️ Failed to update .gitattributes: %v\n", err) + } + } + } else { + compileLog.Print("Skipping .gitattributes update (no compiled workflows and no action cache entries)") + } + + // Print summary instead of just "Recompiled" + printCompilationSummary(stats) +} + // handleFileDeleted handles the deletion of a markdown file by removing its corresponding lock file func handleFileDeleted(mdFile string, verbose bool) { // Generate the corresponding lock file path diff --git a/pkg/cli/dependency_graph.go b/pkg/cli/dependency_graph.go new file mode 100644 index 00000000000..a9062cf43c1 --- /dev/null +++ b/pkg/cli/dependency_graph.go @@ -0,0 +1,378 @@ +package cli + +import ( + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/githubnext/gh-aw/pkg/logger" + "github.com/githubnext/gh-aw/pkg/parser" + "github.com/githubnext/gh-aw/pkg/workflow" +) + +var depGraphLog = logger.New("cli:dependency_graph") + +// WorkflowNode represents a workflow file in the dependency graph +type WorkflowNode struct { + Path string // Absolute path to the workflow file + IsTopLevel bool // True if this is a top-level workflow (not in subdirectory) + Imports []string // List of imported file paths (absolute) +} + +// DependencyGraph tracks workflow dependencies for efficient recompilation +type DependencyGraph struct { + nodes map[string]*WorkflowNode // Map of absolute path -> WorkflowNode + reverseImports map[string][]string // Map of imported file -> list of files that import it + workflowsDir string // Base workflows directory + sharedDirPattern string // Pattern to identify shared workflows (e.g., "shared/") +} + +// NewDependencyGraph creates a new dependency graph +func NewDependencyGraph(workflowsDir string) *DependencyGraph { + depGraphLog.Printf("Creating dependency graph for directory: %s", workflowsDir) + return &DependencyGraph{ + nodes: make(map[string]*WorkflowNode), + reverseImports: make(map[string][]string), + workflowsDir: workflowsDir, + sharedDirPattern: "shared/", + } +} + +// isTopLevelWorkflow determines if a workflow is a top-level workflow (dominator) +// Top-level workflows are those directly in the workflows directory, not in subdirectories +func (g *DependencyGraph) isTopLevelWorkflow(absPath string) bool { + // Get relative path from workflows directory + relPath, err := filepath.Rel(g.workflowsDir, absPath) + if err != nil { + depGraphLog.Printf("Failed to get relative path for %s: %v", absPath, err) + return false + } + + // Check if the file is directly in the workflows directory (no subdirectory) + // If there's a path separator in the relative path, it's in a subdirectory + isTopLevel := !strings.Contains(relPath, string(filepath.Separator)) + depGraphLog.Printf("Checking if %s is top-level: %v (relPath: %s)", absPath, isTopLevel, relPath) + return isTopLevel +} + +// BuildGraph scans all workflow files and builds the dependency graph +func (g *DependencyGraph) BuildGraph(compiler *workflow.Compiler) error { + depGraphLog.Printf("Building dependency graph by scanning %s", g.workflowsDir) + + // Find all markdown files in the workflows directory (including subdirectories) + var allWorkflows []string + err := filepath.Walk(g.workflowsDir, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + if !info.IsDir() && strings.HasSuffix(path, ".md") && !strings.HasSuffix(path, ".lock.yml") { + allWorkflows = append(allWorkflows, path) + } + return nil + }) + if err != nil { + return fmt.Errorf("failed to scan workflows directory: %w", err) + } + + depGraphLog.Printf("Found %d workflow files to analyze", len(allWorkflows)) + + // Parse each workflow to extract its imports + for _, workflowPath := range allWorkflows { + if err := g.addWorkflow(workflowPath, compiler); err != nil { + depGraphLog.Printf("Warning: failed to add workflow %s to graph: %v", workflowPath, err) + // Continue processing other workflows even if one fails + } + } + + depGraphLog.Printf("Dependency graph built: %d nodes, %d reverse import entries", len(g.nodes), len(g.reverseImports)) + return nil +} + +// addWorkflow adds a workflow to the dependency graph by parsing its imports +func (g *DependencyGraph) addWorkflow(workflowPath string, compiler *workflow.Compiler) error { + depGraphLog.Printf("Adding workflow to graph: %s", workflowPath) + + // Check if already in graph + if _, exists := g.nodes[workflowPath]; exists { + depGraphLog.Printf("Workflow already in graph: %s", workflowPath) + return nil + } + + // Extract imports directly from the file + imports, err := g.extractImportsFromFile(workflowPath) + if err != nil { + // If extraction fails, still add the node but with no imports + depGraphLog.Printf("Failed to extract imports from %s: %v", workflowPath, err) + node := &WorkflowNode{ + Path: workflowPath, + IsTopLevel: g.isTopLevelWorkflow(workflowPath), + Imports: []string{}, + } + g.nodes[workflowPath] = node + return err + } + + // Create node + node := &WorkflowNode{ + Path: workflowPath, + IsTopLevel: g.isTopLevelWorkflow(workflowPath), + Imports: imports, + } + g.nodes[workflowPath] = node + + // Build reverse imports (for each imported file, track who imports it) + for _, importPath := range imports { + g.reverseImports[importPath] = append(g.reverseImports[importPath], workflowPath) + depGraphLog.Printf("Tracking reverse import: %s <- %s", importPath, workflowPath) + } + + depGraphLog.Printf("Added workflow to graph: %s (top-level: %v, imports: %d)", workflowPath, node.IsTopLevel, len(imports)) + return nil +} + +// extractImportsFromFile extracts imports directly from a workflow file +func (g *DependencyGraph) extractImportsFromFile(workflowPath string) ([]string, error) { + // Read the file + content, err := os.ReadFile(workflowPath) + if err != nil { + return nil, err + } + + // Parse frontmatter + result, err := parser.ExtractFrontmatterFromContent(string(content)) + if err != nil { + return nil, err + } + + return g.extractImportsFromFrontmatter(workflowPath, result.Frontmatter), nil +} + +// extractImportsFromFrontmatter extracts the list of imported file paths from frontmatter +func (g *DependencyGraph) extractImportsFromFrontmatter(workflowPath string, frontmatter map[string]any) []string { + var imports []string + + // Get frontmatter to extract imports + if frontmatter == nil { + return imports + } + + importsField, exists := frontmatter["imports"] + if !exists { + return imports + } + + // Parse imports field - can be array of strings or objects with path + workflowDir := filepath.Dir(workflowPath) + switch v := importsField.(type) { + case []any: + for _, item := range v { + switch importItem := item.(type) { + case string: + // Simple string import + if resolvedPath := g.resolveImportPath(importItem, workflowDir); resolvedPath != "" { + imports = append(imports, resolvedPath) + } + case map[string]any: + // Object import with path field + if pathValue, hasPath := importItem["path"]; hasPath { + if pathStr, ok := pathValue.(string); ok { + if resolvedPath := g.resolveImportPath(pathStr, workflowDir); resolvedPath != "" { + imports = append(imports, resolvedPath) + } + } + } + } + } + case []string: + for _, importPath := range v { + if resolvedPath := g.resolveImportPath(importPath, workflowDir); resolvedPath != "" { + imports = append(imports, resolvedPath) + } + } + } + + return imports +} + +// resolveImportPath resolves an import path to an absolute file path +func (g *DependencyGraph) resolveImportPath(importPath string, baseDir string) string { + // Handle section references (file.md#Section) - strip the section part + if strings.Contains(importPath, "#") { + parts := strings.SplitN(importPath, "#", 2) + importPath = parts[0] + } + + // Try to resolve as relative path first + if !filepath.IsAbs(importPath) { + absPath := filepath.Join(baseDir, importPath) + if _, err := os.Stat(absPath); err == nil { + depGraphLog.Printf("Resolved import %s to %s", importPath, absPath) + return absPath + } + } + + // If that fails, try resolving with parser's cache-aware resolution + // Note: We create a minimal cache here just for resolution + importCache := parser.NewImportCache(g.findGitRoot()) + fullPath, err := parser.ResolveIncludePath(importPath, baseDir, importCache) + if err != nil { + depGraphLog.Printf("Failed to resolve import path %s: %v", importPath, err) + return "" + } + + depGraphLog.Printf("Resolved import %s to %s", importPath, fullPath) + return fullPath +} + +// findGitRoot finds the git repository root +func (g *DependencyGraph) findGitRoot() string { + // Start from workflows directory and walk up + dir := g.workflowsDir + for { + gitDir := filepath.Join(dir, ".git") + if _, err := os.Stat(gitDir); err == nil { + return dir + } + parent := filepath.Dir(dir) + if parent == dir { + // Reached filesystem root + break + } + dir = parent + } + return g.workflowsDir // Fallback to workflows dir +} + +// GetAffectedWorkflows returns the list of workflows that need to be recompiled +// when the given file is modified +func (g *DependencyGraph) GetAffectedWorkflows(modifiedPath string) []string { + depGraphLog.Printf("Finding affected workflows for modified file: %s", modifiedPath) + + node, exists := g.nodes[modifiedPath] + if !exists { + // File not in graph - it might be a new file + // If it's a top-level workflow, just compile it + if g.isTopLevelWorkflow(modifiedPath) { + depGraphLog.Printf("Modified file is a new top-level workflow: %s", modifiedPath) + return []string{modifiedPath} + } + // If it's a shared workflow, find all top-level workflows + depGraphLog.Printf("Modified file is a new shared workflow, returning all top-level workflows") + return g.getAllTopLevelWorkflows() + } + + // If it's a top-level workflow, just recompile it + if node.IsTopLevel { + depGraphLog.Printf("Modified file is a top-level workflow: %s", modifiedPath) + return []string{modifiedPath} + } + + // If it's a shared workflow, find all workflows that import it (directly or indirectly) + // and return only the top-level ones + affected := g.findAffectedTopLevelWorkflows(modifiedPath) + depGraphLog.Printf("Found %d affected top-level workflows for shared workflow %s", len(affected), modifiedPath) + return affected +} + +// findAffectedTopLevelWorkflows finds all top-level workflows that depend on the given file +func (g *DependencyGraph) findAffectedTopLevelWorkflows(filePath string) []string { + visited := make(map[string]bool) + var topLevelWorkflows []string + + // BFS to find all workflows that import this file + queue := []string{filePath} + visited[filePath] = true + + for len(queue) > 0 { + current := queue[0] + queue = queue[1:] + + // Get all workflows that import this file + importers := g.reverseImports[current] + for _, importer := range importers { + if visited[importer] { + continue + } + visited[importer] = true + + node := g.nodes[importer] + if node != nil && node.IsTopLevel { + // Found a top-level workflow that depends on the modified file + topLevelWorkflows = append(topLevelWorkflows, importer) + depGraphLog.Printf("Found top-level workflow affected: %s", importer) + } else { + // This is an intermediate shared workflow, continue searching + queue = append(queue, importer) + depGraphLog.Printf("Found intermediate workflow: %s", importer) + } + } + } + + return topLevelWorkflows +} + +// getAllTopLevelWorkflows returns all top-level workflows in the graph +func (g *DependencyGraph) getAllTopLevelWorkflows() []string { + var topLevel []string + for path, node := range g.nodes { + if node.IsTopLevel { + topLevel = append(topLevel, path) + } + } + depGraphLog.Printf("Found %d top-level workflows in graph", len(topLevel)) + return topLevel +} + +// UpdateWorkflow updates a workflow in the graph (e.g., after it's been modified) +func (g *DependencyGraph) UpdateWorkflow(workflowPath string, compiler *workflow.Compiler) error { + depGraphLog.Printf("Updating workflow in graph: %s", workflowPath) + + // Remove old reverse imports for this workflow + if oldNode, exists := g.nodes[workflowPath]; exists { + for _, importPath := range oldNode.Imports { + g.removeReverseImport(importPath, workflowPath) + } + } + + // Re-add the workflow with updated imports + delete(g.nodes, workflowPath) + return g.addWorkflow(workflowPath, compiler) +} + +// removeReverseImport removes a reverse import entry +func (g *DependencyGraph) removeReverseImport(importPath string, importer string) { + importers := g.reverseImports[importPath] + for i, imp := range importers { + if imp == importer { + // Remove this entry + g.reverseImports[importPath] = append(importers[:i], importers[i+1:]...) + break + } + } + // Clean up empty entries + if len(g.reverseImports[importPath]) == 0 { + delete(g.reverseImports, importPath) + } +} + +// RemoveWorkflow removes a workflow from the graph (e.g., when deleted) +func (g *DependencyGraph) RemoveWorkflow(workflowPath string) { + depGraphLog.Printf("Removing workflow from graph: %s", workflowPath) + + node, exists := g.nodes[workflowPath] + if !exists { + return + } + + // Remove reverse imports + for _, importPath := range node.Imports { + g.removeReverseImport(importPath, workflowPath) + } + + // Remove the node + delete(g.nodes, workflowPath) + + // Also remove from reverse imports if others import it + delete(g.reverseImports, workflowPath) +} diff --git a/pkg/cli/dependency_graph_test.go b/pkg/cli/dependency_graph_test.go new file mode 100644 index 00000000000..ce668c24c78 --- /dev/null +++ b/pkg/cli/dependency_graph_test.go @@ -0,0 +1,764 @@ +package cli + +import ( + "fmt" + "os" + "path/filepath" + "testing" + + "github.com/githubnext/gh-aw/pkg/workflow" +) + +func TestDependencyGraph_IsTopLevelWorkflow(t *testing.T) { + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + if err := os.MkdirAll(workflowsDir, 0755); err != nil { + t.Fatal(err) + } + + graph := NewDependencyGraph(workflowsDir) + + tests := []struct { + name string + path string + wantTopLevel bool + }{ + { + name: "top-level workflow", + path: filepath.Join(workflowsDir, "main.md"), + wantTopLevel: true, + }, + { + name: "shared workflow in subdirectory", + path: filepath.Join(workflowsDir, "shared", "helper.md"), + wantTopLevel: false, + }, + { + name: "nested shared workflow", + path: filepath.Join(workflowsDir, "shared", "mcp", "tool.md"), + wantTopLevel: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := graph.isTopLevelWorkflow(tt.path) + if got != tt.wantTopLevel { + t.Errorf("isTopLevelWorkflow() = %v, want %v", got, tt.wantTopLevel) + } + }) + } +} + +func TestDependencyGraph_BuildGraphAndGetAffectedWorkflows(t *testing.T) { + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + sharedDir := filepath.Join(workflowsDir, "shared") + if err := os.MkdirAll(sharedDir, 0755); err != nil { + t.Fatal(err) + } + + // Create a shared workflow + sharedWorkflow := filepath.Join(sharedDir, "helper.md") + sharedContent := `--- +description: Helper workflow +--- +# Helper` + if err := os.WriteFile(sharedWorkflow, []byte(sharedContent), 0644); err != nil { + t.Fatal(err) + } + + // Create a top-level workflow that imports the shared workflow + topWorkflow1 := filepath.Join(workflowsDir, "main.md") + topContent1 := `--- +description: Main workflow +imports: + - shared/helper.md +--- +# Main` + if err := os.WriteFile(topWorkflow1, []byte(topContent1), 0644); err != nil { + t.Fatal(err) + } + + // Create another top-level workflow that also imports the shared workflow + topWorkflow2 := filepath.Join(workflowsDir, "secondary.md") + topContent2 := `--- +description: Secondary workflow +imports: + - shared/helper.md +--- +# Secondary` + if err := os.WriteFile(topWorkflow2, []byte(topContent2), 0644); err != nil { + t.Fatal(err) + } + + // Create a top-level workflow without imports + topWorkflow3 := filepath.Join(workflowsDir, "standalone.md") + topContent3 := `--- +description: Standalone workflow +--- +# Standalone` + if err := os.WriteFile(topWorkflow3, []byte(topContent3), 0644); err != nil { + t.Fatal(err) + } + + // Build dependency graph + graph := NewDependencyGraph(workflowsDir) + compiler := workflow.NewCompiler(false, "", "test") + if err := graph.BuildGraph(compiler); err != nil { + t.Fatalf("BuildGraph() error = %v", err) + } + + // Test 1: Modifying the shared workflow should affect both top-level workflows that import it + t.Run("shared workflow modification affects importers", func(t *testing.T) { + affected := graph.GetAffectedWorkflows(sharedWorkflow) + + // Should return both main.md and secondary.md + expectedCount := 2 + if len(affected) != expectedCount { + t.Errorf("GetAffectedWorkflows() returned %d workflows, want %d", len(affected), expectedCount) + } + + // Check that both importers are in the list + affectedMap := make(map[string]bool) + for _, w := range affected { + affectedMap[w] = true + } + + if !affectedMap[topWorkflow1] { + t.Errorf("GetAffectedWorkflows() should include %s", topWorkflow1) + } + if !affectedMap[topWorkflow2] { + t.Errorf("GetAffectedWorkflows() should include %s", topWorkflow2) + } + }) + + // Test 2: Modifying a top-level workflow should only affect itself + t.Run("top-level workflow modification affects only itself", func(t *testing.T) { + affected := graph.GetAffectedWorkflows(topWorkflow1) + + if len(affected) != 1 { + t.Errorf("GetAffectedWorkflows() returned %d workflows, want 1", len(affected)) + } + + if len(affected) > 0 && affected[0] != topWorkflow1 { + t.Errorf("GetAffectedWorkflows() = %v, want [%s]", affected, topWorkflow1) + } + }) + + // Test 3: Modifying a standalone workflow should only affect itself + t.Run("standalone workflow modification affects only itself", func(t *testing.T) { + affected := graph.GetAffectedWorkflows(topWorkflow3) + + if len(affected) != 1 { + t.Errorf("GetAffectedWorkflows() returned %d workflows, want 1", len(affected)) + } + + if len(affected) > 0 && affected[0] != topWorkflow3 { + t.Errorf("GetAffectedWorkflows() = %v, want [%s]", affected, topWorkflow3) + } + }) +} + +func TestDependencyGraph_UpdateAndRemoveWorkflow(t *testing.T) { + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + sharedDir := filepath.Join(workflowsDir, "shared") + if err := os.MkdirAll(sharedDir, 0755); err != nil { + t.Fatal(err) + } + + // Create a shared workflow + sharedWorkflow := filepath.Join(sharedDir, "helper.md") + sharedContent := `--- +description: Helper workflow +--- +# Helper` + if err := os.WriteFile(sharedWorkflow, []byte(sharedContent), 0644); err != nil { + t.Fatal(err) + } + + // Create a top-level workflow that imports the shared workflow + topWorkflow := filepath.Join(workflowsDir, "main.md") + topContent := `--- +description: Main workflow +imports: + - shared/helper.md +--- +# Main` + if err := os.WriteFile(topWorkflow, []byte(topContent), 0644); err != nil { + t.Fatal(err) + } + + // Build dependency graph + graph := NewDependencyGraph(workflowsDir) + compiler := workflow.NewCompiler(false, "", "test") + if err := graph.BuildGraph(compiler); err != nil { + t.Fatalf("BuildGraph() error = %v", err) + } + + // Test: Update workflow to remove import + t.Run("update workflow removes old dependencies", func(t *testing.T) { + // Update the workflow to remove the import + newContent := `--- +description: Main workflow +--- +# Main (no imports)` + if err := os.WriteFile(topWorkflow, []byte(newContent), 0644); err != nil { + t.Fatal(err) + } + + // Update the workflow in the graph + if err := graph.UpdateWorkflow(topWorkflow, compiler); err != nil { + t.Fatalf("UpdateWorkflow() error = %v", err) + } + + // Now modifying the shared workflow should not affect the top-level workflow + affected := graph.GetAffectedWorkflows(sharedWorkflow) + if len(affected) != 0 { + t.Errorf("After update, GetAffectedWorkflows() returned %d workflows, want 0", len(affected)) + } + }) + + // Test: Remove workflow + t.Run("remove workflow cleans up dependencies", func(t *testing.T) { + // Remove the workflow + graph.RemoveWorkflow(topWorkflow) + + // Check that the workflow is no longer in the graph + if _, exists := graph.nodes[topWorkflow]; exists { + t.Error("RemoveWorkflow() did not remove the node from the graph") + } + + // Check that reverse imports are cleaned up + if importers, exists := graph.reverseImports[sharedWorkflow]; exists && len(importers) > 0 { + t.Errorf("RemoveWorkflow() did not clean up reverse imports, still has %d importers", len(importers)) + } + }) +} + +func TestDependencyGraph_NestedImports(t *testing.T) { + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + sharedDir := filepath.Join(workflowsDir, "shared") + if err := os.MkdirAll(sharedDir, 0755); err != nil { + t.Fatal(err) + } + + // Create a base shared workflow (leaf) + baseWorkflow := filepath.Join(sharedDir, "base.md") + baseContent := `--- +description: Base workflow +--- +# Base` + if err := os.WriteFile(baseWorkflow, []byte(baseContent), 0644); err != nil { + t.Fatal(err) + } + + // Create an intermediate shared workflow that imports the base + intermediateWorkflow := filepath.Join(sharedDir, "intermediate.md") + intermediateContent := `--- +description: Intermediate workflow +imports: + - base.md +--- +# Intermediate` + if err := os.WriteFile(intermediateWorkflow, []byte(intermediateContent), 0644); err != nil { + t.Fatal(err) + } + + // Create a top-level workflow that imports the intermediate workflow + topWorkflow := filepath.Join(workflowsDir, "main.md") + topContent := `--- +description: Main workflow +imports: + - shared/intermediate.md +--- +# Main` + if err := os.WriteFile(topWorkflow, []byte(topContent), 0644); err != nil { + t.Fatal(err) + } + + // Build dependency graph + graph := NewDependencyGraph(workflowsDir) + compiler := workflow.NewCompiler(false, "", "test") + if err := graph.BuildGraph(compiler); err != nil { + t.Fatalf("BuildGraph() error = %v", err) + } + + // Test: Modifying the base workflow should transitively affect the top-level workflow + t.Run("nested import modification affects top-level workflow", func(t *testing.T) { + affected := graph.GetAffectedWorkflows(baseWorkflow) + + // Should find the top-level workflow through the intermediate workflow + if len(affected) != 1 { + t.Errorf("GetAffectedWorkflows() returned %d workflows, want 1", len(affected)) + } + + if len(affected) > 0 && affected[0] != topWorkflow { + t.Errorf("GetAffectedWorkflows() = %v, want [%s]", affected, topWorkflow) + } + }) +} + +func TestDependencyGraph_MultipleTopLevelImporters(t *testing.T) { + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + sharedDir := filepath.Join(workflowsDir, "shared") + mcpDir := filepath.Join(sharedDir, "mcp") + if err := os.MkdirAll(mcpDir, 0755); err != nil { + t.Fatal(err) + } + + // Create a deeply nested shared workflow + deepShared := filepath.Join(mcpDir, "tool.md") + deepContent := `--- +description: MCP Tool +--- +# Tool` + if err := os.WriteFile(deepShared, []byte(deepContent), 0644); err != nil { + t.Fatal(err) + } + + // Create multiple top-level workflows that import the deep shared workflow + workflows := make([]string, 3) + for i := 0; i < 3; i++ { + workflows[i] = filepath.Join(workflowsDir, fmt.Sprintf("workflow%d.md", i)) + content := fmt.Sprintf(`--- +description: Workflow %d +imports: + - shared/mcp/tool.md +--- +# Workflow %d`, i, i) + if err := os.WriteFile(workflows[i], []byte(content), 0644); err != nil { + t.Fatal(err) + } + } + + // Build dependency graph + graph := NewDependencyGraph(workflowsDir) + compiler := workflow.NewCompiler(false, "", "test") + if err := graph.BuildGraph(compiler); err != nil { + t.Fatalf("BuildGraph() error = %v", err) + } + + // Test: Modifying the deep shared workflow should affect all three top-level workflows + affected := graph.GetAffectedWorkflows(deepShared) + + if len(affected) != 3 { + t.Errorf("GetAffectedWorkflows() returned %d workflows, want 3", len(affected)) + } + + // Verify all three workflows are in the affected list + affectedMap := make(map[string]bool) + for _, w := range affected { + affectedMap[w] = true + } + + for i, wf := range workflows { + if !affectedMap[wf] { + t.Errorf("GetAffectedWorkflows() should include workflow%d.md", i) + } + } +} + +func TestDependencyGraph_CircularImportDetection(t *testing.T) { + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + sharedDir := filepath.Join(workflowsDir, "shared") + if err := os.MkdirAll(sharedDir, 0755); err != nil { + t.Fatal(err) + } + + // Create workflow A that imports B + workflowA := filepath.Join(sharedDir, "a.md") + contentA := `--- +description: Workflow A +imports: + - b.md +--- +# A` + if err := os.WriteFile(workflowA, []byte(contentA), 0644); err != nil { + t.Fatal(err) + } + + // Create workflow B that imports A (circular dependency) + workflowB := filepath.Join(sharedDir, "b.md") + contentB := `--- +description: Workflow B +imports: + - a.md +--- +# B` + if err := os.WriteFile(workflowB, []byte(contentB), 0644); err != nil { + t.Fatal(err) + } + + // Create a top-level workflow that imports A + topWorkflow := filepath.Join(workflowsDir, "main.md") + topContent := `--- +description: Main workflow +imports: + - shared/a.md +--- +# Main` + if err := os.WriteFile(topWorkflow, []byte(topContent), 0644); err != nil { + t.Fatal(err) + } + + // Build dependency graph - should handle circular imports gracefully + graph := NewDependencyGraph(workflowsDir) + compiler := workflow.NewCompiler(false, "", "test") + if err := graph.BuildGraph(compiler); err != nil { + t.Fatalf("BuildGraph() error = %v", err) + } + + // Test: Modifying workflow A should affect the top-level workflow + affected := graph.GetAffectedWorkflows(workflowA) + + if len(affected) != 1 { + t.Errorf("GetAffectedWorkflows() returned %d workflows, want 1", len(affected)) + } + + if len(affected) > 0 && affected[0] != topWorkflow { + t.Errorf("GetAffectedWorkflows() = %v, want [%s]", affected, topWorkflow) + } +} + +func TestDependencyGraph_NewFileAddition(t *testing.T) { + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + sharedDir := filepath.Join(workflowsDir, "shared") + if err := os.MkdirAll(sharedDir, 0755); err != nil { + t.Fatal(err) + } + + // Create initial shared workflow + sharedWorkflow := filepath.Join(sharedDir, "helper.md") + sharedContent := `--- +description: Helper workflow +--- +# Helper` + if err := os.WriteFile(sharedWorkflow, []byte(sharedContent), 0644); err != nil { + t.Fatal(err) + } + + // Build initial dependency graph + graph := NewDependencyGraph(workflowsDir) + compiler := workflow.NewCompiler(false, "", "test") + if err := graph.BuildGraph(compiler); err != nil { + t.Fatalf("BuildGraph() error = %v", err) + } + + // Test: Adding a new top-level workflow file + t.Run("new top-level workflow affects only itself", func(t *testing.T) { + newWorkflow := filepath.Join(workflowsDir, "new.md") + newContent := `--- +description: New workflow +imports: + - shared/helper.md +--- +# New` + if err := os.WriteFile(newWorkflow, []byte(newContent), 0644); err != nil { + t.Fatal(err) + } + + // Get affected workflows for new file (not yet in graph) + affected := graph.GetAffectedWorkflows(newWorkflow) + + // Should compile only itself + if len(affected) != 1 { + t.Errorf("GetAffectedWorkflows() for new file returned %d workflows, want 1", len(affected)) + } + + if len(affected) > 0 && affected[0] != newWorkflow { + t.Errorf("GetAffectedWorkflows() = %v, want [%s]", affected, newWorkflow) + } + }) + + // Test: Adding a new shared workflow file + t.Run("new shared workflow returns all top-level workflows", func(t *testing.T) { + // Create a top-level workflow first + topWorkflow := filepath.Join(workflowsDir, "main.md") + topContent := `--- +description: Main workflow +--- +# Main` + if err := os.WriteFile(topWorkflow, []byte(topContent), 0644); err != nil { + t.Fatal(err) + } + + // Update graph to include the top-level workflow + if err := graph.addWorkflow(topWorkflow, compiler); err != nil { + t.Fatal(err) + } + + // Now test with a new shared workflow + newShared := filepath.Join(sharedDir, "new-shared.md") + newSharedContent := `--- +description: New shared workflow +--- +# New Shared` + if err := os.WriteFile(newShared, []byte(newSharedContent), 0644); err != nil { + t.Fatal(err) + } + + // Get affected workflows for new shared file (not yet in graph) + affected := graph.GetAffectedWorkflows(newShared) + + // Should return all top-level workflows as we don't know dependencies yet + if len(affected) != 1 { + t.Errorf("GetAffectedWorkflows() for new shared file returned %d workflows, want 1 (all top-level)", len(affected)) + } + }) +} + +func TestDependencyGraph_EmptyGraph(t *testing.T) { + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + if err := os.MkdirAll(workflowsDir, 0755); err != nil { + t.Fatal(err) + } + + // Build dependency graph with no workflows + graph := NewDependencyGraph(workflowsDir) + compiler := workflow.NewCompiler(false, "", "test") + if err := graph.BuildGraph(compiler); err != nil { + t.Fatalf("BuildGraph() error = %v", err) + } + + // Test: Query on empty graph + t.Run("empty graph returns empty for any file", func(t *testing.T) { + affected := graph.GetAffectedWorkflows("/nonexistent/file.md") + if len(affected) != 0 { + t.Errorf("GetAffectedWorkflows() on empty graph returned %d workflows, want 0", len(affected)) + } + }) + + // Test: Add workflow to empty graph + t.Run("add first workflow to empty graph", func(t *testing.T) { + firstWorkflow := filepath.Join(workflowsDir, "first.md") + firstContent := `--- +description: First workflow +--- +# First` + if err := os.WriteFile(firstWorkflow, []byte(firstContent), 0644); err != nil { + t.Fatal(err) + } + + if err := graph.addWorkflow(firstWorkflow, compiler); err != nil { + t.Fatal(err) + } + + // Should have one node now + if len(graph.nodes) != 1 { + t.Errorf("After adding first workflow, graph has %d nodes, want 1", len(graph.nodes)) + } + + affected := graph.GetAffectedWorkflows(firstWorkflow) + if len(affected) != 1 { + t.Errorf("GetAffectedWorkflows() returned %d workflows, want 1", len(affected)) + } + }) +} + +func TestDependencyGraph_ComplexDependencyChain(t *testing.T) { + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + sharedDir := filepath.Join(workflowsDir, "shared") + mcpDir := filepath.Join(sharedDir, "mcp") + if err := os.MkdirAll(mcpDir, 0755); err != nil { + t.Fatal(err) + } + + // Create a complex dependency chain: + // top1 -> shared/a -> shared/b -> shared/mcp/c + // top2 -> shared/a + // top3 -> shared/b + + // Level 3: Deepest shared workflow + workflowC := filepath.Join(mcpDir, "c.md") + contentC := `--- +description: Workflow C +--- +# C` + if err := os.WriteFile(workflowC, []byte(contentC), 0644); err != nil { + t.Fatal(err) + } + + // Level 2: Shared workflow B imports C + workflowB := filepath.Join(sharedDir, "b.md") + contentB := `--- +description: Workflow B +imports: + - mcp/c.md +--- +# B` + if err := os.WriteFile(workflowB, []byte(contentB), 0644); err != nil { + t.Fatal(err) + } + + // Level 1: Shared workflow A imports B + workflowA := filepath.Join(sharedDir, "a.md") + contentA := `--- +description: Workflow A +imports: + - b.md +--- +# A` + if err := os.WriteFile(workflowA, []byte(contentA), 0644); err != nil { + t.Fatal(err) + } + + // Top-level workflows + top1 := filepath.Join(workflowsDir, "top1.md") + content1 := `--- +description: Top 1 +imports: + - shared/a.md +--- +# Top 1` + if err := os.WriteFile(top1, []byte(content1), 0644); err != nil { + t.Fatal(err) + } + + top2 := filepath.Join(workflowsDir, "top2.md") + content2 := `--- +description: Top 2 +imports: + - shared/a.md +--- +# Top 2` + if err := os.WriteFile(top2, []byte(content2), 0644); err != nil { + t.Fatal(err) + } + + top3 := filepath.Join(workflowsDir, "top3.md") + content3 := `--- +description: Top 3 +imports: + - shared/b.md +--- +# Top 3` + if err := os.WriteFile(top3, []byte(content3), 0644); err != nil { + t.Fatal(err) + } + + // Build dependency graph + graph := NewDependencyGraph(workflowsDir) + compiler := workflow.NewCompiler(false, "", "test") + if err := graph.BuildGraph(compiler); err != nil { + t.Fatalf("BuildGraph() error = %v", err) + } + + // Test: Modifying C should affect top1, top2, and top3 + t.Run("modifying deepest workflow affects all importers", func(t *testing.T) { + affected := graph.GetAffectedWorkflows(workflowC) + + if len(affected) != 3 { + t.Errorf("GetAffectedWorkflows(C) returned %d workflows, want 3", len(affected)) + } + + affectedMap := make(map[string]bool) + for _, w := range affected { + affectedMap[w] = true + } + + if !affectedMap[top1] || !affectedMap[top2] || !affectedMap[top3] { + t.Errorf("GetAffectedWorkflows(C) should include top1, top2, and top3") + } + }) + + // Test: Modifying B should affect top1, top2, and top3 + t.Run("modifying intermediate workflow affects correct importers", func(t *testing.T) { + affected := graph.GetAffectedWorkflows(workflowB) + + if len(affected) != 3 { + t.Errorf("GetAffectedWorkflows(B) returned %d workflows, want 3", len(affected)) + } + + affectedMap := make(map[string]bool) + for _, w := range affected { + affectedMap[w] = true + } + + if !affectedMap[top1] || !affectedMap[top2] || !affectedMap[top3] { + t.Errorf("GetAffectedWorkflows(B) should include top1, top2, and top3") + } + }) + + // Test: Modifying A should affect only top1 and top2 + t.Run("modifying upper workflow affects only direct importers", func(t *testing.T) { + affected := graph.GetAffectedWorkflows(workflowA) + + if len(affected) != 2 { + t.Errorf("GetAffectedWorkflows(A) returned %d workflows, want 2", len(affected)) + } + + affectedMap := make(map[string]bool) + for _, w := range affected { + affectedMap[w] = true + } + + if !affectedMap[top1] || !affectedMap[top2] { + t.Errorf("GetAffectedWorkflows(A) should include top1 and top2") + } + + if affectedMap[top3] { + t.Errorf("GetAffectedWorkflows(A) should NOT include top3") + } + }) +} + +func TestDependencyGraph_ImportsWithInputs(t *testing.T) { + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + sharedDir := filepath.Join(workflowsDir, "shared") + if err := os.MkdirAll(sharedDir, 0755); err != nil { + t.Fatal(err) + } + + // Create a shared workflow + sharedWorkflow := filepath.Join(sharedDir, "parameterized.md") + sharedContent := `--- +description: Parameterized workflow +--- +# Parameterized` + if err := os.WriteFile(sharedWorkflow, []byte(sharedContent), 0644); err != nil { + t.Fatal(err) + } + + // Create a top-level workflow that imports with inputs + topWorkflow := filepath.Join(workflowsDir, "main.md") + topContent := `--- +description: Main workflow +imports: + - path: shared/parameterized.md + inputs: + key: value +--- +# Main` + if err := os.WriteFile(topWorkflow, []byte(topContent), 0644); err != nil { + t.Fatal(err) + } + + // Build dependency graph + graph := NewDependencyGraph(workflowsDir) + compiler := workflow.NewCompiler(false, "", "test") + if err := graph.BuildGraph(compiler); err != nil { + t.Fatalf("BuildGraph() error = %v", err) + } + + // Test: Graph should handle imports with inputs object format + t.Run("imports with inputs are tracked correctly", func(t *testing.T) { + affected := graph.GetAffectedWorkflows(sharedWorkflow) + + if len(affected) != 1 { + t.Errorf("GetAffectedWorkflows() returned %d workflows, want 1", len(affected)) + } + + if len(affected) > 0 && affected[0] != topWorkflow { + t.Errorf("GetAffectedWorkflows() = %v, want [%s]", affected, topWorkflow) + } + }) +} diff --git a/pkg/parser/frontmatter.go b/pkg/parser/frontmatter.go index 7ad45d984c7..9c03735fd52 100644 --- a/pkg/parser/frontmatter.go +++ b/pkg/parser/frontmatter.go @@ -228,7 +228,7 @@ func processImportsFromFrontmatterWithManifestAndSource(frontmatter map[string]a } // Resolve import path (supports workflowspec format) - fullPath, err := resolveIncludePath(filePath, baseDir, cache) + fullPath, err := ResolveIncludePath(filePath, baseDir, cache) if err != nil { // If we have source information, create a structured import error if workflowFilePath != "" && yamlContent != "" { @@ -354,7 +354,7 @@ func processImportsFromFrontmatterWithManifestAndSource(frontmatter map[string]a } // Resolve nested import path relative to the workflows directory, not the nested file's directory - nestedFullPath, err := resolveIncludePath(nestedFilePath, baseDir, cache) + nestedFullPath, err := ResolveIncludePath(nestedFilePath, baseDir, cache) if err != nil { // If we have source information for the parent workflow, create a structured error if workflowFilePath != "" && yamlContent != "" { @@ -543,7 +543,7 @@ func processIncludesWithVisited(content, baseDir string, extractTools bool, visi } // Resolve file path first to get the canonical path - fullPath, err := resolveIncludePath(filePath, baseDir, nil) + fullPath, err := ResolveIncludePath(filePath, baseDir, nil) if err != nil { if isOptional { // For optional includes, show a friendly informational message to stdout @@ -1002,7 +1002,7 @@ func processIncludesForField(content, baseDir string, extractFunc func(string) ( } // Resolve file path - fullPath, err := resolveIncludePath(filePath, baseDir, nil) + fullPath, err := ResolveIncludePath(filePath, baseDir, nil) if err != nil { if isOptional { // For optional includes, skip extraction diff --git a/pkg/parser/frontmatter_utils_test.go b/pkg/parser/frontmatter_utils_test.go index 1102c8708f3..191900f207c 100644 --- a/pkg/parser/frontmatter_utils_test.go +++ b/pkg/parser/frontmatter_utils_test.go @@ -95,22 +95,22 @@ func TestResolveIncludePath(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result, err := resolveIncludePath(tt.filePath, tt.baseDir, nil) + result, err := ResolveIncludePath(tt.filePath, tt.baseDir, nil) if tt.wantErr { if err == nil { - t.Errorf("resolveIncludePath() expected error, got nil") + t.Errorf("ResolveIncludePath() expected error, got nil") } return } if err != nil { - t.Errorf("resolveIncludePath() error = %v", err) + t.Errorf("ResolveIncludePath() error = %v", err) return } if result != tt.expected { - t.Errorf("resolveIncludePath() = %q, want %q", result, tt.expected) + t.Errorf("ResolveIncludePath() = %q, want %q", result, tt.expected) } }) } diff --git a/pkg/parser/remote_fetch.go b/pkg/parser/remote_fetch.go index 18d5f64de83..491e32287e7 100644 --- a/pkg/parser/remote_fetch.go +++ b/pkg/parser/remote_fetch.go @@ -37,8 +37,8 @@ func isUnderWorkflowsDirectory(filePath string) bool { return !strings.Contains(afterWorkflows, "/") } -// resolveIncludePath resolves include path based on workflowspec format or relative path -func resolveIncludePath(filePath, baseDir string, cache *ImportCache) (string, error) { +// ResolveIncludePath resolves include path based on workflowspec format or relative path +func ResolveIncludePath(filePath, baseDir string, cache *ImportCache) (string, error) { remoteLog.Printf("Resolving include path: file_path=%s, base_dir=%s", filePath, baseDir) // Check if this is a workflowspec (contains owner/repo/path format)