Skip to content

Commit 15a01e5

Browse files
CopilotCopilot
authored
Enhance dispatch-workflow validation errors with actionable guidance (#14447)
1 parent 324e407 commit 15a01e5

3 files changed

Lines changed: 328 additions & 6 deletions

File tree

‎.golangci.yml‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,12 @@ issues:
8181
exclude-rules:
8282
- linters:
8383
- staticcheck
84-
text: "ST1005:" # Allow multiline user-facing error messages with formatting
84+
text: "ST1005: error strings should not end with punctuation or newlines" # Allow multiline user-facing error messages with formatting
8585
path: pkg/workflow/compiler_orchestrator\.go
86+
- linters:
87+
- staticcheck
88+
text: "ST1005: error strings should not end with punctuation or newlines" # Allow multiline user-facing error messages with formatting
89+
path: pkg/workflow/dispatch_workflow_validation\.go
8690
- linters:
8791
- gosec
8892
text: "^G304:" # Ignore "file inclusion via variable" - validated file paths

‎pkg/workflow/dispatch_workflow_validation.go‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ func (c *Compiler) validateDispatchWorkflow(data *WorkflowData, workflowPath str
2424
config := data.SafeOutputs.DispatchWorkflow
2525

2626
if len(config.Workflows) == 0 {
27-
return fmt.Errorf("dispatch-workflow: must specify at least one workflow in the list")
27+
return fmt.Errorf("dispatch-workflow: must specify at least one workflow in the list\n\nExample configuration in workflow frontmatter:\nsafe-outputs:\n dispatch-workflow:\n workflows: [workflow-name-1, workflow-name-2]\n\nWorkflow names should match the filename without the .md extension")
2828
}
2929

3030
// Get the current workflow name for self-reference check
@@ -39,7 +39,7 @@ func (c *Compiler) validateDispatchWorkflow(data *WorkflowData, workflowPath str
3939

4040
// Check for self-reference
4141
if workflowName == currentWorkflowName {
42-
selfRefErr := fmt.Errorf("dispatch-workflow: self-reference not allowed (workflow '%s' cannot dispatch itself)", workflowName)
42+
selfRefErr := fmt.Errorf("dispatch-workflow: self-reference not allowed (workflow '%s' cannot dispatch itself)\n\nA workflow cannot trigger itself to prevent infinite loops.\nIf you need recurring execution, use a schedule trigger or workflow_dispatch instead", workflowName)
4343
if returnErr := collector.Add(selfRefErr); returnErr != nil {
4444
return returnErr // Fail-fast mode
4545
}
@@ -64,8 +64,7 @@ func (c *Compiler) validateDispatchWorkflow(data *WorkflowData, workflowPath str
6464
repoRoot := filepath.Dir(githubDir)
6565
workflowsDir := filepath.Join(repoRoot, ".github", "workflows")
6666

67-
notFoundErr := fmt.Errorf("dispatch-workflow: workflow '%s' not found in %s (tried .md, .lock.yml, and .yml extensions)",
68-
workflowName, workflowsDir)
67+
notFoundErr := fmt.Errorf("dispatch-workflow: workflow '%s' not found in %s\n\nChecked for: %s.md, %s.lock.yml, %s.yml\n\nTo fix:\n1. Verify the workflow file exists in .github/workflows/\n2. Ensure the filename matches exactly (case-sensitive)\n3. Use the filename without extension in your configuration", workflowName, workflowsDir, workflowName, workflowName, workflowName)
6968
if returnErr := collector.Add(notFoundErr); returnErr != nil {
7069
return returnErr // Fail-fast mode
7170
}
@@ -100,7 +99,7 @@ func (c *Compiler) validateDispatchWorkflow(data *WorkflowData, workflowPath str
10099
}
101100
} else {
102101
// Only .md exists - needs to be compiled first
103-
compileErr := fmt.Errorf("dispatch-workflow: workflow '%s' must be compiled first (run 'gh aw compile %s')", workflowName, fileResult.mdPath)
102+
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)
104103
if returnErr := collector.Add(compileErr); returnErr != nil {
105104
return returnErr // Fail-fast mode
106105
}

‎pkg/workflow/dispatch_workflow_validation_test.go‎

Lines changed: 319 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,319 @@
1+
//go:build !integration
2+
3+
package workflow
4+
5+
import (
6+
"os"
7+
"path/filepath"
8+
"testing"
9+
10+
"github.com/stretchr/testify/assert"
11+
"github.com/stretchr/testify/require"
12+
)
13+
14+
// TestDispatchWorkflowErrorMessage_EmptyList tests that empty workflow list
15+
// error message includes example configuration
16+
// Note: This test directly creates WorkflowData to bypass JSON schema validation
17+
// which also rejects empty arrays. The runtime validation provides a better error message.
18+
func TestDispatchWorkflowErrorMessage_EmptyList(t *testing.T) {
19+
compiler := NewCompilerWithVersion("1.0.0")
20+
21+
tmpDir := t.TempDir()
22+
awDir := filepath.Join(tmpDir, ".github", "aw")
23+
24+
err := os.MkdirAll(awDir, 0755)
25+
require.NoError(t, err, "Failed to create aw directory")
26+
27+
dispatcherFile := filepath.Join(awDir, "dispatcher.md")
28+
29+
// Create WorkflowData directly to bypass JSON schema validation
30+
// (which also rejects empty arrays with minItems: 1)
31+
workflowData := &WorkflowData{
32+
SafeOutputs: &SafeOutputsConfig{
33+
DispatchWorkflow: &DispatchWorkflowConfig{
34+
BaseSafeOutputConfig: BaseSafeOutputConfig{
35+
Max: 1,
36+
},
37+
Workflows: []string{}, // Empty list
38+
},
39+
},
40+
}
41+
42+
// Validate the workflow - should fail with enhanced error message
43+
err = compiler.validateDispatchWorkflow(workflowData, dispatcherFile)
44+
require.Error(t, err, "Validation should fail for empty workflows list")
45+
46+
// Verify enhanced error message content
47+
errMsg := err.Error()
48+
assert.Contains(t, errMsg, "must specify at least one workflow", "Should mention the requirement")
49+
assert.Contains(t, errMsg, "Example configuration", "Should include example header")
50+
assert.Contains(t, errMsg, "safe-outputs:", "Should show YAML structure")
51+
assert.Contains(t, errMsg, "dispatch-workflow:", "Should show feature name")
52+
assert.Contains(t, errMsg, "workflows: [workflow-name-1, workflow-name-2]", "Should show example list")
53+
assert.Contains(t, errMsg, "without the .md extension", "Should explain naming convention")
54+
}
55+
56+
// TestDispatchWorkflowErrorMessage_NotFound tests that workflow not found
57+
// error message includes troubleshooting steps
58+
func TestDispatchWorkflowErrorMessage_NotFound(t *testing.T) {
59+
compiler := NewCompilerWithVersion("1.0.0")
60+
61+
tmpDir := t.TempDir()
62+
awDir := filepath.Join(tmpDir, ".github", "aw")
63+
workflowsDir := filepath.Join(tmpDir, ".github", "workflows")
64+
65+
err := os.MkdirAll(awDir, 0755)
66+
require.NoError(t, err, "Failed to create aw directory")
67+
err = os.MkdirAll(workflowsDir, 0755)
68+
require.NoError(t, err, "Failed to create workflows directory")
69+
70+
// Create a dispatcher workflow that references a non-existent workflow
71+
dispatcherWorkflow := `---
72+
on: issues
73+
engine: copilot
74+
permissions:
75+
contents: read
76+
safe-outputs:
77+
dispatch-workflow:
78+
workflows:
79+
- missing-workflow
80+
max: 1
81+
---
82+
83+
# Dispatcher Workflow
84+
85+
This workflow references a non-existent workflow.
86+
`
87+
dispatcherFile := filepath.Join(awDir, "dispatcher.md")
88+
err = os.WriteFile(dispatcherFile, []byte(dispatcherWorkflow), 0644)
89+
require.NoError(t, err, "Failed to write dispatcher workflow")
90+
91+
// Change to the aw directory
92+
oldDir, err := os.Getwd()
93+
require.NoError(t, err, "Failed to get current directory")
94+
err = os.Chdir(awDir)
95+
require.NoError(t, err, "Failed to change directory")
96+
defer func() { _ = os.Chdir(oldDir) }()
97+
98+
// Parse the dispatcher workflow
99+
workflowData, err := compiler.ParseWorkflowFile("dispatcher.md")
100+
require.NoError(t, err, "Failed to parse workflow")
101+
102+
// Validate the workflow - should fail with enhanced error message
103+
err = compiler.validateDispatchWorkflow(workflowData, dispatcherFile)
104+
require.Error(t, err, "Validation should fail for missing workflow")
105+
106+
// Verify enhanced error message content
107+
errMsg := err.Error()
108+
assert.Contains(t, errMsg, "workflow 'missing-workflow' not found", "Should mention workflow name")
109+
assert.Contains(t, errMsg, "Checked for:", "Should list checked extensions")
110+
assert.Contains(t, errMsg, "missing-workflow.md", "Should mention .md extension")
111+
assert.Contains(t, errMsg, "missing-workflow.lock.yml", "Should mention .lock.yml extension")
112+
assert.Contains(t, errMsg, "missing-workflow.yml", "Should mention .yml extension")
113+
assert.Contains(t, errMsg, "To fix:", "Should include fix instructions header")
114+
assert.Contains(t, errMsg, "Verify the workflow file exists", "Should include verification step")
115+
assert.Contains(t, errMsg, "case-sensitive", "Should warn about case sensitivity")
116+
assert.Contains(t, errMsg, "without extension", "Should explain naming convention")
117+
}
118+
119+
// TestDispatchWorkflowErrorMessage_SelfReference tests that self-reference
120+
// error message includes explanation and alternatives
121+
func TestDispatchWorkflowErrorMessage_SelfReference(t *testing.T) {
122+
compiler := NewCompilerWithVersion("1.0.0")
123+
124+
tmpDir := t.TempDir()
125+
awDir := filepath.Join(tmpDir, ".github", "aw")
126+
workflowsDir := filepath.Join(tmpDir, ".github", "workflows")
127+
128+
err := os.MkdirAll(awDir, 0755)
129+
require.NoError(t, err, "Failed to create aw directory")
130+
err = os.MkdirAll(workflowsDir, 0755)
131+
require.NoError(t, err, "Failed to create workflows directory")
132+
133+
// Create a dispatcher workflow that references itself
134+
dispatcherWorkflow := `---
135+
on: issues
136+
engine: copilot
137+
permissions:
138+
contents: read
139+
safe-outputs:
140+
dispatch-workflow:
141+
workflows:
142+
- dispatcher
143+
max: 1
144+
---
145+
146+
# Dispatcher Workflow
147+
148+
This workflow tries to dispatch to itself.
149+
`
150+
dispatcherFile := filepath.Join(awDir, "dispatcher.md")
151+
err = os.WriteFile(dispatcherFile, []byte(dispatcherWorkflow), 0644)
152+
require.NoError(t, err, "Failed to write dispatcher workflow")
153+
154+
// Change to the aw directory
155+
oldDir, err := os.Getwd()
156+
require.NoError(t, err, "Failed to get current directory")
157+
err = os.Chdir(awDir)
158+
require.NoError(t, err, "Failed to change directory")
159+
defer func() { _ = os.Chdir(oldDir) }()
160+
161+
// Parse the dispatcher workflow
162+
workflowData, err := compiler.ParseWorkflowFile("dispatcher.md")
163+
require.NoError(t, err, "Failed to parse workflow")
164+
165+
// Validate the workflow - should fail with enhanced error message
166+
err = compiler.validateDispatchWorkflow(workflowData, dispatcherFile)
167+
require.Error(t, err, "Validation should fail for self-reference")
168+
169+
// Verify enhanced error message content
170+
errMsg := err.Error()
171+
assert.Contains(t, errMsg, "self-reference not allowed", "Should state the restriction")
172+
assert.Contains(t, errMsg, "dispatcher", "Should mention workflow name")
173+
assert.Contains(t, errMsg, "cannot dispatch itself", "Should explain the issue")
174+
assert.Contains(t, errMsg, "infinite loops", "Should explain why it's prevented")
175+
assert.Contains(t, errMsg, "schedule trigger", "Should suggest schedule alternative")
176+
assert.Contains(t, errMsg, "workflow_dispatch", "Should suggest workflow_dispatch alternative")
177+
}
178+
179+
// TestDispatchWorkflowErrorMessage_MustCompile tests that uncompiled workflow
180+
// error message includes compilation instructions
181+
func TestDispatchWorkflowErrorMessage_MustCompile(t *testing.T) {
182+
compiler := NewCompilerWithVersion("1.0.0")
183+
184+
tmpDir := t.TempDir()
185+
awDir := filepath.Join(tmpDir, ".github", "aw")
186+
workflowsDir := filepath.Join(tmpDir, ".github", "workflows")
187+
188+
err := os.MkdirAll(awDir, 0755)
189+
require.NoError(t, err, "Failed to create aw directory")
190+
err = os.MkdirAll(workflowsDir, 0755)
191+
require.NoError(t, err, "Failed to create workflows directory")
192+
193+
// Create an uncompiled workflow (only .md file, no .lock.yml)
194+
targetWorkflow := `---
195+
on: workflow_dispatch
196+
engine: copilot
197+
permissions:
198+
contents: read
199+
---
200+
201+
# Target Workflow
202+
203+
This workflow needs to be compiled.
204+
`
205+
targetFile := filepath.Join(workflowsDir, "target.md")
206+
err = os.WriteFile(targetFile, []byte(targetWorkflow), 0644)
207+
require.NoError(t, err, "Failed to write target workflow")
208+
209+
// Create a dispatcher workflow that references the uncompiled workflow
210+
dispatcherWorkflow := `---
211+
on: issues
212+
engine: copilot
213+
permissions:
214+
contents: read
215+
safe-outputs:
216+
dispatch-workflow:
217+
workflows:
218+
- target
219+
max: 1
220+
---
221+
222+
# Dispatcher Workflow
223+
224+
This workflow references an uncompiled workflow.
225+
`
226+
dispatcherFile := filepath.Join(awDir, "dispatcher.md")
227+
err = os.WriteFile(dispatcherFile, []byte(dispatcherWorkflow), 0644)
228+
require.NoError(t, err, "Failed to write dispatcher workflow")
229+
230+
// Change to the aw directory
231+
oldDir, err := os.Getwd()
232+
require.NoError(t, err, "Failed to get current directory")
233+
err = os.Chdir(awDir)
234+
require.NoError(t, err, "Failed to change directory")
235+
defer func() { _ = os.Chdir(oldDir) }()
236+
237+
// Parse the dispatcher workflow
238+
workflowData, err := compiler.ParseWorkflowFile("dispatcher.md")
239+
require.NoError(t, err, "Failed to parse workflow")
240+
241+
// Validate the workflow - should fail with enhanced error message
242+
err = compiler.validateDispatchWorkflow(workflowData, dispatcherFile)
243+
require.Error(t, err, "Validation should fail for uncompiled workflow")
244+
245+
// Verify enhanced error message content
246+
errMsg := err.Error()
247+
assert.Contains(t, errMsg, "must be compiled first", "Should state the requirement")
248+
assert.Contains(t, errMsg, "target", "Should mention workflow name")
249+
assert.Contains(t, errMsg, "source file exists", "Should acknowledge file exists")
250+
assert.Contains(t, errMsg, "compiled .lock.yml file is missing", "Should explain what's missing")
251+
assert.Contains(t, errMsg, "To fix:", "Should include fix instructions header")
252+
assert.Contains(t, errMsg, "gh aw compile target", "Should show exact compilation command")
253+
assert.Contains(t, errMsg, "Commit the generated .lock.yml", "Should mention committing")
254+
assert.Contains(t, errMsg, ".gitignore", "Should warn about gitignore")
255+
}
256+
257+
// TestDispatchWorkflowErrorMessage_MultipleErrors tests that multiple errors
258+
// with enhanced messages are aggregated correctly
259+
func TestDispatchWorkflowErrorMessage_MultipleErrors(t *testing.T) {
260+
compiler := NewCompilerWithVersion("1.0.0")
261+
compiler.failFast = false // Enable error aggregation
262+
263+
tmpDir := t.TempDir()
264+
awDir := filepath.Join(tmpDir, ".github", "aw")
265+
workflowsDir := filepath.Join(tmpDir, ".github", "workflows")
266+
267+
err := os.MkdirAll(awDir, 0755)
268+
require.NoError(t, err, "Failed to create aw directory")
269+
err = os.MkdirAll(workflowsDir, 0755)
270+
require.NoError(t, err, "Failed to create workflows directory")
271+
272+
// Create dispatcher workflow with multiple errors
273+
dispatcherWorkflow := `---
274+
on: issues
275+
engine: copilot
276+
permissions:
277+
contents: read
278+
safe-outputs:
279+
dispatch-workflow:
280+
workflows:
281+
- dispatcher # Self-reference
282+
- missing # Not found
283+
max: 2
284+
---
285+
286+
# Dispatcher Workflow
287+
`
288+
dispatcherFile := filepath.Join(awDir, "dispatcher.md")
289+
err = os.WriteFile(dispatcherFile, []byte(dispatcherWorkflow), 0644)
290+
require.NoError(t, err, "Failed to write dispatcher workflow")
291+
292+
// Change to the aw directory
293+
oldDir, err := os.Getwd()
294+
require.NoError(t, err, "Failed to get current directory")
295+
err = os.Chdir(awDir)
296+
require.NoError(t, err, "Failed to change directory")
297+
defer func() { _ = os.Chdir(oldDir) }()
298+
299+
// Parse the dispatcher workflow
300+
workflowData, err := compiler.ParseWorkflowFile("dispatcher.md")
301+
require.NoError(t, err, "Failed to parse workflow")
302+
303+
// Validate the workflow - should fail with multiple enhanced error messages
304+
err = compiler.validateDispatchWorkflow(workflowData, dispatcherFile)
305+
require.Error(t, err, "Validation should fail with multiple errors")
306+
307+
// Verify both enhanced error messages are in the aggregated error
308+
errMsg := err.Error()
309+
assert.Contains(t, errMsg, "Found 2 dispatch-workflow errors:", "Should show error count")
310+
311+
// Check self-reference error with enhancements
312+
assert.Contains(t, errMsg, "self-reference not allowed", "Should include self-reference error")
313+
assert.Contains(t, errMsg, "infinite loops", "Should include explanation for self-reference")
314+
315+
// Check not found error with enhancements
316+
assert.Contains(t, errMsg, "not found", "Should include not found error")
317+
assert.Contains(t, errMsg, "To fix:", "Should include fix instructions")
318+
assert.Contains(t, errMsg, "Checked for:", "Should include checked extensions")
319+
}

0 commit comments

Comments
 (0)