diff --git a/pkg/workflow/awf_config.go b/pkg/workflow/awf_config.go index fd16598e20c..8678f5f5368 100644 --- a/pkg/workflow/awf_config.go +++ b/pkg/workflow/awf_config.go @@ -488,7 +488,7 @@ func BuildAWFConfigJSON(config AWFCommandConfig) (string, error) { // ── Runner section ────────────────────────────────────────────────────── if topology := getRunnerTopology(config.WorkflowData); topology != "" { - awfConfig.Runner = &AWFRunnerConfig{Topology: topology} + awfConfig.Runner = &AWFRunnerConfig{Topology: string(topology)} awfConfigLog.Printf("Runner section: topology=%s", topology) } @@ -1042,9 +1042,9 @@ func extractBoundedQueriesConfig(workflowData *WorkflowData) *AWFBoundedQueriesC return awfBQ } -// getRunnerTopology extracts the runner topology string from WorkflowData. +// getRunnerTopology extracts the runner topology from WorkflowData. // Returns an empty string when no topology is configured. -func getRunnerTopology(workflowData *WorkflowData) string { +func getRunnerTopology(workflowData *WorkflowData) RunnerTopology { if workflowData == nil || workflowData.RunnerConfig == nil { return "" } diff --git a/pkg/workflow/central_slash_command_workflow.go b/pkg/workflow/central_slash_command_workflow.go index b603d53f05f..9d750163262 100644 --- a/pkg/workflow/central_slash_command_workflow.go +++ b/pkg/workflow/central_slash_command_workflow.go @@ -328,15 +328,15 @@ func resolveCentralizedEventReaction(wd *WorkflowData, eventName string) string switch eventName { case "issues", "issue_comment": if shouldIncludeIssueReactions(wd) { - return wd.AIReaction + return string(wd.AIReaction) } case "pull_request", "pull_request_comment", "pull_request_review_comment": if shouldIncludePullRequestReactions(wd) { - return wd.AIReaction + return string(wd.AIReaction) } case "discussion", "discussion_comment": if shouldIncludeDiscussionReactions(wd) { - return wd.AIReaction + return string(wd.AIReaction) } } diff --git a/pkg/workflow/compiler_activation_jobs_test.go b/pkg/workflow/compiler_activation_jobs_test.go index e7d50ccf517..ea13e58c9ce 100644 --- a/pkg/workflow/compiler_activation_jobs_test.go +++ b/pkg/workflow/compiler_activation_jobs_test.go @@ -119,7 +119,7 @@ func TestBuildPreActivationJob_WithReaction(t *testing.T) { workflowData := &WorkflowData{ Name: "Test Workflow", Command: []string{"test"}, - AIReaction: tt.reaction, + AIReaction: ReactionType(tt.reaction), } // Pre-activation job should NOT contain the reaction step any more diff --git a/pkg/workflow/compiler_reactions_numeric_test.go b/pkg/workflow/compiler_reactions_numeric_test.go index 60976186e62..f1d5c838e82 100644 --- a/pkg/workflow/compiler_reactions_numeric_test.go +++ b/pkg/workflow/compiler_reactions_numeric_test.go @@ -70,8 +70,8 @@ Test workflow with invalid reaction value. func TestNumericReactionParsing(t *testing.T) { testCases := []struct { name string - reactionInYAML string // How it appears in YAML - expectedReaction string // Expected AIReaction value + reactionInYAML string // How it appears in YAML + expectedReaction ReactionType // Expected AIReaction value }{ { name: "plus one without quotes becomes +1", diff --git a/pkg/workflow/compiler_safe_outputs_test.go b/pkg/workflow/compiler_safe_outputs_test.go index 3010b0e334c..88655ffd93f 100644 --- a/pkg/workflow/compiler_safe_outputs_test.go +++ b/pkg/workflow/compiler_safe_outputs_test.go @@ -18,7 +18,7 @@ func TestParseOnSection(t *testing.T) { markdownPath string expectedError bool expectedCommand []string - expectedReaction string + expectedReaction ReactionType expectedLockAgent bool expectedOn string expectedCentralized bool @@ -870,7 +870,7 @@ func TestParseOnSectionWithParsedFrontmatter(t *testing.T) { tests := []struct { name string parsedFrontmatter *FrontmatterConfig - expectedReaction string + expectedReaction ReactionType expectedError bool }{ { @@ -1161,7 +1161,7 @@ func TestParseOnSectionReactionMapFormat(t *testing.T) { err := c.parseOnSection(frontmatter, workflowData, "/path/to/test.md") require.NoError(t, err, "reaction map format should be accepted") - assert.Equal(t, "heart", workflowData.AIReaction, "reaction type should be parsed from reaction.type") + assert.Equal(t, ReactionTypeHeart, workflowData.AIReaction, "reaction type should be parsed from reaction.type") require.NotNil(t, workflowData.ReactionIssues, "reaction issue target flag should be set") assert.True(t, *workflowData.ReactionIssues, "reaction issues target should default to true") require.NotNil(t, workflowData.ReactionPullRequests, "reaction pull request target flag should be set") diff --git a/pkg/workflow/frontmatter_types.go b/pkg/workflow/frontmatter_types.go index 5fbb778ee7f..e131a727c4a 100644 --- a/pkg/workflow/frontmatter_types.go +++ b/pkg/workflow/frontmatter_types.go @@ -6,8 +6,10 @@ import ( var frontmatterTypesLog = logger.New("workflow:frontmatter_types") +type RunnerTopology string + // RunnerTopologyArcDind is the topology value for ARC runners with Docker-in-Docker sidecars. -const RunnerTopologyArcDind = "arc-dind" +const RunnerTopologyArcDind RunnerTopology = "arc-dind" // RunnerConfig represents runner topology configuration from the workflow frontmatter. // The topology field is the single stable contract between gh-aw and AWF for runner @@ -16,7 +18,7 @@ const RunnerTopologyArcDind = "arc-dind" type RunnerConfig struct { // Topology identifies the runner execution topology. // Supported values: "arc-dind" (ARC with Docker-in-Docker sidecar). - Topology string `json:"topology,omitempty" yaml:"topology,omitempty"` + Topology RunnerTopology `json:"topology,omitempty" yaml:"topology,omitempty"` } // RuntimeConfig represents the configuration for a single runtime diff --git a/pkg/workflow/implicit_string_enums_test.go b/pkg/workflow/implicit_string_enums_test.go new file mode 100644 index 00000000000..bb8a0ee002e --- /dev/null +++ b/pkg/workflow/implicit_string_enums_test.go @@ -0,0 +1,29 @@ +//go:build !integration + +package workflow + +import ( + "reflect" + "testing" +) + +func TestImplicitStringEnumFieldTypes(t *testing.T) { + tests := []struct { + name string + value any + typeName string + }{ + {"safe outputs URLs", SafeOutputsConfig{}.URLs, "SafeOutputsURLsPolicy"}, + {"AI reaction", WorkflowData{}.AIReaction, "ReactionType"}, + {"MCP script parameter", MCPScriptParam{}.Type, "MCPParamType"}, + {"runner topology", RunnerConfig{}.Topology, "RunnerTopology"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if actual := reflect.TypeOf(tt.value).Name(); actual != tt.typeName { + t.Errorf("field type = %q, want %q", actual, tt.typeName) + } + }) + } +} diff --git a/pkg/workflow/label_command_test.go b/pkg/workflow/label_command_test.go index 419646da626..8232ad5946f 100644 --- a/pkg/workflow/label_command_test.go +++ b/pkg/workflow/label_command_test.go @@ -239,7 +239,7 @@ Deploy the application because label "deploy" was added. require.NoError(t, err, "ParseWorkflowFile() should not error") // Verify AIReaction defaults to "eyes" for label_command workflows - assert.Equal(t, "eyes", workflowData.AIReaction, "AIReaction should default to 'eyes' for label_command workflows") + assert.Equal(t, ReactionTypeEyes, workflowData.AIReaction, "AIReaction should default to 'eyes' for label_command workflows") // Verify StatusComment defaults to true for label_command workflows require.NotNil(t, workflowData.StatusComment, "StatusComment should not be nil for label_command workflows") diff --git a/pkg/workflow/mcp_scripts_parser.go b/pkg/workflow/mcp_scripts_parser.go index 306a1130b01..9285ea869ce 100644 --- a/pkg/workflow/mcp_scripts_parser.go +++ b/pkg/workflow/mcp_scripts_parser.go @@ -52,12 +52,22 @@ type MCPScriptToolConfig struct { Timeout int // Timeout in seconds for tool execution (default: 60) } +type MCPParamType string + +const ( + MCPParamTypeString MCPParamType = "string" + MCPParamTypeNumber MCPParamType = "number" + MCPParamTypeBoolean MCPParamType = "boolean" + MCPParamTypeArray MCPParamType = "array" + MCPParamTypeObject MCPParamType = "object" +) + // MCPScriptParam holds the configuration for a tool input parameter type MCPScriptParam struct { - Type string // JSON schema type (string, number, boolean, array, object) - Description string // Description of the parameter - Required bool // Whether the parameter is required - Default any // Default value + Type MCPParamType // JSON schema type (string, number, boolean, array, object) + Description string // Description of the parameter + Required bool // Whether the parameter is required + Default any // Default value } // MCPScriptsMode constants define the available transport modes @@ -102,12 +112,12 @@ func parseMCPScriptToolConfig(toolName string, toolMap map[string]any) *MCPScrip for paramName, paramValue := range inputsMap { if paramMap, ok := paramValue.(map[string]any); ok { param := &MCPScriptParam{ - Type: "string", // default type + Type: MCPParamTypeString, // default type } if t, exists := paramMap["type"]; exists { if tStr, ok := t.(string); ok { - param.Type = tStr + param.Type = MCPParamType(tStr) } } diff --git a/pkg/workflow/notify_comment_test.go b/pkg/workflow/notify_comment_test.go index 3efbfddb181..96776c35f54 100644 --- a/pkg/workflow/notify_comment_test.go +++ b/pkg/workflow/notify_comment_test.go @@ -158,7 +158,7 @@ func TestConclusionJob(t *testing.T) { compiler := NewCompiler() workflowData := &WorkflowData{ Name: "Test Workflow", - AIReaction: tt.aiReaction, + AIReaction: ReactionType(tt.aiReaction), Command: tt.command, } @@ -871,7 +871,7 @@ func TestStatusCommentDecoupling(t *testing.T) { // Test activation job workflowData := &WorkflowData{ Name: "Test Workflow", - AIReaction: tt.aiReaction, + AIReaction: ReactionType(tt.aiReaction), StatusComment: tt.statusComment, SafeOutputs: &SafeOutputsConfig{ MissingTool: &MissingToolConfig{}, diff --git a/pkg/workflow/reactions.go b/pkg/workflow/reactions.go index 5b1b7967a72..3ea4700ca52 100644 --- a/pkg/workflow/reactions.go +++ b/pkg/workflow/reactions.go @@ -9,29 +9,43 @@ import ( var reactionsLog = logger.New("workflow:reactions") +type ReactionType string + +const ( + ReactionTypePlusOne ReactionType = "+1" + ReactionTypeMinusOne ReactionType = "-1" + ReactionTypeLaugh ReactionType = "laugh" + ReactionTypeConfused ReactionType = "confused" + ReactionTypeHeart ReactionType = "heart" + ReactionTypeHooray ReactionType = "hooray" + ReactionTypeRocket ReactionType = "rocket" + ReactionTypeEyes ReactionType = "eyes" + ReactionTypeNone ReactionType = "none" +) + // validReactions defines the set of valid reaction values -var validReactions = map[string]bool{ - "+1": true, - "-1": true, - "laugh": true, - "confused": true, - "heart": true, - "hooray": true, - "rocket": true, - "eyes": true, - "none": true, +var validReactions = map[ReactionType]bool{ + ReactionTypePlusOne: true, + ReactionTypeMinusOne: true, + ReactionTypeLaugh: true, + ReactionTypeConfused: true, + ReactionTypeHeart: true, + ReactionTypeHooray: true, + ReactionTypeRocket: true, + ReactionTypeEyes: true, + ReactionTypeNone: true, } // isValidReaction checks if a reaction value is valid according to the schema func isValidReaction(reaction string) bool { - return validReactions[reaction] + return validReactions[ReactionType(reaction)] } // getValidReactions returns the list of valid reaction entries func getValidReactions() []string { reactions := make([]string, 0, len(validReactions)) for reaction := range validReactions { - reactions = append(reactions, reaction) + reactions = append(reactions, string(reaction)) } return reactions } @@ -61,7 +75,7 @@ func parseReactionValue(value any) (string, error) { case uint64: if v == 1 { reactionsLog.Print("Parsed uint64 reaction: +1") - return "+1", nil + return string(ReactionTypePlusOne), nil } reactionsLog.Printf("Invalid uint64 reaction value: %d", v) return "", fmt.Errorf("reaction value '%d' is not supported, expected one of %v. Example: reaction: eyes", v, getValidReactions()) @@ -69,11 +83,11 @@ func parseReactionValue(value any) (string, error) { // YAML may parse +1 and -1 as float64 if v == 1.0 { reactionsLog.Print("Parsed float64 reaction: +1") - return "+1", nil + return string(ReactionTypePlusOne), nil } if v == -1.0 { reactionsLog.Print("Parsed float64 reaction: -1") - return "-1", nil + return string(ReactionTypeMinusOne), nil } reactionsLog.Printf("Invalid float64 reaction value: %f", v) return "", fmt.Errorf("reaction value '%v' is not supported, expected one of %v. Example: reaction: eyes", v, getValidReactions()) @@ -89,7 +103,7 @@ func parseReactionValue(value any) (string, error) { // - object: {type, issues, pull-requests, discussions} func parseReactionConfig(value any) (string, *bool, *bool, *bool, error) { if reactionMap, ok := value.(map[string]any); ok { - reactionType := "eyes" + reactionType := string(ReactionTypeEyes) if typeValue, hasType := reactionMap["type"]; hasType { parsedType, err := parseReactionValue(typeValue) if err != nil { @@ -146,9 +160,9 @@ func parseBoolReactionField(m map[string]any, key string) (bool, error) { func intToReactionString(v int64) (string, error) { switch v { case 1: - return "+1", nil + return string(ReactionTypePlusOne), nil case -1: - return "-1", nil + return string(ReactionTypeMinusOne), nil default: return "", fmt.Errorf("reaction value '%d' is not supported, expected one of %v. Example: reaction: eyes", v, getValidReactions()) } diff --git a/pkg/workflow/runner_config.go b/pkg/workflow/runner_config.go index b1ca1a27a54..48844351d4f 100644 --- a/pkg/workflow/runner_config.go +++ b/pkg/workflow/runner_config.go @@ -24,7 +24,7 @@ func extractRunnerConfig(frontmatter map[string]any) *RunnerConfig { config := &RunnerConfig{} if topology, ok := runnerObj["topology"].(string); ok { - config.Topology = topology + config.Topology = RunnerTopology(topology) runnerConfigLog.Printf("Runner topology: %s", topology) } diff --git a/pkg/workflow/runner_config_test.go b/pkg/workflow/runner_config_test.go index 5f1b63ead3f..be0f06cca05 100644 --- a/pkg/workflow/runner_config_test.go +++ b/pkg/workflow/runner_config_test.go @@ -87,6 +87,6 @@ func TestGetRunnerTopology(t *testing.T) { wd := &WorkflowData{ RunnerConfig: &RunnerConfig{Topology: "arc-dind"}, } - assert.Equal(t, "arc-dind", getRunnerTopology(wd)) + assert.Equal(t, RunnerTopologyArcDind, getRunnerTopology(wd)) }) } diff --git a/pkg/workflow/safe_outputs_config_global.go b/pkg/workflow/safe_outputs_config_global.go index c37a9d2ca86..62e52b65975 100644 --- a/pkg/workflow/safe_outputs_config_global.go +++ b/pkg/workflow/safe_outputs_config_global.go @@ -28,7 +28,7 @@ func (c *Compiler) extractGlobalConfigFields(outputMap map[string]any, config *S // Parse URL sanitization policy if urls, exists := outputMap["urls"]; exists { if urlsStr, ok := urls.(string); ok { - config.URLs = urlsStr + config.URLs = SafeOutputsURLsPolicy(urlsStr) } } diff --git a/pkg/workflow/safe_outputs_config_types.go b/pkg/workflow/safe_outputs_config_types.go index 657e4e2cbd5..5a6454e63dc 100644 --- a/pkg/workflow/safe_outputs_config_types.go +++ b/pkg/workflow/safe_outputs_config_types.go @@ -89,7 +89,7 @@ type SafeOutputsConfig struct { Jobs map[string]*SafeJobConfig `yaml:"jobs,omitempty"` // Safe-jobs configuration (moved from top-level) Scripts map[string]*SafeScriptConfig `yaml:"scripts,omitempty"` // Custom inline handlers that run in the safe-output handler loop GitHubApp *GitHubAppConfig `yaml:"github-app,omitempty"` // GitHub App credentials for token minting - URLs string `yaml:"urls,omitempty"` // URL sanitization policy: SafeOutputsURLsPolicyAllowedOnly (default) or SafeOutputsURLsPolicyAllowedOrCodeRegion + URLs SafeOutputsURLsPolicy `yaml:"urls,omitempty"` // URL sanitization policy: SafeOutputsURLsPolicyAllowedOnly (default) or SafeOutputsURLsPolicyAllowedOrCodeRegion Data any `yaml:"data,omitempty"` // Structured data mode for body-based safe outputs: false/omitted (disabled), true (allow any object), object (inline schema), or GitHub Actions expression string DataEnabled bool `yaml:"-"` // Internal flag controlling whether `data` is allowed for body-based safe outputs NormalizedDataSchema map[string]any `yaml:"-"` // Internal normalized schema derived from inline `data` object schemas diff --git a/pkg/workflow/safe_outputs_validation.go b/pkg/workflow/safe_outputs_validation.go index abbf2358dc4..cb69fa32894 100644 --- a/pkg/workflow/safe_outputs_validation.go +++ b/pkg/workflow/safe_outputs_validation.go @@ -12,9 +12,11 @@ import ( var safeOutputsDomainsValidationLog = logger.New("workflow:safe_outputs_domains_validation") +type SafeOutputsURLsPolicy string + const ( - SafeOutputsURLsPolicyAllowedOnly = "allowed-only" - SafeOutputsURLsPolicyAllowedOrCodeRegion = "allowed-or-code-region" + SafeOutputsURLsPolicyAllowedOnly SafeOutputsURLsPolicy = "allowed-only" + SafeOutputsURLsPolicyAllowedOrCodeRegion SafeOutputsURLsPolicy = "allowed-or-code-region" ) // validateSafeOutputsURLs validates the urls policy in safe-outputs. diff --git a/pkg/workflow/trigger_parser.go b/pkg/workflow/trigger_parser.go index 4e4788e57a1..8576241c4e6 100644 --- a/pkg/workflow/trigger_parser.go +++ b/pkg/workflow/trigger_parser.go @@ -823,7 +823,7 @@ func parseOnMapPreamble(onMap map[string]any, workflowData *WorkflowData) (hasRe if !isValidReaction(reactionStr) { return false, false, false, fmt.Errorf("reaction value '%s' is not supported. Valid reactions are: %v. Example: reaction: eyes", reactionStr, getValidReactions()) } - workflowData.AIReaction = reactionStr + workflowData.AIReaction = ReactionType(reactionStr) workflowData.ReactionIssues = reactionIssues workflowData.ReactionPullRequests = reactionPullRequests workflowData.ReactionDiscussions = reactionDiscussions diff --git a/pkg/workflow/workflow_data.go b/pkg/workflow/workflow_data.go index 8bf44a15177..820e351ce7f 100644 --- a/pkg/workflow/workflow_data.go +++ b/pkg/workflow/workflow_data.go @@ -116,7 +116,7 @@ type WorkflowData struct { LabelCommandDecentralized bool // when true, label_command uses decentralized dispatch routing via agentic_commands.yml LabelCommandOtherEvents map[string]any // for merging label-command with other events LabelCommandRemoveLabel bool // whether to automatically remove the triggering label (default: true) - AIReaction string // AI reaction type like "eyes", "heart", etc. + AIReaction ReactionType // AI reaction type like "eyes", "heart", etc. ReactionIssues *bool // whether reactions are allowed on issues/issue_comment triggers (default: true) ReactionPullRequests *bool // whether reactions are allowed on pull_request/pull_request_review_comment triggers (default: true) ReactionDiscussions *bool // whether reactions are allowed on discussion/discussion_comment triggers (default: true)