diff --git a/.github/workflows/smoke-claude.lock.yml b/.github/workflows/smoke-claude.lock.yml index bae981e3b31..5e842865c4e 100644 --- a/.github/workflows/smoke-claude.lock.yml +++ b/.github/workflows/smoke-claude.lock.yml @@ -865,7 +865,7 @@ jobs: mkdir -p /tmp/gh-aw/safeoutputs mkdir -p /tmp/gh-aw/mcp-logs/safeoutputs cat > ${RUNNER_TEMP}/gh-aw/safeoutputs/config.json << 'GH_AW_SAFE_OUTPUTS_CONFIG_EOF' - {"add_comment":{"max":2},"add_labels":{"allowed":["smoke-claude"],"max":3},"add_reviewer":{"max":2},"close_pull_request":{"max":1,"staged":true},"create_issue":{"expires":2,"group":true,"max":1},"create_pull_request_review_comment":{"max":5},"missing_data":{},"missing_tool":{},"noop":{"max":1},"post_slack_message":{"description":"Post a message to a fictitious Slack channel (smoke test only — no real Slack integration)","inputs":{"channel":{"default":"#general","description":"Slack channel name to post to","required":false,"type":"string"},"message":{"description":"Message text to post","required":false,"type":"string"}}},"push_to_pull_request_branch":{"max":1,"target":"*"},"resolve_pull_request_review_thread":{"max":5},"submit_pull_request_review":{"max":1},"update_pull_request":{"max":1}} + {"add_comment":{"max":2},"add_labels":{"allowed":["smoke-claude"],"max":3},"add_reviewer":{"max":2},"close_pull_request":{"max":1,"staged":true},"create_issue":{"expires":2,"group":true,"max":1},"create_pull_request_review_comment":{"max":5},"missing_data":{},"missing_tool":{},"noop":{"max":1},"post_slack_message":{"description":"Post a message to a fictitious Slack channel (smoke test only — no real Slack integration)","inputs":{"channel":{"default":"#general","description":"Slack channel name to post to","required":false,"type":"string"},"message":{"description":"Message text to post","required":false,"type":"string"}}},"push_to_pull_request_branch":{"max":1,"staged":true,"target":"*"},"resolve_pull_request_review_thread":{"max":5},"submit_pull_request_review":{"max":1},"update_pull_request":{"max":1}} GH_AW_SAFE_OUTPUTS_CONFIG_EOF - name: Write Safe Outputs Tools run: | diff --git a/pkg/workflow/compiler_safe_outputs_config.go b/pkg/workflow/compiler_safe_outputs_config.go index 66ab52c0c95..667af6e9174 100644 --- a/pkg/workflow/compiler_safe_outputs_config.go +++ b/pkg/workflow/compiler_safe_outputs_config.go @@ -7,6 +7,28 @@ import ( "github.com/github/gh-aw/pkg/logger" ) +// ======================================== +// Handler Manager Config Generation +// ======================================== +// +// This file produces the GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG env var consumed +// by the handler manager at runtime, using the handlerRegistry and the fluent +// handlerConfigBuilder API. +// +// # Dual Config Generation Systems +// +// There are two code paths that generate safe-output handler configuration: +// +// 1. generateSafeOutputsConfig() in safe_outputs_config_generation.go — produces +// GH_AW_SAFE_OUTPUTS_CONFIG_PATH (config.json) consumed by the MCP server at startup. +// +// 2. addHandlerManagerConfigEnvVar() (this file) — produces GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG +// consumed by the handler manager at runtime. This is the authoritative field contract: +// the handlerRegistry entries here define which fields each handler accepts. +// +// When adding a new handler field, update both this file AND safe_outputs_config_generation.go +// to keep the two paths in sync. + var compilerSafeOutputsConfigLog = logger.New("workflow:compiler_safe_outputs_config") // getEffectiveFooterForTemplatable returns the effective footer as a templatable string. diff --git a/pkg/workflow/frontmatter_extraction_metadata.go b/pkg/workflow/frontmatter_extraction_metadata.go index 165f08dcab6..8b6b259f1d3 100644 --- a/pkg/workflow/frontmatter_extraction_metadata.go +++ b/pkg/workflow/frontmatter_extraction_metadata.go @@ -142,9 +142,6 @@ func buildSourceURL(source string) string { return url } -// safeUintToInt safely converts uint to int, returning 0 if overflow would occur -func safeUintToInt(u uint) int { return safeUint64ToInt(uint64(u)) } - // extractToolsTimeout extracts the timeout setting from tools // Returns 0 if not set (engines will use their own defaults) // Returns error if timeout is explicitly set but invalid (< 1) diff --git a/pkg/workflow/map_helpers.go b/pkg/workflow/map_helpers.go index 4e374369b16..3e3b93479cf 100644 --- a/pkg/workflow/map_helpers.go +++ b/pkg/workflow/map_helpers.go @@ -17,9 +17,14 @@ // # Key Functions // // Type Conversion: -// - parseIntValue() - Safely parse numeric types to int with truncation warnings +// - parseIntValue() - Strictly parse numeric types to int; returns (value, ok). Use when +// the caller needs to distinguish "missing/invalid" from a zero value, or when string +// inputs are not expected (e.g. YAML config field parsing). // - safeUint64ToInt() - Convert uint64 to int, returning 0 on overflow -// - ConvertToInt() - Safely convert any value (int/int64/float64/string) to int +// - safeUintToInt() - Convert uint to int, returning 0 on overflow (thin wrapper around safeUint64ToInt) +// - ConvertToInt() - Leniently convert any value (int/int64/float64/string) to int, returning 0 +// on failure. Use when the input may come from heterogeneous sources such as JSON metrics, +// log parsing, or user-provided strings where a zero default on failure is acceptable. // - ConvertToFloat() - Safely convert any value (float64/int/int64/string) to float64 // // Map Operations: @@ -41,8 +46,15 @@ import ( var mapHelpersLog = logger.New("workflow:map_helpers") -// parseIntValue safely parses various numeric types to int -// This is a common utility used across multiple parsing functions +// parseIntValue strictly parses numeric types to int, returning (value, true) on success +// and (0, false) for any unrecognized or non-numeric type. +// +// Use this when the caller needs to distinguish a missing/invalid value from a legitimate +// zero, or when string inputs are not expected (e.g. YAML config field parsing where the +// YAML library has already produced a typed numeric value). +// +// For lenient conversion that also handles string inputs and returns 0 on failure, use +// ConvertToInt instead. func parseIntValue(value any) (int, bool) { switch v := value.(type) { case int: @@ -77,6 +89,10 @@ func safeUint64ToInt(u uint64) int { return int(u) } +// safeUintToInt safely converts uint to int, returning 0 if overflow would occur. +// This is a thin wrapper around safeUint64ToInt that widens the uint argument first. +func safeUintToInt(u uint) int { return safeUint64ToInt(uint64(u)) } + // filterMapKeys creates a new map excluding the specified keys func filterMapKeys(original map[string]any, excludeKeys ...string) map[string]any { excludeSet := make(map[string]bool) @@ -104,7 +120,15 @@ func sortedMapKeys(m map[string]string) []string { return keys } -// ConvertToInt safely converts any to int +// ConvertToInt leniently converts any value to int, returning 0 on failure. +// +// Unlike parseIntValue, this function also handles string inputs via strconv.Atoi, +// making it suitable for heterogeneous sources such as JSON metrics, log-parsed data, +// or user-provided configuration where a zero default on failure is acceptable and +// the caller does not need to distinguish "invalid" from a genuine zero. +// +// For strict numeric-only parsing where the caller must distinguish missing/invalid +// values from zero, use parseIntValue instead. func ConvertToInt(val any) int { switch v := val.(type) { case int: diff --git a/pkg/workflow/safe_outputs_config_generation.go b/pkg/workflow/safe_outputs_config_generation.go index f087e09d4f3..40df5f48062 100644 --- a/pkg/workflow/safe_outputs_config_generation.go +++ b/pkg/workflow/safe_outputs_config_generation.go @@ -18,6 +18,23 @@ import ( // normalized JSON configuration objects consumed by the safe-outputs MCP server. // // Helper functions for building per-tool config maps are in safe_outputs_config_helpers.go. +// +// # Dual Config Generation Systems +// +// There are two code paths that generate safe-output handler configuration: +// +// 1. generateSafeOutputsConfig() (this file) — produces the GH_AW_SAFE_OUTPUTS_CONFIG_PATH +// (config.json) consumed by the safe-outputs MCP server at startup. +// Uses ad-hoc generateMax*Config() helper functions from safe_outputs_config_helpers.go. +// +// 2. addHandlerManagerConfigEnvVar() in compiler_safe_outputs_config.go — produces the +// GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG env var consumed by the handler manager at runtime. +// Uses the handlerRegistry + fluent handlerConfigBuilder API. +// +// These two paths serve distinct runtime purposes and must be kept in sync when adding +// new handler fields. When adding a new field to a handler, update both this file and +// compiler_safe_outputs_config.go. See compiler_safe_outputs_config.go for the full +// field contract defined by the handlerRegistry. // generateSafeOutputsConfig transforms workflow safe-outputs configuration into a // JSON string consumed by the safe-outputs MCP server at runtime. @@ -47,16 +64,20 @@ func generateSafeOutputsConfig(data *WorkflowData) string { if data.SafeOutputs.CreateIssues.Expires > 0 { config["expires"] = data.SafeOutputs.CreateIssues.Expires } + addStagedIfTrue(config, data.SafeOutputs.CreateIssues.Staged) safeOutputsConfig["create_issue"] = config } if data.SafeOutputs.CreateAgentSessions != nil { - safeOutputsConfig["create_agent_session"] = generateMaxConfig( + config := generateMaxConfig( data.SafeOutputs.CreateAgentSessions.Max, 1, // default max ) + addStagedIfTrue(config, data.SafeOutputs.CreateAgentSessions.Staged) + safeOutputsConfig["create_agent_session"] = config } if data.SafeOutputs.AddComments != nil { additionalFields := make(map[string]any) + addStagedIfTrue(additionalFields, data.SafeOutputs.AddComments.Staged) // Note: AddCommentsConfig has Target, TargetRepoSlug, AllowedRepos but not embedded SafeOutputTargetConfig // So we need to construct the target config manually targetConfig := SafeOutputTargetConfig{ @@ -81,16 +102,19 @@ func generateSafeOutputsConfig(data *WorkflowData) string { if data.SafeOutputs.CreateDiscussions.Expires > 0 { config["expires"] = data.SafeOutputs.CreateDiscussions.Expires } + addStagedIfTrue(config, data.SafeOutputs.CreateDiscussions.Staged) safeOutputsConfig["create_discussion"] = config } if data.SafeOutputs.CloseDiscussions != nil { - safeOutputsConfig["close_discussion"] = generateMaxWithDiscussionFieldsConfig( + config := generateMaxWithDiscussionFieldsConfig( data.SafeOutputs.CloseDiscussions.Max, 1, // default max data.SafeOutputs.CloseDiscussions.RequiredCategory, data.SafeOutputs.CloseDiscussions.RequiredLabels, data.SafeOutputs.CloseDiscussions.RequiredTitlePrefix, ) + addStagedIfTrue(config, data.SafeOutputs.CloseDiscussions.Staged) + safeOutputsConfig["close_discussion"] = config } if data.SafeOutputs.CloseIssues != nil { additionalFields := make(map[string]any) @@ -100,6 +124,7 @@ func generateSafeOutputsConfig(data *WorkflowData) string { if data.SafeOutputs.CloseIssues.RequiredTitlePrefix != "" { additionalFields["required_title_prefix"] = data.SafeOutputs.CloseIssues.RequiredTitlePrefix } + addStagedIfTrue(additionalFields, data.SafeOutputs.CloseIssues.Staged) safeOutputsConfig["close_issue"] = generateTargetConfigWithRepos( data.SafeOutputs.CloseIssues.SafeOutputTargetConfig, data.SafeOutputs.CloseIssues.Max, @@ -118,9 +143,7 @@ func generateSafeOutputsConfig(data *WorkflowData) string { if data.SafeOutputs.ClosePullRequests.GitHubToken != "" { additionalFields["github-token"] = data.SafeOutputs.ClosePullRequests.GitHubToken } - if data.SafeOutputs.ClosePullRequests.Staged { - additionalFields["staged"] = true - } + addStagedIfTrue(additionalFields, data.SafeOutputs.ClosePullRequests.Staged) safeOutputsConfig["close_pull_request"] = generateTargetConfigWithRepos( data.SafeOutputs.ClosePullRequests.SafeOutputTargetConfig, data.SafeOutputs.ClosePullRequests.Max, @@ -129,26 +152,33 @@ func generateSafeOutputsConfig(data *WorkflowData) string { ) } if data.SafeOutputs.CreatePullRequests != nil { - safeOutputsConfig["create_pull_request"] = generatePullRequestConfig( + config := generatePullRequestConfig( data.SafeOutputs.CreatePullRequests, 1, // default max ) + addStagedIfTrue(config, data.SafeOutputs.CreatePullRequests.Staged) + safeOutputsConfig["create_pull_request"] = config } if data.SafeOutputs.CreatePullRequestReviewComments != nil { - safeOutputsConfig["create_pull_request_review_comment"] = generateMaxConfig( + config := generateMaxConfig( data.SafeOutputs.CreatePullRequestReviewComments.Max, 10, // default max ) + addStagedIfTrue(config, data.SafeOutputs.CreatePullRequestReviewComments.Staged) + safeOutputsConfig["create_pull_request_review_comment"] = config } if data.SafeOutputs.SubmitPullRequestReview != nil { - safeOutputsConfig["submit_pull_request_review"] = generateMaxConfig( + config := generateMaxConfig( data.SafeOutputs.SubmitPullRequestReview.Max, 1, // default max ) + addStagedIfTrue(config, data.SafeOutputs.SubmitPullRequestReview.Staged) + safeOutputsConfig["submit_pull_request_review"] = config } if data.SafeOutputs.ReplyToPullRequestReviewComment != nil { additionalFields := newHandlerConfigBuilder(). AddTemplatableBool("footer", data.SafeOutputs.ReplyToPullRequestReviewComment.Footer). + AddIfTrue("staged", data.SafeOutputs.ReplyToPullRequestReviewComment.Staged). Build() safeOutputsConfig["reply_to_pull_request_review_comment"] = generateTargetConfigWithRepos( data.SafeOutputs.ReplyToPullRequestReviewComment.SafeOutputTargetConfig, @@ -158,22 +188,28 @@ func generateSafeOutputsConfig(data *WorkflowData) string { ) } if data.SafeOutputs.ResolvePullRequestReviewThread != nil { - safeOutputsConfig["resolve_pull_request_review_thread"] = generateMaxConfig( + config := generateMaxConfig( data.SafeOutputs.ResolvePullRequestReviewThread.Max, 10, // default max ) + addStagedIfTrue(config, data.SafeOutputs.ResolvePullRequestReviewThread.Staged) + safeOutputsConfig["resolve_pull_request_review_thread"] = config } if data.SafeOutputs.CreateCodeScanningAlerts != nil { - safeOutputsConfig["create_code_scanning_alert"] = generateMaxConfig( + config := generateMaxConfig( data.SafeOutputs.CreateCodeScanningAlerts.Max, 0, // default: unlimited ) + addStagedIfTrue(config, data.SafeOutputs.CreateCodeScanningAlerts.Staged) + safeOutputsConfig["create_code_scanning_alert"] = config } if data.SafeOutputs.AutofixCodeScanningAlert != nil { - safeOutputsConfig["autofix_code_scanning_alert"] = generateMaxConfig( + config := generateMaxConfig( data.SafeOutputs.AutofixCodeScanningAlert.Max, 10, // default max ) + addStagedIfTrue(config, data.SafeOutputs.AutofixCodeScanningAlert.Staged) + safeOutputsConfig["autofix_code_scanning_alert"] = config } if data.SafeOutputs.AddLabels != nil { additionalFields := make(map[string]any) @@ -183,6 +219,7 @@ func generateSafeOutputsConfig(data *WorkflowData) string { if len(data.SafeOutputs.AddLabels.Blocked) > 0 { additionalFields["blocked"] = data.SafeOutputs.AddLabels.Blocked } + addStagedIfTrue(additionalFields, data.SafeOutputs.AddLabels.Staged) safeOutputsConfig["add_labels"] = generateTargetConfigWithRepos( data.SafeOutputs.AddLabels.SafeOutputTargetConfig, data.SafeOutputs.AddLabels.Max, @@ -191,88 +228,112 @@ func generateSafeOutputsConfig(data *WorkflowData) string { ) } if data.SafeOutputs.RemoveLabels != nil { - safeOutputsConfig["remove_labels"] = generateMaxWithAllowedConfig( + config := generateMaxWithAllowedConfig( data.SafeOutputs.RemoveLabels.Max, 3, // default max data.SafeOutputs.RemoveLabels.Allowed, ) + addStagedIfTrue(config, data.SafeOutputs.RemoveLabels.Staged) + safeOutputsConfig["remove_labels"] = config } if data.SafeOutputs.AddReviewer != nil { - safeOutputsConfig["add_reviewer"] = generateMaxWithReviewersConfig( + config := generateMaxWithReviewersConfig( data.SafeOutputs.AddReviewer.Max, 3, // default max data.SafeOutputs.AddReviewer.Reviewers, ) + addStagedIfTrue(config, data.SafeOutputs.AddReviewer.Staged) + safeOutputsConfig["add_reviewer"] = config } if data.SafeOutputs.AssignMilestone != nil { - safeOutputsConfig["assign_milestone"] = generateMaxWithAllowedConfig( + config := generateMaxWithAllowedConfig( data.SafeOutputs.AssignMilestone.Max, 1, // default max data.SafeOutputs.AssignMilestone.Allowed, ) + addStagedIfTrue(config, data.SafeOutputs.AssignMilestone.Staged) + safeOutputsConfig["assign_milestone"] = config } if data.SafeOutputs.AssignToAgent != nil { - safeOutputsConfig["assign_to_agent"] = generateAssignToAgentConfig( + config := generateAssignToAgentConfig( data.SafeOutputs.AssignToAgent.Max, 1, // default max data.SafeOutputs.AssignToAgent.DefaultAgent, data.SafeOutputs.AssignToAgent.Target, data.SafeOutputs.AssignToAgent.Allowed, ) + addStagedIfTrue(config, data.SafeOutputs.AssignToAgent.Staged) + safeOutputsConfig["assign_to_agent"] = config } if data.SafeOutputs.AssignToUser != nil { - safeOutputsConfig["assign_to_user"] = generateMaxWithAllowedAndBlockedConfig( + config := generateMaxWithAllowedAndBlockedConfig( data.SafeOutputs.AssignToUser.Max, 1, // default max data.SafeOutputs.AssignToUser.Allowed, data.SafeOutputs.AssignToUser.Blocked, ) + addStagedIfTrue(config, data.SafeOutputs.AssignToUser.Staged) + safeOutputsConfig["assign_to_user"] = config } if data.SafeOutputs.UnassignFromUser != nil { - safeOutputsConfig["unassign_from_user"] = generateMaxWithAllowedAndBlockedConfig( + config := generateMaxWithAllowedAndBlockedConfig( data.SafeOutputs.UnassignFromUser.Max, 1, // default max data.SafeOutputs.UnassignFromUser.Allowed, data.SafeOutputs.UnassignFromUser.Blocked, ) + addStagedIfTrue(config, data.SafeOutputs.UnassignFromUser.Staged) + safeOutputsConfig["unassign_from_user"] = config } if data.SafeOutputs.UpdateIssues != nil { - safeOutputsConfig["update_issue"] = generateMaxConfig( + config := generateMaxConfig( data.SafeOutputs.UpdateIssues.Max, 1, // default max ) + addStagedIfTrue(config, data.SafeOutputs.UpdateIssues.Staged) + safeOutputsConfig["update_issue"] = config } if data.SafeOutputs.UpdateDiscussions != nil { - safeOutputsConfig["update_discussion"] = generateMaxWithAllowedLabelsConfig( + config := generateMaxWithAllowedLabelsConfig( data.SafeOutputs.UpdateDiscussions.Max, 1, // default max data.SafeOutputs.UpdateDiscussions.AllowedLabels, ) + addStagedIfTrue(config, data.SafeOutputs.UpdateDiscussions.Staged) + safeOutputsConfig["update_discussion"] = config } if data.SafeOutputs.UpdatePullRequests != nil { - safeOutputsConfig["update_pull_request"] = generateMaxConfig( + config := generateMaxConfig( data.SafeOutputs.UpdatePullRequests.Max, 1, // default max ) + addStagedIfTrue(config, data.SafeOutputs.UpdatePullRequests.Staged) + safeOutputsConfig["update_pull_request"] = config } if data.SafeOutputs.MarkPullRequestAsReadyForReview != nil { - safeOutputsConfig["mark_pull_request_as_ready_for_review"] = generateMaxConfig( + config := generateMaxConfig( data.SafeOutputs.MarkPullRequestAsReadyForReview.Max, 10, // default max ) + addStagedIfTrue(config, data.SafeOutputs.MarkPullRequestAsReadyForReview.Staged) + safeOutputsConfig["mark_pull_request_as_ready_for_review"] = config } if data.SafeOutputs.PushToPullRequestBranch != nil { - safeOutputsConfig["push_to_pull_request_branch"] = generateMaxWithTargetConfig( + config := generateMaxWithTargetConfig( data.SafeOutputs.PushToPullRequestBranch.Max, 1, // default max: 1 data.SafeOutputs.PushToPullRequestBranch.Target, ) + addStagedIfTrue(config, data.SafeOutputs.PushToPullRequestBranch.Staged) + safeOutputsConfig["push_to_pull_request_branch"] = config } if data.SafeOutputs.UploadAssets != nil { - safeOutputsConfig["upload_asset"] = generateMaxConfig( + config := generateMaxConfig( data.SafeOutputs.UploadAssets.Max, 0, // default: unlimited ) + addStagedIfTrue(config, data.SafeOutputs.UploadAssets.Staged) + safeOutputsConfig["upload_asset"] = config } if data.SafeOutputs.MissingTool != nil { // Generate config for missing_tool with issue creation support @@ -329,6 +390,8 @@ func generateSafeOutputsConfig(data *WorkflowData) string { safeOutputsConfig["missing_data"] = missingDataConfig } if data.SafeOutputs.UpdateProjects != nil { + additionalFields := make(map[string]any) + addStagedIfTrue(additionalFields, data.SafeOutputs.UpdateProjects.Staged) safeOutputsConfig["update_project"] = generateTargetConfigWithRepos( SafeOutputTargetConfig{ TargetRepoSlug: data.SafeOutputs.UpdateProjects.TargetRepoSlug, @@ -336,14 +399,16 @@ func generateSafeOutputsConfig(data *WorkflowData) string { }, data.SafeOutputs.UpdateProjects.Max, 10, // default max - nil, + additionalFields, ) } if data.SafeOutputs.CreateProjectStatusUpdates != nil { - safeOutputsConfig["create_project_status_update"] = generateMaxConfig( + config := generateMaxConfig( data.SafeOutputs.CreateProjectStatusUpdates.Max, 10, // default max ) + addStagedIfTrue(config, data.SafeOutputs.CreateProjectStatusUpdates.Staged) + safeOutputsConfig["create_project_status_update"] = config } if data.SafeOutputs.CreateProjects != nil { config := generateMaxConfig( @@ -358,19 +423,24 @@ func generateSafeOutputsConfig(data *WorkflowData) string { if data.SafeOutputs.CreateProjects.TitlePrefix != "" { config["title_prefix"] = data.SafeOutputs.CreateProjects.TitlePrefix } + addStagedIfTrue(config, data.SafeOutputs.CreateProjects.Staged) safeOutputsConfig["create_project"] = config } if data.SafeOutputs.UpdateRelease != nil { - safeOutputsConfig["update_release"] = generateMaxConfig( + config := generateMaxConfig( data.SafeOutputs.UpdateRelease.Max, 1, // default max ) + addStagedIfTrue(config, data.SafeOutputs.UpdateRelease.Staged) + safeOutputsConfig["update_release"] = config } if data.SafeOutputs.LinkSubIssue != nil { - safeOutputsConfig["link_sub_issue"] = generateMaxConfig( + config := generateMaxConfig( data.SafeOutputs.LinkSubIssue.Max, 5, // default max ) + addStagedIfTrue(config, data.SafeOutputs.LinkSubIssue.Staged) + safeOutputsConfig["link_sub_issue"] = config } if data.SafeOutputs.NoOp != nil { safeOutputsConfig["noop"] = generateMaxConfig( @@ -379,17 +449,20 @@ func generateSafeOutputsConfig(data *WorkflowData) string { ) } if data.SafeOutputs.HideComment != nil { - safeOutputsConfig["hide_comment"] = generateHideCommentConfig( + config := generateHideCommentConfig( data.SafeOutputs.HideComment.Max, 5, // default max data.SafeOutputs.HideComment.AllowedReasons, ) + addStagedIfTrue(config, data.SafeOutputs.HideComment.Staged) + safeOutputsConfig["hide_comment"] = config } if data.SafeOutputs.SetIssueType != nil { additionalFields := make(map[string]any) if len(data.SafeOutputs.SetIssueType.Allowed) > 0 { additionalFields["allowed"] = data.SafeOutputs.SetIssueType.Allowed } + addStagedIfTrue(additionalFields, data.SafeOutputs.SetIssueType.Staged) safeOutputsConfig["set_issue_type"] = generateTargetConfigWithRepos( data.SafeOutputs.SetIssueType.SafeOutputTargetConfig, data.SafeOutputs.SetIssueType.Max, @@ -530,6 +603,8 @@ func generateSafeOutputsConfig(data *WorkflowData) string { // Include max count dispatchWorkflowConfig["max"] = resolveMaxForConfig(data.SafeOutputs.DispatchWorkflow.Max, 1) + addStagedIfTrue(dispatchWorkflowConfig, data.SafeOutputs.DispatchWorkflow.Staged) + // Only add if it has fields if len(dispatchWorkflowConfig) > 0 { safeOutputsConfig["dispatch_workflow"] = dispatchWorkflowConfig diff --git a/pkg/workflow/safe_outputs_config_helpers.go b/pkg/workflow/safe_outputs_config_helpers.go index c76d4ddfdb6..16c249c9bcd 100644 --- a/pkg/workflow/safe_outputs_config_helpers.go +++ b/pkg/workflow/safe_outputs_config_helpers.go @@ -41,6 +41,14 @@ func resolveMaxForConfig(max *string, defaultMax int) any { return defaultMax } +// addStagedIfTrue sets config["staged"] = true when staged is true. +// Used by generateSafeOutputsConfig to propagate per-handler staged flags into config.json. +func addStagedIfTrue(config map[string]any, staged bool) { + if staged { + config["staged"] = true + } +} + // generateMaxConfig creates a simple config map with just a max value func generateMaxConfig(max *string, defaultMax int) map[string]any { config := make(map[string]any) diff --git a/pkg/workflow/time_delta_integration_test.go b/pkg/workflow/time_delta_integration_test.go index 9a97f8b6951..fa4e3f735eb 100644 --- a/pkg/workflow/time_delta_integration_test.go +++ b/pkg/workflow/time_delta_integration_test.go @@ -181,6 +181,10 @@ on: } timestamp := strings.TrimSpace(parts[1]) + // The value is YAML-quoted (e.g. "2026-03-24 21:18:46") because the + // compiler uses %q to prevent YAML from interpreting the time string as + // a date type. Strip the surrounding double-quotes before parsing. + timestamp = strings.Trim(timestamp, `"`) // Parse as timestamp to verify it's valid _, err := time.Parse("2006-01-02 15:04:05", timestamp) diff --git a/pkg/workflow/update_discussion_helpers.go b/pkg/workflow/update_discussion_helpers.go new file mode 100644 index 00000000000..020274bf848 --- /dev/null +++ b/pkg/workflow/update_discussion_helpers.go @@ -0,0 +1,45 @@ +// This file provides configuration types and parsing for the update-discussion safe output. +// +// For shared update entity infrastructure (types, generic parsers, field parsing modes), +// see update_entity_helpers.go. + +package workflow + +import "github.com/github/gh-aw/pkg/logger" + +var updateDiscussionLog = logger.New("workflow:update_discussion") + +// UpdateDiscussionsConfig holds configuration for updating GitHub discussions from agent output +type UpdateDiscussionsConfig struct { + UpdateEntityConfig `yaml:",inline"` + Title *bool `yaml:"title,omitempty"` // Allow updating discussion title - presence indicates field can be updated + Body *bool `yaml:"body,omitempty"` // Allow updating discussion body - presence indicates field can be updated + Labels *bool `yaml:"labels,omitempty"` // Allow updating discussion labels - presence indicates field can be updated + AllowedLabels []string `yaml:"allowed-labels,omitempty"` // Optional list of allowed labels. If omitted, any labels are allowed (including creating new ones). + Footer *string `yaml:"footer,omitempty"` // Controls whether AI-generated footer is added. When false, visible footer is omitted but XML markers are kept. +} + +// parseUpdateDiscussionsConfig handles update-discussion configuration +func (c *Compiler) parseUpdateDiscussionsConfig(outputMap map[string]any) *UpdateDiscussionsConfig { + return parseUpdateEntityConfigTyped(c, outputMap, + UpdateEntityDiscussion, "update-discussion", updateDiscussionLog, + func(cfg *UpdateDiscussionsConfig) []UpdateEntityFieldSpec { + return []UpdateEntityFieldSpec{ + {Name: "title", Mode: FieldParsingKeyExistence, Dest: &cfg.Title}, + {Name: "body", Mode: FieldParsingKeyExistence, Dest: &cfg.Body}, + {Name: "labels", Mode: FieldParsingKeyExistence, Dest: &cfg.Labels}, + {Name: "footer", Mode: FieldParsingTemplatableBool, StringDest: &cfg.Footer}, + } + }, + func(cm map[string]any, cfg *UpdateDiscussionsConfig) { + // Parse allowed-labels using shared helper + cfg.AllowedLabels = parseAllowedLabelsFromConfig(cm) + if len(cfg.AllowedLabels) > 0 { + updateDiscussionLog.Printf("Allowed labels configured: %v", cfg.AllowedLabels) + // If allowed-labels is specified, implicitly enable labels + if cfg.Labels == nil { + cfg.Labels = new(bool) + } + } + }) +} diff --git a/pkg/workflow/update_entity_helpers.go b/pkg/workflow/update_entity_helpers.go index 31cce299813..7e611bfaf0a 100644 --- a/pkg/workflow/update_entity_helpers.go +++ b/pkg/workflow/update_entity_helpers.go @@ -1,4 +1,4 @@ -// This file provides helper functions for updating GitHub entities. +// This file provides shared infrastructure for updating GitHub entities. // // This file contains shared utilities for building update entity jobs (issues, // pull requests, discussions, releases). These helpers extract common patterns @@ -13,6 +13,12 @@ // - Support two field parsing modes (key existence vs. bool value) // - Enable DRY principles for update operations // +// Domain-specific configuration types and parsers live in dedicated files: +// - update_issue_helpers.go — UpdateIssuesConfig, parseUpdateIssuesConfig +// - update_discussion_helpers.go — UpdateDiscussionsConfig, parseUpdateDiscussionsConfig +// - update_pull_request_helpers.go — UpdatePullRequestsConfig, parseUpdatePullRequestsConfig +// - update_release.go — UpdateReleaseConfig, parseUpdateReleaseConfig +// // This follows the helper file conventions documented in the developer instructions. // See skills/developer/SKILL.md#helper-file-conventions for details. // @@ -72,9 +78,6 @@ import ( ) var updateEntityHelpersLog = logger.New("workflow:update_entity_helpers") -var updateIssueLog = logger.New("workflow:update_issue") -var updateDiscussionLog = logger.New("workflow:update_discussion") -var updatePullRequestLog = logger.New("workflow:update_pull_request") // UpdateEntityType represents the type of entity being updated type UpdateEntityType string @@ -449,95 +452,3 @@ func parseUpdateEntityConfigTyped[T any]( return cfg } - -// UpdateIssuesConfig holds configuration for updating GitHub issues from agent output -type UpdateIssuesConfig struct { - UpdateEntityConfig `yaml:",inline"` - Status *bool `yaml:"status,omitempty"` // Allow updating issue status (open/closed) - presence indicates field can be updated - Title *bool `yaml:"title,omitempty"` // Allow updating issue title - presence indicates field can be updated - Body *bool `yaml:"body,omitempty"` // Allow updating issue body - boolean value controls permission (defaults to true) - Footer *string `yaml:"footer,omitempty"` // Controls whether AI-generated footer is added. When false, visible footer is omitted but XML markers are kept. - TitlePrefix string `yaml:"title-prefix,omitempty"` // Required title prefix for issue validation - only issues with this prefix can be updated -} - -// parseUpdateIssuesConfig handles update-issue configuration -func (c *Compiler) parseUpdateIssuesConfig(outputMap map[string]any) *UpdateIssuesConfig { - return parseUpdateEntityConfigTyped(c, outputMap, - UpdateEntityIssue, "update-issue", updateIssueLog, - func(cfg *UpdateIssuesConfig) []UpdateEntityFieldSpec { - return []UpdateEntityFieldSpec{ - {Name: "status", Mode: FieldParsingKeyExistence, Dest: &cfg.Status}, - {Name: "title", Mode: FieldParsingKeyExistence, Dest: &cfg.Title}, - {Name: "body", Mode: FieldParsingBoolValue, Dest: &cfg.Body}, - {Name: "footer", Mode: FieldParsingTemplatableBool, StringDest: &cfg.Footer}, - } - }, func(configMap map[string]any, cfg *UpdateIssuesConfig) { - cfg.TitlePrefix = parseTitlePrefixFromConfig(configMap) - }) -} - -// UpdateDiscussionsConfig holds configuration for updating GitHub discussions from agent output -type UpdateDiscussionsConfig struct { - UpdateEntityConfig `yaml:",inline"` - Title *bool `yaml:"title,omitempty"` // Allow updating discussion title - presence indicates field can be updated - Body *bool `yaml:"body,omitempty"` // Allow updating discussion body - presence indicates field can be updated - Labels *bool `yaml:"labels,omitempty"` // Allow updating discussion labels - presence indicates field can be updated - AllowedLabels []string `yaml:"allowed-labels,omitempty"` // Optional list of allowed labels. If omitted, any labels are allowed (including creating new ones). - Footer *string `yaml:"footer,omitempty"` // Controls whether AI-generated footer is added. When false, visible footer is omitted but XML markers are kept. -} - -// parseUpdateDiscussionsConfig handles update-discussion configuration -func (c *Compiler) parseUpdateDiscussionsConfig(outputMap map[string]any) *UpdateDiscussionsConfig { - return parseUpdateEntityConfigTyped(c, outputMap, - UpdateEntityDiscussion, "update-discussion", updateDiscussionLog, - func(cfg *UpdateDiscussionsConfig) []UpdateEntityFieldSpec { - return []UpdateEntityFieldSpec{ - {Name: "title", Mode: FieldParsingKeyExistence, Dest: &cfg.Title}, - {Name: "body", Mode: FieldParsingKeyExistence, Dest: &cfg.Body}, - {Name: "labels", Mode: FieldParsingKeyExistence, Dest: &cfg.Labels}, - {Name: "footer", Mode: FieldParsingTemplatableBool, StringDest: &cfg.Footer}, - } - }, - func(cm map[string]any, cfg *UpdateDiscussionsConfig) { - // Parse allowed-labels using shared helper - cfg.AllowedLabels = parseAllowedLabelsFromConfig(cm) - if len(cfg.AllowedLabels) > 0 { - updateDiscussionLog.Printf("Allowed labels configured: %v", cfg.AllowedLabels) - // If allowed-labels is specified, implicitly enable labels - if cfg.Labels == nil { - cfg.Labels = new(bool) - } - } - }) -} - -// UpdatePullRequestsConfig holds configuration for updating GitHub pull requests from agent output -type UpdatePullRequestsConfig struct { - UpdateEntityConfig `yaml:",inline"` - Title *bool `yaml:"title,omitempty"` // Allow updating PR title - defaults to true, set to false to disable - Body *bool `yaml:"body,omitempty"` // Allow updating PR body - defaults to true, set to false to disable - Operation *string `yaml:"operation,omitempty"` // Default operation for body updates: "append", "prepend", or "replace" (defaults to "replace") - Footer *string `yaml:"footer,omitempty"` // Controls whether AI-generated footer is added. When false, visible footer is omitted. -} - -// parseUpdatePullRequestsConfig handles update-pull-request configuration -func (c *Compiler) parseUpdatePullRequestsConfig(outputMap map[string]any) *UpdatePullRequestsConfig { - updatePullRequestLog.Print("Parsing update pull request configuration") - - return parseUpdateEntityConfigTyped(c, outputMap, - UpdateEntityPullRequest, "update-pull-request", updatePullRequestLog, - func(cfg *UpdatePullRequestsConfig) []UpdateEntityFieldSpec { - return []UpdateEntityFieldSpec{ - {Name: "title", Mode: FieldParsingBoolValue, Dest: &cfg.Title}, - {Name: "body", Mode: FieldParsingBoolValue, Dest: &cfg.Body}, - {Name: "footer", Mode: FieldParsingTemplatableBool, StringDest: &cfg.Footer}, - } - }, func(configMap map[string]any, cfg *UpdatePullRequestsConfig) { - // Parse operation field - if operationVal, exists := configMap["operation"]; exists { - if operationStr, ok := operationVal.(string); ok { - cfg.Operation = &operationStr - } - } - }) -} diff --git a/pkg/workflow/update_issue_helpers.go b/pkg/workflow/update_issue_helpers.go new file mode 100644 index 00000000000..da18801a437 --- /dev/null +++ b/pkg/workflow/update_issue_helpers.go @@ -0,0 +1,36 @@ +// This file provides configuration types and parsing for the update-issue safe output. +// +// For shared update entity infrastructure (types, generic parsers, field parsing modes), +// see update_entity_helpers.go. + +package workflow + +import "github.com/github/gh-aw/pkg/logger" + +var updateIssueLog = logger.New("workflow:update_issue") + +// UpdateIssuesConfig holds configuration for updating GitHub issues from agent output +type UpdateIssuesConfig struct { + UpdateEntityConfig `yaml:",inline"` + Status *bool `yaml:"status,omitempty"` // Allow updating issue status (open/closed) - presence indicates field can be updated + Title *bool `yaml:"title,omitempty"` // Allow updating issue title - presence indicates field can be updated + Body *bool `yaml:"body,omitempty"` // Allow updating issue body - boolean value controls permission (defaults to true) + Footer *string `yaml:"footer,omitempty"` // Controls whether AI-generated footer is added. When false, visible footer is omitted but XML markers are kept. + TitlePrefix string `yaml:"title-prefix,omitempty"` // Required title prefix for issue validation - only issues with this prefix can be updated +} + +// parseUpdateIssuesConfig handles update-issue configuration +func (c *Compiler) parseUpdateIssuesConfig(outputMap map[string]any) *UpdateIssuesConfig { + return parseUpdateEntityConfigTyped(c, outputMap, + UpdateEntityIssue, "update-issue", updateIssueLog, + func(cfg *UpdateIssuesConfig) []UpdateEntityFieldSpec { + return []UpdateEntityFieldSpec{ + {Name: "status", Mode: FieldParsingKeyExistence, Dest: &cfg.Status}, + {Name: "title", Mode: FieldParsingKeyExistence, Dest: &cfg.Title}, + {Name: "body", Mode: FieldParsingBoolValue, Dest: &cfg.Body}, + {Name: "footer", Mode: FieldParsingTemplatableBool, StringDest: &cfg.Footer}, + } + }, func(configMap map[string]any, cfg *UpdateIssuesConfig) { + cfg.TitlePrefix = parseTitlePrefixFromConfig(configMap) + }) +} diff --git a/pkg/workflow/update_pull_request_helpers.go b/pkg/workflow/update_pull_request_helpers.go new file mode 100644 index 00000000000..058e783e524 --- /dev/null +++ b/pkg/workflow/update_pull_request_helpers.go @@ -0,0 +1,41 @@ +// This file provides configuration types and parsing for the update-pull-request safe output. +// +// For shared update entity infrastructure (types, generic parsers, field parsing modes), +// see update_entity_helpers.go. + +package workflow + +import "github.com/github/gh-aw/pkg/logger" + +var updatePullRequestLog = logger.New("workflow:update_pull_request") + +// UpdatePullRequestsConfig holds configuration for updating GitHub pull requests from agent output +type UpdatePullRequestsConfig struct { + UpdateEntityConfig `yaml:",inline"` + Title *bool `yaml:"title,omitempty"` // Allow updating PR title - defaults to true, set to false to disable + Body *bool `yaml:"body,omitempty"` // Allow updating PR body - defaults to true, set to false to disable + Operation *string `yaml:"operation,omitempty"` // Default operation for body updates: "append", "prepend", or "replace" (defaults to "replace") + Footer *string `yaml:"footer,omitempty"` // Controls whether AI-generated footer is added. When false, visible footer is omitted. +} + +// parseUpdatePullRequestsConfig handles update-pull-request configuration +func (c *Compiler) parseUpdatePullRequestsConfig(outputMap map[string]any) *UpdatePullRequestsConfig { + updatePullRequestLog.Print("Parsing update pull request configuration") + + return parseUpdateEntityConfigTyped(c, outputMap, + UpdateEntityPullRequest, "update-pull-request", updatePullRequestLog, + func(cfg *UpdatePullRequestsConfig) []UpdateEntityFieldSpec { + return []UpdateEntityFieldSpec{ + {Name: "title", Mode: FieldParsingBoolValue, Dest: &cfg.Title}, + {Name: "body", Mode: FieldParsingBoolValue, Dest: &cfg.Body}, + {Name: "footer", Mode: FieldParsingTemplatableBool, StringDest: &cfg.Footer}, + } + }, func(configMap map[string]any, cfg *UpdatePullRequestsConfig) { + // Parse operation field + if operationVal, exists := configMap["operation"]; exists { + if operationStr, ok := operationVal.(string); ok { + cfg.Operation = &operationStr + } + } + }) +}