refactor parser and schedule helpers to reduce semantic drift

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Author: Copilot

pkg/parser/github.go

@@ -10,7 +10,6 @@ import (
 	"strings"
 
 	"github.com/github/gh-aw/pkg/constants"
-	"github.com/github/gh-aw/pkg/envutil"
 	"github.com/github/gh-aw/pkg/logger"
 	"github.com/github/gh-aw/pkg/stringutil"
 )
@@ -63,10 +62,7 @@ func GetGitHubToken() (string, error) {
 	githubLog.Print("Getting GitHub token")
 
 	// First try environment variable
-	if token := envutil.GetStringFromEnv("GITHUB_TOKEN", "", githubLog); token != "" {
-		return token, nil
-	}
-	if token := envutil.GetStringFromEnv("GH_TOKEN", "", githubLog); token != "" {
+	if token := githubTokenFromEnv(githubLog); token != "" {
 		return token, nil
 	}
 

pkg/parser/github_token_env.go

@@ -0,0 +1,13 @@
+package parser
+
+import (
+	"github.com/github/gh-aw/pkg/envutil"
+	"github.com/github/gh-aw/pkg/logger"
+)
+
+func githubTokenFromEnv(log *logger.Logger) string {
+	if token := envutil.GetStringFromEnv("GITHUB_TOKEN", "", log); token != "" {
+		return token
+	}
+	return envutil.GetStringFromEnv("GH_TOKEN", "", log)
+}

pkg/parser/github_token_env_test.go

@@ -0,0 +1,33 @@
+//go:build !integration
+
+package parser
+
+import (
+	"testing"
+
+	"github.com/github/gh-aw/pkg/logger"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGithubTokenFromEnv(t *testing.T) {
+	t.Run("prefers GITHUB_TOKEN over GH_TOKEN", func(t *testing.T) {
+		t.Setenv("GITHUB_TOKEN", "github-token")
+		t.Setenv("GH_TOKEN", "gh-token")
+
+		assert.Equal(t, "github-token", githubTokenFromEnv(logger.New("parser:test")))
+	})
+
+	t.Run("falls back to GH_TOKEN when GITHUB_TOKEN is empty", func(t *testing.T) {
+		t.Setenv("GITHUB_TOKEN", "")
+		t.Setenv("GH_TOKEN", "gh-token")
+
+		assert.Equal(t, "gh-token", githubTokenFromEnv(nil))
+	})
+
+	t.Run("returns empty when neither token is set", func(t *testing.T) {
+		t.Setenv("GITHUB_TOKEN", "")
+		t.Setenv("GH_TOKEN", "")
+
+		assert.Empty(t, githubTokenFromEnv(nil))
+	})
+}

pkg/parser/github_wasm.go

@@ -4,17 +4,12 @@ package parser
 
 import (
 	"errors"
-
-	"github.com/github/gh-aw/pkg/envutil"
 )
 
 func GetGitHubToken() (string, error) {
 	// Wasm callers do not use the package logger, so pass nil to suppress debug
 	// logging while still centralizing environment variable access.
-	if token := envutil.GetStringFromEnv("GITHUB_TOKEN", "", nil); token != "" {
-		return token, nil
-	}
-	if token := envutil.GetStringFromEnv("GH_TOKEN", "", nil); token != "" {
+	if token := githubTokenFromEnv(nil); token != "" {
 		return token, nil
 	}
 	return "", errors.New("GitHub token not available in Wasm (set GITHUB_TOKEN or GH_TOKEN environment variable)")

pkg/parser/schedule_cron_detection.go

@@ -16,11 +16,28 @@ var cronDetectionLog = logger.New("parser:schedule_cron_detection")
 // cronFieldPattern matches valid cron field syntax (pre-compiled for performance)
 var cronFieldPattern = regexp.MustCompile(`^[\d\*\-/,]+$`)
 
+func cronFields(cron string) ([]string, bool) {
+	fields := strings.Fields(cron)
+	return fields, len(fields) == 5
+}
+
+func isNumericCronField(field string) bool {
+	if field == "" {
+		return false
+	}
+	for _, ch := range field {
+		if ch < '0' || ch > '9' {
+			return false
+		}
+	}
+	return true
+}
+
 // IsDailyCron checks if a cron expression represents a daily schedule at a fixed time
 // (e.g., "0 0 * * *", "30 14 * * *", etc.)
 func IsDailyCron(cron string) bool {
-	fields := strings.Fields(cron)
-	if len(fields) != 5 {
+	fields, ok := cronFields(cron)
+	if !ok {
 		return false
 	}
 	// Daily pattern: minute hour * * *
@@ -32,16 +49,8 @@ func IsDailyCron(cron string) bool {
 	minute := fields[0]
 	hour := fields[1]
 
-	// Minute and hour should be digits only (no *, /, -, ,)
-	for _, ch := range minute {
-		if ch < '0' || ch > '9' {
-			return false
-		}
-	}
-	for _, ch := range hour {
-		if ch < '0' || ch > '9' {
-			return false
-		}
+	if !isNumericCronField(minute) || !isNumericCronField(hour) {
+		return false
 	}
 
 	result := fields[2] == "*" && fields[3] == "*" && fields[4] == "*"
@@ -54,8 +63,8 @@ func IsDailyCron(cron string) bool {
 // IsHourlyCron checks if a cron expression represents an hourly interval with a fixed minute
 // (e.g., "0 */1 * * *", "30 */2 * * *", etc.)
 func IsHourlyCron(cron string) bool {
-	fields := strings.Fields(cron)
-	if len(fields) != 5 {
+	fields, ok := cronFields(cron)
+	if !ok {
 		return false
 	}
 	// Hourly pattern: minute */N * * * or minute *N * * *
@@ -65,11 +74,8 @@ func IsHourlyCron(cron string) bool {
 	minute := fields[0]
 	hour := fields[1]
 
-	// Minute should be digits only (no *, /, -, ,)
-	for _, ch := range minute {
-		if ch < '0' || ch > '9' {
-			return false
-		}
+	if !isNumericCronField(minute) {
+		return false
 	}
 
 	// Hour should be an interval pattern like */N
@@ -88,8 +94,8 @@ func IsHourlyCron(cron string) bool {
 // IsWeeklyCron checks if a cron expression represents a weekly schedule at a fixed time
 // (e.g., "0 0 * * 1", "30 14 * * 5", etc.)
 func IsWeeklyCron(cron string) bool {
-	fields := strings.Fields(cron)
-	if len(fields) != 5 {
+	fields, ok := cronFields(cron)
+	if !ok {
 		return false
 	}
 	// Weekly pattern: minute hour * * DOW
@@ -101,16 +107,8 @@ func IsWeeklyCron(cron string) bool {
 	minute := fields[0]
 	hour := fields[1]
 
-	// Minute and hour should be digits only (no *, /, -, ,)
-	for _, ch := range minute {
-		if ch < '0' || ch > '9' {
-			return false
-		}
-	}
-	for _, ch := range hour {
-		if ch < '0' || ch > '9' {
-			return false
-		}
+	if !isNumericCronField(minute) || !isNumericCronField(hour) {
+		return false
 	}
 
 	// Check day-of-month and month are wildcards

pkg/parser/schedule_cron_detection_test.go

@@ -28,7 +28,6 @@ func TestIsCronExpression(t *testing.T) {
 		{"invalid expression", "invalid cron expression", false},
 		{"empty string", "", false},
 	}
-
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			result := IsCronExpression(tt.input)
@@ -37,6 +36,28 @@ func TestIsCronExpression(t *testing.T) {
 	}
 }
 
+func TestIsNumericCronField(t *testing.T) {
+	tests := []struct {
+		name     string
+		input    string
+		expected bool
+	}{
+		{"single digit", "5", true},
+		{"multiple digits", "15", true},
+		{"empty", "", false},
+		{"wildcard", "*", false},
+		{"interval", "*/5", false},
+		{"range", "1-5", false},
+		{"list", "1,2", false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equal(t, tt.expected, isNumericCronField(tt.input))
+		})
+	}
+}
+
 func TestIsDailyCron(t *testing.T) {
 	tests := []struct {
 		name     string

pkg/workflow/schedule_preprocessing.go

@@ -472,12 +472,7 @@ func (c *Compiler) addDailyCronWarning(cronExpr string) {
 			hour, minute,
 		)
 
-		// This warning is added to the warning count
-		// It will be collected and displayed by the compilation process
-		c.IncrementWarningCount()
-
-		// Store the warning for later display
-		c.addScheduleWarning(warningMsg)
+		c.emitScheduleWarning(warningMsg)
 	}
 }
 
@@ -499,11 +494,7 @@ func (c *Compiler) addHourlyCronWarning(cronExpr string) {
 			minute, interval,
 		)
 
-		// This warning is added to the warning count
-		c.IncrementWarningCount()
-
-		// Store the warning for later display
-		c.addScheduleWarning(warningMsg)
+		c.emitScheduleWarning(warningMsg)
 	}
 }
 
@@ -538,14 +529,19 @@ func (c *Compiler) addWeeklyCronWarning(cronExpr string) {
 			weekdayName, hour, minute, strings.ToLower(weekdayName),
 		)
 
-		// This warning is added to the warning count
-		c.IncrementWarningCount()
-
-		// Store the warning for later display
-		c.addScheduleWarning(warningMsg)
+		c.emitScheduleWarning(warningMsg)
 	}
 }
 
+func (c *Compiler) emitScheduleWarning(warning string) {
+	// This warning is added to the warning count.
+	// It will be collected and displayed by the compilation process.
+	c.IncrementWarningCount()
+
+	// Store the warning for later display.
+	c.addScheduleWarning(warning)
+}
+
 // addScheduleWarning adds a warning to the compiler's schedule warnings list
 func (c *Compiler) addScheduleWarning(warning string) {
 	if c.scheduleWarnings == nil {