diff --git a/pkg/workflow/dispatch_workflow_validation.go b/pkg/workflow/dispatch_workflow_validation.go index a68b98c6d05..af07aaef572 100644 --- a/pkg/workflow/dispatch_workflow_validation.go +++ b/pkg/workflow/dispatch_workflow_validation.go @@ -8,6 +8,7 @@ import ( "strings" "github.com/github/gh-aw/pkg/fileutil" + "github.com/github/gh-aw/pkg/parser" "github.com/goccy/go-yaml" ) @@ -99,12 +100,26 @@ func (c *Compiler) validateDispatchWorkflow(data *WorkflowData, workflowPath str continue // Skip further validation for this workflow } } else { - // Only .md exists - needs to be compiled first - compileErr := fmt.Errorf("dispatch-workflow: workflow '%s' must be compiled first\n\nThe workflow source file exists at: %s\nBut the compiled .lock.yml file is missing.\n\nTo fix:\n1. Compile the workflow: gh aw compile %s\n2. Commit the generated .lock.yml file\n3. Ensure .lock.yml files are not in .gitignore", workflowName, fileResult.mdPath, workflowName) - if returnErr := collector.Add(compileErr); returnErr != nil { - return returnErr // Fail-fast mode + // Only .md exists — it may be a same-batch compilation target. + // Validate via the .md frontmatter so a second compile pass is not required. + mdHasDispatch, checkErr := mdHasWorkflowDispatch(fileResult.mdPath) + if checkErr != nil { + readErr := fmt.Errorf("dispatch-workflow: failed to read workflow source %s: %w", fileResult.mdPath, checkErr) + if returnErr := collector.Add(readErr); returnErr != nil { + return returnErr // Fail-fast mode + } + continue // Skip further validation for this workflow } - continue // Skip further validation for this workflow + if !mdHasDispatch { + dispatchErr := fmt.Errorf("dispatch-workflow: workflow '%s' does not support workflow_dispatch trigger (must include 'workflow_dispatch' in the 'on' section)", workflowName) + if returnErr := collector.Add(dispatchErr); returnErr != nil { + return returnErr // Fail-fast mode + } + continue // Skip further validation for this workflow + } + // .md exists with workflow_dispatch — valid same-batch compilation target. + dispatchWorkflowValidationLog.Printf("Workflow '%s' is valid for dispatch (found .md source at %s with workflow_dispatch trigger)", workflowName, fileResult.mdPath) + continue // Trigger validated; skip YAML-specific checks below } // Parse the workflow YAML to check for workflow_dispatch trigger @@ -127,28 +142,7 @@ func (c *Compiler) validateDispatchWorkflow(data *WorkflowData, workflowPath str continue // Skip further validation for this workflow } - // Check if workflow_dispatch is in the "on" section - hasWorkflowDispatch := false - switch on := onSection.(type) { - case string: - // Simple trigger like "on: push" - if on == "workflow_dispatch" { - hasWorkflowDispatch = true - } - case []any: - // Array of triggers like "on: [push, workflow_dispatch]" - for _, trigger := range on { - if triggerStr, ok := trigger.(string); ok && triggerStr == "workflow_dispatch" { - hasWorkflowDispatch = true - break - } - } - case map[string]any: - // Map of triggers like "on: { push: {}, workflow_dispatch: {} }" - _, hasWorkflowDispatch = on["workflow_dispatch"] - } - - if !hasWorkflowDispatch { + if !containsWorkflowDispatch(onSection) { dispatchErr := fmt.Errorf("dispatch-workflow: workflow '%s' does not support workflow_dispatch trigger (must include 'workflow_dispatch' in the 'on' section)", workflowName) if returnErr := collector.Add(dispatchErr); returnErr != nil { return returnErr // Fail-fast mode @@ -296,3 +290,83 @@ func findWorkflowFile(workflowName string, currentWorkflowPath string) (*findWor return result, nil } + +// mdHasWorkflowDispatch reads a .md workflow file's frontmatter and reports whether +// the workflow includes a workflow_dispatch trigger in its 'on:' section. +// This is used to validate same-batch dispatch-workflow targets whose .lock.yml has +// not yet been generated. +func mdHasWorkflowDispatch(mdPath string) (bool, error) { + content, err := os.ReadFile(mdPath) // #nosec G304 -- mdPath is validated via isPathWithinDir in findWorkflowFile + if err != nil { + return false, err + } + result, err := parser.ExtractFrontmatterFromContent(string(content)) + if err != nil || result == nil { + return false, err + } + onSection, hasOn := result.Frontmatter["on"] + if !hasOn { + return false, nil + } + return containsWorkflowDispatch(onSection), nil +} + +// extractMDWorkflowDispatchInputs reads a .md workflow file's frontmatter and extracts +// the workflow_dispatch inputs schema, mirroring extractWorkflowDispatchInputs for .md sources. +func extractMDWorkflowDispatchInputs(mdPath string) (map[string]any, error) { + content, err := os.ReadFile(mdPath) // #nosec G304 -- mdPath is validated via isPathWithinDir in findWorkflowFile + if err != nil { + return nil, err + } + result, err := parser.ExtractFrontmatterFromContent(string(content)) + if err != nil || result == nil { + return make(map[string]any), nil + } + onSection, hasOn := result.Frontmatter["on"] + if !hasOn { + return make(map[string]any), nil + } + onMap, ok := onSection.(map[string]any) + if !ok { + return make(map[string]any), nil + } + workflowDispatch, hasWorkflowDispatch := onMap["workflow_dispatch"] + if !hasWorkflowDispatch { + return make(map[string]any), nil + } + workflowDispatchMap, ok := workflowDispatch.(map[string]any) + if !ok { + return make(map[string]any), nil + } + inputs, hasInputs := workflowDispatchMap["inputs"] + if !hasInputs { + return make(map[string]any), nil + } + inputsMap, ok := inputs.(map[string]any) + if !ok { + return make(map[string]any), nil + } + return inputsMap, nil +} + +// containsWorkflowDispatch reports whether the given 'on:' section value includes +// a workflow_dispatch trigger. It handles the three GitHub Actions forms: +// - string: "on: workflow_dispatch" +// - []any: "on: [push, workflow_dispatch]" +// - map[string]any: "on:\n workflow_dispatch: ..." +func containsWorkflowDispatch(onSection any) bool { + switch on := onSection.(type) { + case string: + return on == "workflow_dispatch" + case []any: + for _, trigger := range on { + if triggerStr, ok := trigger.(string); ok && triggerStr == "workflow_dispatch" { + return true + } + } + case map[string]any: + _, ok := on["workflow_dispatch"] + return ok + } + return false +} diff --git a/pkg/workflow/dispatch_workflow_validation_test.go b/pkg/workflow/dispatch_workflow_validation_test.go index b385fcad57e..adfaab291ac 100644 --- a/pkg/workflow/dispatch_workflow_validation_test.go +++ b/pkg/workflow/dispatch_workflow_validation_test.go @@ -176,9 +176,10 @@ This workflow tries to dispatch to itself. assert.Contains(t, errMsg, "workflow_dispatch", "Should suggest workflow_dispatch alternative") } -// TestDispatchWorkflowErrorMessage_MustCompile tests that uncompiled workflow -// error message includes compilation instructions -func TestDispatchWorkflowErrorMessage_MustCompile(t *testing.T) { +// TestDispatchWorkflowBatchAware_MDWithDispatch tests that a workflow that only has a .md file +// (no .lock.yml) is accepted as a valid same-batch dispatch target when the .md has +// workflow_dispatch in its 'on:' section. +func TestDispatchWorkflowBatchAware_MDWithDispatch(t *testing.T) { compiler := NewCompilerWithVersion("1.0.0") tmpDir := t.TempDir() @@ -190,7 +191,7 @@ func TestDispatchWorkflowErrorMessage_MustCompile(t *testing.T) { err = os.MkdirAll(workflowsDir, 0755) require.NoError(t, err, "Failed to create workflows directory") - // Create an uncompiled workflow (only .md file, no .lock.yml) + // Create a target workflow that only has .md (no .lock.yml) with workflow_dispatch trigger targetWorkflow := `--- on: workflow_dispatch engine: copilot @@ -200,13 +201,13 @@ permissions: # Target Workflow -This workflow needs to be compiled. +This workflow is a same-batch compilation target. ` targetFile := filepath.Join(workflowsDir, "target.md") err = os.WriteFile(targetFile, []byte(targetWorkflow), 0644) require.NoError(t, err, "Failed to write target workflow") - // Create a dispatcher workflow that references the uncompiled workflow + // Create a dispatcher workflow that references the .md-only target dispatcherWorkflow := `--- on: issues engine: copilot @@ -221,7 +222,7 @@ safe-outputs: # Dispatcher Workflow -This workflow references an uncompiled workflow. +This workflow dispatches to a same-batch target. ` dispatcherFile := filepath.Join(awDir, "dispatcher.md") err = os.WriteFile(dispatcherFile, []byte(dispatcherWorkflow), 0644) @@ -238,20 +239,75 @@ This workflow references an uncompiled workflow. workflowData, err := compiler.ParseWorkflowFile("dispatcher.md") require.NoError(t, err, "Failed to parse workflow") - // Validate the workflow - should fail with enhanced error message + // Validation should succeed: .md exists with workflow_dispatch (same-batch target) err = compiler.validateDispatchWorkflow(workflowData, dispatcherFile) - require.Error(t, err, "Validation should fail for uncompiled workflow") + assert.NoError(t, err, "Validation should pass for .md-only same-batch target with workflow_dispatch") +} - // Verify enhanced error message content - errMsg := err.Error() - assert.Contains(t, errMsg, "must be compiled first", "Should state the requirement") - assert.Contains(t, errMsg, "target", "Should mention workflow name") - assert.Contains(t, errMsg, "source file exists", "Should acknowledge file exists") - assert.Contains(t, errMsg, "compiled .lock.yml file is missing", "Should explain what's missing") - assert.Contains(t, errMsg, "To fix:", "Should include fix instructions header") - assert.Contains(t, errMsg, "gh aw compile target", "Should show exact compilation command") - assert.Contains(t, errMsg, "Commit the generated .lock.yml", "Should mention committing") - assert.Contains(t, errMsg, ".gitignore", "Should warn about gitignore") +// TestDispatchWorkflowBatchAware_MDWithoutDispatch tests that a workflow that only has a .md file +// (no .lock.yml) and does NOT have workflow_dispatch in its 'on:' section fails validation. +func TestDispatchWorkflowBatchAware_MDWithoutDispatch(t *testing.T) { + compiler := NewCompilerWithVersion("1.0.0") + + tmpDir := t.TempDir() + awDir := filepath.Join(tmpDir, ".github", "aw") + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + + err := os.MkdirAll(awDir, 0755) + require.NoError(t, err, "Failed to create aw directory") + err = os.MkdirAll(workflowsDir, 0755) + require.NoError(t, err, "Failed to create workflows directory") + + // Create a target workflow that only has .md (no .lock.yml) WITHOUT workflow_dispatch + targetWorkflow := `--- +on: issues +engine: copilot +permissions: + contents: read +--- + +# Target Workflow + +This workflow does not support workflow_dispatch. +` + targetFile := filepath.Join(workflowsDir, "target.md") + err = os.WriteFile(targetFile, []byte(targetWorkflow), 0644) + require.NoError(t, err, "Failed to write target workflow") + + // Create a dispatcher workflow that references the .md-only target + dispatcherWorkflow := `--- +on: issues +engine: copilot +permissions: + contents: read +safe-outputs: + dispatch-workflow: + workflows: + - target + max: 1 +--- + +# Dispatcher Workflow +` + dispatcherFile := filepath.Join(awDir, "dispatcher.md") + err = os.WriteFile(dispatcherFile, []byte(dispatcherWorkflow), 0644) + require.NoError(t, err, "Failed to write dispatcher workflow") + + // Change to the aw directory + oldDir, err := os.Getwd() + require.NoError(t, err, "Failed to get current directory") + err = os.Chdir(awDir) + require.NoError(t, err, "Failed to change directory") + defer func() { _ = os.Chdir(oldDir) }() + + // Parse the dispatcher workflow + workflowData, err := compiler.ParseWorkflowFile("dispatcher.md") + require.NoError(t, err, "Failed to parse workflow") + + // Validation should fail: .md exists but lacks workflow_dispatch + err = compiler.validateDispatchWorkflow(workflowData, dispatcherFile) + require.Error(t, err, "Validation should fail when .md target lacks workflow_dispatch") + assert.Contains(t, err.Error(), "does not support workflow_dispatch trigger", "Should explain missing trigger") } // TestDispatchWorkflowErrorMessage_MultipleErrors tests that multiple errors diff --git a/pkg/workflow/safe_outputs_generation.go b/pkg/workflow/safe_outputs_generation.go index 4c90f7b7605..5cf7bfaac8a 100644 --- a/pkg/workflow/safe_outputs_generation.go +++ b/pkg/workflow/safe_outputs_generation.go @@ -132,14 +132,17 @@ func populateDispatchWorkflowFiles(data *WorkflowData, markdownPath string) { continue } - // Determine which file to use - priority: .lock.yml > .yml + // Determine which file to use - priority: .lock.yml > .yml > .md (batch target) var extension string if fileResult.lockExists { extension = ".lock.yml" } else if fileResult.ymlExists { extension = ".yml" + } else if fileResult.mdExists { + // .md-only: the workflow is a same-batch compilation target that will produce a .lock.yml + extension = ".lock.yml" } else { - safeOutputsConfigLog.Printf("Warning: workflow file not found for %s (only .md exists, needs compilation)", workflowName) + safeOutputsConfigLog.Printf("Warning: no workflow file found for %s (checked .lock.yml, .yml, .md)", workflowName) continue } @@ -1314,17 +1317,23 @@ func generateFilteredToolsJSON(data *WorkflowData, markdownPath string) (string, continue } - // Determine which file to use - priority: .lock.yml > .yml + // Determine which file to use - priority: .lock.yml > .yml > .md (batch target) var workflowPath string var extension string + var useMD bool if fileResult.lockExists { workflowPath = fileResult.lockPath extension = ".lock.yml" } else if fileResult.ymlExists { workflowPath = fileResult.ymlPath extension = ".yml" + } else if fileResult.mdExists { + // .md-only: the workflow is a same-batch compilation target that will produce a .lock.yml + workflowPath = fileResult.mdPath + extension = ".lock.yml" + useMD = true } else { - safeOutputsConfigLog.Printf("Warning: workflow file not found for %s (only .md exists, needs compilation)", workflowName) + safeOutputsConfigLog.Printf("Warning: no workflow file found for %s (checked .lock.yml, .yml, .md)", workflowName) // Continue with empty inputs tool := generateDispatchWorkflowTool(workflowName, make(map[string]any)) filteredTools = append(filteredTools, tool) @@ -1335,9 +1344,15 @@ func generateFilteredToolsJSON(data *WorkflowData, markdownPath string) (string, data.SafeOutputs.DispatchWorkflow.WorkflowFiles[workflowName] = extension // Extract workflow_dispatch inputs - workflowInputs, err := extractWorkflowDispatchInputs(workflowPath) - if err != nil { - safeOutputsConfigLog.Printf("Warning: failed to extract inputs for workflow %s from %s: %v", workflowName, workflowPath, err) + var workflowInputs map[string]any + var inputsErr error + if useMD { + workflowInputs, inputsErr = extractMDWorkflowDispatchInputs(workflowPath) + } else { + workflowInputs, inputsErr = extractWorkflowDispatchInputs(workflowPath) + } + if inputsErr != nil { + safeOutputsConfigLog.Printf("Warning: failed to extract inputs for workflow %s from %s: %v", workflowName, workflowPath, inputsErr) // Continue with empty inputs workflowInputs = make(map[string]any) }