Skip to content

Commit 06bddac

Browse files
CopilotCopilot
authored
Add comprehensive trigger shorthand syntax parser with fuzz testing and IDE-integrated error messages (#7160)
1 parent 5f5d14c commit 06bddac

8 files changed

Lines changed: 1984 additions & 19 deletions

‎pkg/workflow/compiler_parse.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ func (c *Compiler) ParseWorkflowFile(markdownPath string) (*WorkflowData, error)
4646
}
4747

4848
// Preprocess schedule fields to convert human-friendly format to cron expressions
49-
if err := c.preprocessScheduleFields(result.Frontmatter); err != nil {
49+
if err := c.preprocessScheduleFields(result.Frontmatter, markdownPath, string(content)); err != nil {
5050
return nil, err
5151
}
5252

‎pkg/workflow/label_trigger_integration_test.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ func TestLabelTriggerIntegrationSimple(t *testing.T) {
1111
}
1212

1313
compiler := NewCompiler(false, "", "test")
14-
err := compiler.preprocessScheduleFields(frontmatter)
14+
err := compiler.preprocessScheduleFields(frontmatter, "", "")
1515
if err != nil {
1616
t.Fatalf("preprocessScheduleFields() error = %v", err)
1717
}
@@ -90,7 +90,7 @@ func TestLabelTriggerIntegrationIssue(t *testing.T) {
9090
}
9191

9292
compiler := NewCompiler(false, "", "test")
93-
err := compiler.preprocessScheduleFields(frontmatter)
93+
err := compiler.preprocessScheduleFields(frontmatter, "", "")
9494
if err != nil {
9595
t.Fatalf("preprocessScheduleFields() error = %v", err)
9696
}
@@ -120,7 +120,7 @@ func TestLabelTriggerIntegrationPullRequest(t *testing.T) {
120120
}
121121

122122
compiler := NewCompiler(false, "", "test")
123-
err := compiler.preprocessScheduleFields(frontmatter)
123+
err := compiler.preprocessScheduleFields(frontmatter, "", "")
124124
if err != nil {
125125
t.Fatalf("preprocessScheduleFields() error = %v", err)
126126
}
@@ -186,7 +186,7 @@ func TestLabelTriggerIntegrationDiscussion(t *testing.T) {
186186
}
187187

188188
compiler := NewCompiler(false, "", "test")
189-
err := compiler.preprocessScheduleFields(frontmatter)
189+
err := compiler.preprocessScheduleFields(frontmatter, "", "")
190190
if err != nil {
191191
t.Fatalf("preprocessScheduleFields() error = %v", err)
192192
}
@@ -249,7 +249,7 @@ func TestLabelTriggerIntegrationError(t *testing.T) {
249249
}
250250

251251
compiler := NewCompiler(false, "", "test")
252-
err := compiler.preprocessScheduleFields(frontmatter)
252+
err := compiler.preprocessScheduleFields(frontmatter, "", "")
253253
if err != nil {
254254
t.Fatalf("preprocessScheduleFields() unexpected error = %v", err)
255255
}

‎pkg/workflow/schedule_preprocessing.go‎

Lines changed: 95 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
package workflow
22

33
import (
4+
"errors"
45
"fmt"
56
"strings"
67

8+
"github.com/githubnext/gh-aw/pkg/console"
79
"github.com/githubnext/gh-aw/pkg/logger"
810
"github.com/githubnext/gh-aw/pkg/parser"
911
)
@@ -16,7 +18,7 @@ var scheduleFriendlyFormats = make(map[string]map[int]string)
1618

1719
// preprocessScheduleFields converts human-friendly schedule expressions to cron expressions
1820
// in the frontmatter's "on" section. It modifies the frontmatter map in place.
19-
func (c *Compiler) preprocessScheduleFields(frontmatter map[string]any) error {
21+
func (c *Compiler) preprocessScheduleFields(frontmatter map[string]any, markdownPath string, content string) error {
2022
schedulePreprocessingLog.Print("Preprocessing schedule fields in frontmatter")
2123

2224
// Check if "on" field exists
@@ -25,7 +27,7 @@ func (c *Compiler) preprocessScheduleFields(frontmatter map[string]any) error {
2527
return nil
2628
}
2729

28-
// Check if "on" is a string - might be a schedule expression, slash command shorthand, or label trigger shorthand
30+
// Check if "on" is a string - might be a schedule expression, slash command shorthand, label trigger shorthand, or other trigger shorthand
2931
if onStr, ok := onValue.(string); ok {
3032
schedulePreprocessingLog.Printf("Processing on field as string: %s", onStr)
3133

@@ -59,11 +61,28 @@ func (c *Compiler) preprocessScheduleFields(frontmatter map[string]any) error {
5961
return nil
6062
}
6163

62-
// Try to parse as a schedule expression
64+
// Try the new unified trigger parser for other trigger shorthands
65+
triggerIR, err := ParseTriggerShorthand(onStr)
66+
if err != nil {
67+
// Wrap the error with source location information
68+
return c.createTriggerParseError(markdownPath, content, onStr, err)
69+
}
70+
if triggerIR != nil {
71+
schedulePreprocessingLog.Printf("Converting shorthand 'on: %s' to structured trigger", onStr)
72+
73+
// Convert IR to YAML map
74+
onMap := triggerIR.ToYAMLMap()
75+
frontmatter["on"] = onMap
76+
77+
return nil
78+
}
79+
80+
// Try to parse as a schedule expression (only if not already recognized as another trigger type)
6381
parsedCron, original, err := parser.ParseSchedule(onStr)
6482
if err != nil {
65-
// Not a schedule expression, treat as a simple event trigger
66-
schedulePreprocessingLog.Printf("Not a schedule expression: %s", onStr)
83+
// Not a schedule expression either - leave as simple string trigger
84+
// (simple event names like "push", "fork", etc. are valid)
85+
schedulePreprocessingLog.Printf("Not a recognized shorthand or schedule: %s - leaving as-is", onStr)
6786
return nil
6887
}
6988

@@ -323,6 +342,77 @@ func (c *Compiler) preprocessScheduleFields(frontmatter map[string]any) error {
323342
return nil
324343
}
325344

345+
// createTriggerParseError creates a detailed error for trigger parsing issues with source location
346+
func (c *Compiler) createTriggerParseError(filePath, content, triggerStr string, err error) error {
347+
schedulePreprocessingLog.Printf("Creating trigger parse error for: %s", triggerStr)
348+
349+
lines := strings.Split(content, "\n")
350+
351+
// Find the line where "on:" appears in the frontmatter
352+
var onLine int
353+
var onColumn int
354+
inFrontmatter := false
355+
356+
for i, line := range lines {
357+
lineNum := i + 1
358+
359+
// Check for frontmatter delimiter
360+
if strings.TrimSpace(line) == "---" {
361+
if !inFrontmatter {
362+
inFrontmatter = true
363+
} else {
364+
// End of frontmatter
365+
break
366+
}
367+
continue
368+
}
369+
370+
if inFrontmatter {
371+
// Look for "on:" field
372+
trimmed := strings.TrimSpace(line)
373+
if strings.HasPrefix(trimmed, "on:") {
374+
onLine = lineNum
375+
// Find the column where "on:" starts
376+
onColumn = strings.Index(line, "on:") + 1
377+
break
378+
}
379+
}
380+
}
381+
382+
// If we found the line, create a formatted error
383+
if onLine > 0 {
384+
// Create context lines around the error
385+
var context []string
386+
startLine := max(1, onLine-2)
387+
endLine := min(len(lines), onLine+2)
388+
389+
for i := startLine; i <= endLine; i++ {
390+
if i-1 < len(lines) {
391+
context = append(context, lines[i-1])
392+
}
393+
}
394+
395+
compilerErr := console.CompilerError{
396+
Position: console.ErrorPosition{
397+
File: filePath,
398+
Line: onLine,
399+
Column: onColumn,
400+
},
401+
Type: "error",
402+
Message: fmt.Sprintf("trigger syntax error: %s", err.Error()),
403+
Context: context,
404+
}
405+
406+
// Format and return the error
407+
formattedErr := console.FormatError(compilerErr)
408+
return errors.New(formattedErr)
409+
}
410+
411+
// Fallback to original error if we can't find the line
412+
schedulePreprocessingLog.Printf("Could not find 'on:' line in frontmatter, using fallback error")
413+
return fmt.Errorf("trigger syntax error: %w", err)
414+
}
415+
326416
// addFriendlyScheduleComments adds comments showing the original friendly format for schedule cron expressions
327417
// This function is called after the YAML has been generated from the frontmatter
328418
func (c *Compiler) addFriendlyScheduleComments(yamlStr string, frontmatter map[string]any) string {

‎pkg/workflow/schedule_preprocessing_test.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ func TestSchedulePreprocessingShorthandOnString(t *testing.T) {
8989
// (required for all schedule tests to avoid fuzzy schedule errors)
9090
compiler.SetWorkflowIdentifier("test-workflow.md")
9191

92-
err := compiler.preprocessScheduleFields(tt.frontmatter)
92+
err := compiler.preprocessScheduleFields(tt.frontmatter, "", "")
9393

9494
if tt.expectedError {
9595
if err == nil {
@@ -332,7 +332,7 @@ func TestSchedulePreprocessing(t *testing.T) {
332332
for _, tt := range tests {
333333
t.Run(tt.name, func(t *testing.T) {
334334
compiler := NewCompiler(false, "", "test")
335-
err := compiler.preprocessScheduleFields(tt.frontmatter)
335+
err := compiler.preprocessScheduleFields(tt.frontmatter, "", "")
336336

337337
if tt.expectedError {
338338
if err == nil {
@@ -378,7 +378,7 @@ func TestScheduleFriendlyComments(t *testing.T) {
378378
compiler := NewCompiler(false, "", "test")
379379

380380
// Preprocess to convert and store friendly formats
381-
err := compiler.preprocessScheduleFields(frontmatter)
381+
err := compiler.preprocessScheduleFields(frontmatter, "", "")
382382
if err != nil {
383383
t.Fatalf("preprocessing failed: %v", err)
384384
}
@@ -452,7 +452,7 @@ func TestFuzzyScheduleScattering(t *testing.T) {
452452
compiler.SetWorkflowIdentifier(tt.workflowIdentifier)
453453
}
454454

455-
err := compiler.preprocessScheduleFields(tt.frontmatter)
455+
err := compiler.preprocessScheduleFields(tt.frontmatter, "", "")
456456

457457
if tt.expectError {
458458
if err == nil {
@@ -510,7 +510,7 @@ func TestFuzzyScheduleScatteringDeterministic(t *testing.T) {
510510
compiler := NewCompiler(false, "", "test")
511511
compiler.SetWorkflowIdentifier(wf)
512512

513-
err := compiler.preprocessScheduleFields(frontmatter)
513+
err := compiler.preprocessScheduleFields(frontmatter, "", "")
514514
if err != nil {
515515
t.Fatalf("unexpected error for workflow %s: %v", wf, err)
516516
}
@@ -604,7 +604,7 @@ func TestSchedulePreprocessingWithFuzzyDaily(t *testing.T) {
604604
compiler := NewCompiler(false, "", "test")
605605
compiler.SetWorkflowIdentifier("test-workflow.md")
606606

607-
err := compiler.preprocessScheduleFields(tt.frontmatter)
607+
err := compiler.preprocessScheduleFields(tt.frontmatter, "", "")
608608

609609
if tt.expectError {
610610
if err == nil {
@@ -669,7 +669,7 @@ func TestSchedulePreprocessingDailyVariations(t *testing.T) {
669669
},
670670
}
671671

672-
err := compiler.preprocessScheduleFields(frontmatter)
672+
err := compiler.preprocessScheduleFields(frontmatter, "", "")
673673
if err != nil {
674674
t.Fatalf("unexpected error: %v", err)
675675
}
@@ -760,7 +760,7 @@ func TestSlashCommandShorthand(t *testing.T) {
760760
compiler := NewCompiler(false, "", "test")
761761
compiler.SetWorkflowIdentifier("test-workflow.md")
762762

763-
err := compiler.preprocessScheduleFields(tt.frontmatter)
763+
err := compiler.preprocessScheduleFields(tt.frontmatter, "", "")
764764

765765
if tt.expectedError {
766766
if err == nil {

0 commit comments

Comments
 (0)