diff --git a/.github/workflows/weekly-safe-outputs-spec-review.lock.yml b/.github/workflows/weekly-safe-outputs-spec-review.lock.yml index 9e36fff9d7..019d221205 100644 --- a/.github/workflows/weekly-safe-outputs-spec-review.lock.yml +++ b/.github/workflows/weekly-safe-outputs-spec-review.lock.yml @@ -1173,7 +1173,7 @@ jobs: uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 env: GH_AW_AGENT_OUTPUT: ${{ env.GH_AW_AGENT_OUTPUT }} - GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: "{\"create_pull_request\":{\"base_branch\":\"${{ github.base_ref || github.ref_name }}\",\"draft\":false,\"expires\":168,\"labels\":[\"documentation\",\"safe-outputs\",\"automation\"],\"max\":1,\"max_patch_size\":1024,\"title_prefix\":\"[spec-review] \"},\"missing_data\":{},\"missing_tool\":{}}" + GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: "{\"create_pull_request\":{\"auto_merge\":false,\"base_branch\":\"${{ github.base_ref || github.ref_name }}\",\"draft\":false,\"expires\":168,\"labels\":[\"documentation\",\"safe-outputs\",\"automation\"],\"max\":1,\"max_patch_size\":1024,\"title_prefix\":\"[spec-review] \"},\"missing_data\":{},\"missing_tool\":{}}" with: github-token: ${{ secrets.GH_AW_GITHUB_TOKEN || secrets.GITHUB_TOKEN }} script: | diff --git a/actions/setup/js/add_comment.cjs b/actions/setup/js/add_comment.cjs index 7911166a30..56c7690f08 100644 --- a/actions/setup/js/add_comment.cjs +++ b/actions/setup/js/add_comment.cjs @@ -10,6 +10,7 @@ const { getRepositoryUrl } = require("./get_repository_url.cjs"); const { replaceTemporaryIdReferences, loadTemporaryIdMapFromResolved, resolveRepoIssueTarget } = require("./temporary_id.cjs"); const { getTrackerID } = require("./get_tracker_id.cjs"); const { getErrorMessage } = require("./error_helpers.cjs"); +const { parseBoolTemplatable } = require("./templatable.cjs"); const { resolveTarget } = require("./safe_output_helpers.cjs"); const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs"); const { getMissingInfoSections } = require("./missing_messages_helper.cjs"); @@ -280,7 +281,7 @@ async function commentOnDiscussion(github, owner, repo, discussionNumber, messag */ async function main(config = {}) { // Extract configuration - const hideOlderCommentsEnabled = config.hide_older_comments === true; + const hideOlderCommentsEnabled = parseBoolTemplatable(config.hide_older_comments, false); const commentTarget = config.target || "triggering"; const maxCount = config.max || 20; const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config); diff --git a/actions/setup/js/assign_to_user.cjs b/actions/setup/js/assign_to_user.cjs index b80cd1c82d..b785d46113 100644 --- a/actions/setup/js/assign_to_user.cjs +++ b/actions/setup/js/assign_to_user.cjs @@ -10,6 +10,7 @@ const { getErrorMessage } = require("./error_helpers.cjs"); const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs"); const { resolveIssueNumber, extractAssignees } = require("./safe_output_helpers.cjs"); const { logStagedPreviewInfo } = require("./staged_preview.cjs"); +const { parseBoolTemplatable } = require("./templatable.cjs"); /** @type {string} Safe output type handled by this module */ const HANDLER_TYPE = "assign_to_user"; @@ -24,7 +25,7 @@ async function main(config = {}) { const allowedAssignees = config.allowed || []; const blockedAssignees = config.blocked || []; const maxCount = config.max || 10; - const unassignFirst = config.unassign_first || false; + const unassignFirst = parseBoolTemplatable(config.unassign_first, false); const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config); // Check if we're in staged mode diff --git a/actions/setup/js/create_discussion.cjs b/actions/setup/js/create_discussion.cjs index e8c831f155..71ee6ed31e 100644 --- a/actions/setup/js/create_discussion.cjs +++ b/actions/setup/js/create_discussion.cjs @@ -19,6 +19,7 @@ const { generateWorkflowIdMarker } = require("./generate_footer.cjs"); const { sanitizeLabelContent } = require("./sanitize_label_content.cjs"); const { tryEnforceArrayLimit } = require("./limit_enforcement_helpers.cjs"); const { logStagedPreviewInfo } = require("./staged_preview.cjs"); +const { parseBoolTemplatable } = require("./templatable.cjs"); /** * Maximum limits for discussion parameters to prevent resource exhaustion. @@ -306,8 +307,8 @@ async function main(config = {}) { const maxCount = config.max || 10; const expiresHours = config.expires ? parseInt(String(config.expires), 10) : 0; const fallbackToIssue = config.fallback_to_issue !== false; // Default to true - const closeOlderDiscussions = config.close_older_discussions === true || config.close_older_discussions === "true"; - const includeFooter = config.footer !== false; // Default to true (include footer) + const closeOlderDiscussions = parseBoolTemplatable(config.close_older_discussions, false); + const includeFooter = parseBoolTemplatable(config.footer, true); // Check if we're in staged mode const isStaged = process.env.GH_AW_SAFE_OUTPUTS_STAGED === "true"; diff --git a/actions/setup/js/create_issue.cjs b/actions/setup/js/create_issue.cjs index 905ffa95dc..79b75c53bb 100644 --- a/actions/setup/js/create_issue.cjs +++ b/actions/setup/js/create_issue.cjs @@ -38,6 +38,7 @@ const { renderTemplate } = require("./messages_core.cjs"); const { createExpirationLine, addExpirationToFooter } = require("./ephemerals.cjs"); const { MAX_SUB_ISSUES, getSubIssueCount } = require("./sub_issue_helpers.cjs"); const { closeOlderIssues } = require("./close_older_issues.cjs"); +const { parseBoolTemplatable } = require("./templatable.cjs"); const { tryEnforceArrayLimit } = require("./limit_enforcement_helpers.cjs"); const fs = require("fs"); const { logStagedPreviewInfo } = require("./staged_preview.cjs"); @@ -209,9 +210,9 @@ async function main(config = {}) { const expiresHours = config.expires ? parseInt(String(config.expires), 10) : 0; const maxCount = config.max ?? 10; const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config); - const groupEnabled = config.group === true || config.group === "true"; - const closeOlderIssuesEnabled = config.close_older_issues === true || config.close_older_issues === "true"; - const includeFooter = config.footer !== false; // Default to true (include footer) + const groupEnabled = parseBoolTemplatable(config.group, false); + const closeOlderIssuesEnabled = parseBoolTemplatable(config.close_older_issues, false); + const includeFooter = parseBoolTemplatable(config.footer, true); // Check if copilot assignment is enabled const assignCopilot = process.env.GH_AW_ASSIGN_COPILOT === "true"; diff --git a/actions/setup/js/create_pull_request.cjs b/actions/setup/js/create_pull_request.cjs index fdb211a12f..f838019643 100644 --- a/actions/setup/js/create_pull_request.cjs +++ b/actions/setup/js/create_pull_request.cjs @@ -95,14 +95,14 @@ async function main(config = {}) { const envLabels = config.labels ? (Array.isArray(config.labels) ? config.labels : config.labels.split(",")).map(label => String(label).trim()).filter(label => label) : []; const draftDefault = parseBoolTemplatable(config.draft, true); const ifNoChanges = config.if_no_changes || "warn"; - const allowEmpty = config.allow_empty || false; - const autoMerge = config.auto_merge || false; + const allowEmpty = parseBoolTemplatable(config.allow_empty, false); + const autoMerge = parseBoolTemplatable(config.auto_merge, false); const expiresHours = config.expires ? parseInt(String(config.expires), 10) : 0; const maxCount = config.max || 1; // PRs are typically limited to 1 let baseBranch = config.base_branch || ""; const maxSizeKb = config.max_patch_size ? parseInt(String(config.max_patch_size), 10) : 1024; const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config); - const includeFooter = config.footer !== false; // Default to true (include footer) + const includeFooter = parseBoolTemplatable(config.footer, true); const fallbackAsIssue = config.fallback_as_issue !== false; // Default to true (fallback enabled) // Environment validation - fail early if required variables are missing diff --git a/actions/setup/js/reply_to_pr_review_comment.cjs b/actions/setup/js/reply_to_pr_review_comment.cjs index b4cde99801..f47aeed333 100644 --- a/actions/setup/js/reply_to_pr_review_comment.cjs +++ b/actions/setup/js/reply_to_pr_review_comment.cjs @@ -11,6 +11,7 @@ const { generateFooterWithMessages } = require("./messages_footer.cjs"); const { sanitizeContent } = require("./sanitize_content.cjs"); const { getPRNumber } = require("./update_context_helpers.cjs"); const { logStagedPreviewInfo } = require("./staged_preview.cjs"); +const { parseBoolTemplatable } = require("./templatable.cjs"); /** * Type constant for handler identification @@ -30,7 +31,7 @@ async function main(config = {}) { // Extract configuration const maxCount = config.max || 10; const replyTarget = config.target || "triggering"; - const includeFooter = config.footer !== false; // Default to true + const includeFooter = parseBoolTemplatable(config.footer, true); const isStaged = process.env.GH_AW_SAFE_OUTPUTS_STAGED === "true"; const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config); diff --git a/actions/setup/js/update_discussion.cjs b/actions/setup/js/update_discussion.cjs index d7463339c8..036bed7371 100644 --- a/actions/setup/js/update_discussion.cjs +++ b/actions/setup/js/update_discussion.cjs @@ -9,6 +9,7 @@ const { isDiscussionContext, getDiscussionNumber } = require("./update_context_h const { createUpdateHandlerFactory, createStandardFormatResult } = require("./update_handler_factory.cjs"); const { sanitizeTitle } = require("./sanitize_title.cjs"); const { ERR_NOT_FOUND } = require("./error_codes.cjs"); +const { parseBoolTemplatable } = require("./templatable.cjs"); /** * Execute the discussion update API call using GraphQL @@ -136,7 +137,7 @@ function buildDiscussionUpdateData(item, config) { } // Pass footer config to executeUpdate (default to true) - updateData._includeFooter = config.footer !== false; + updateData._includeFooter = parseBoolTemplatable(config.footer, true); return { success: true, data: updateData }; } diff --git a/actions/setup/js/update_issue.cjs b/actions/setup/js/update_issue.cjs index 6208744d96..7a99890b34 100644 --- a/actions/setup/js/update_issue.cjs +++ b/actions/setup/js/update_issue.cjs @@ -15,6 +15,7 @@ const { loadTemporaryProjectMap, replaceTemporaryProjectReferences } = require(" const { sanitizeTitle } = require("./sanitize_title.cjs"); const { tryEnforceArrayLimit } = require("./limit_enforcement_helpers.cjs"); const { ERR_VALIDATION } = require("./error_codes.cjs"); +const { parseBoolTemplatable } = require("./templatable.cjs"); /** * Maximum limits for issue update parameters to prevent resource exhaustion. @@ -169,7 +170,7 @@ function buildIssueUpdateData(item, config) { } // Pass footer config to executeUpdate (default to true) - updateData._includeFooter = config.footer !== false; + updateData._includeFooter = parseBoolTemplatable(config.footer, true); // Store title prefix for validation in executeIssueUpdate if (config.title_prefix) { diff --git a/actions/setup/js/update_pull_request.cjs b/actions/setup/js/update_pull_request.cjs index 5d6f14b697..3cde54dec0 100644 --- a/actions/setup/js/update_pull_request.cjs +++ b/actions/setup/js/update_pull_request.cjs @@ -12,6 +12,7 @@ const { updateBody } = require("./update_pr_description_helpers.cjs"); const { resolveTarget } = require("./safe_output_helpers.cjs"); const { createUpdateHandlerFactory, createStandardResolveNumber, createStandardFormatResult } = require("./update_handler_factory.cjs"); const { sanitizeTitle } = require("./sanitize_title.cjs"); +const { parseBoolTemplatable } = require("./templatable.cjs"); /** * Execute the pull request update API call @@ -132,7 +133,7 @@ function buildPRUpdateData(item, config) { } // Pass footer config to executeUpdate (default to true) - updateData._includeFooter = config.footer !== false; + updateData._includeFooter = parseBoolTemplatable(config.footer, true); return { success: true, data: updateData }; } diff --git a/actions/setup/js/update_release.cjs b/actions/setup/js/update_release.cjs index d69a0dd806..2462b20817 100644 --- a/actions/setup/js/update_release.cjs +++ b/actions/setup/js/update_release.cjs @@ -12,6 +12,7 @@ const { getErrorMessage } = require("./error_helpers.cjs"); const { updateBody } = require("./update_pr_description_helpers.cjs"); const { logStagedPreviewInfo } = require("./staged_preview.cjs"); const { ERR_API, ERR_CONFIG, ERR_VALIDATION } = require("./error_codes.cjs"); +const { parseBoolTemplatable } = require("./templatable.cjs"); // Content sanitization: message.body is sanitized by updateBody() helper /** @@ -27,7 +28,7 @@ async function main(config = {}) { // Check if we're in staged mode const isStaged = process.env.GH_AW_SAFE_OUTPUTS_STAGED === "true"; const workflowName = process.env.GH_AW_WORKFLOW_NAME || "GitHub Agentic Workflow"; - const includeFooter = config.footer !== false; // Default to true (include footer) + const includeFooter = parseBoolTemplatable(config.footer, true); /** * Process a single update-release message diff --git a/pkg/workflow/add_comment.go b/pkg/workflow/add_comment.go index 32aef2b101..c2a853feda 100644 --- a/pkg/workflow/add_comment.go +++ b/pkg/workflow/add_comment.go @@ -21,7 +21,7 @@ type AddCommentsConfig struct { TargetRepoSlug string `yaml:"target-repo,omitempty"` // Target repository in format "owner/repo" for cross-repository comments AllowedRepos []string `yaml:"allowed-repos,omitempty"` // List of additional repositories that comments can be added to (additionally to the target-repo) Discussion *bool `yaml:"discussion,omitempty"` // Target discussion comments instead of issue/PR comments. Must be true if present. - HideOlderComments bool `yaml:"hide-older-comments,omitempty"` // When true, minimizes/hides all previous comments from the same workflow before creating the new comment + HideOlderComments *string `yaml:"hide-older-comments,omitempty"` // When true, minimizes/hides all previous comments from the same workflow before creating the new comment AllowedReasons []string `yaml:"allowed-reasons,omitempty"` // List of allowed reasons for hiding older comments (default: all reasons allowed) Discussions *bool `yaml:"discussions,omitempty"` // When false, excludes discussions:write permission. Default (nil or true) includes discussions:write for GitHub Apps with Discussions permission. } @@ -55,9 +55,7 @@ func (c *Compiler) buildCreateOutputAddCommentJob(data *WorkflowData, mainJobNam customEnvVars = append(customEnvVars, " GITHUB_AW_COMMENT_DISCUSSION: \"true\"\n") } // Pass the hide-older-comments flag configuration - if data.SafeOutputs.AddComments.HideOlderComments { - customEnvVars = append(customEnvVars, " GH_AW_HIDE_OLDER_COMMENTS: \"true\"\n") - } + customEnvVars = append(customEnvVars, buildTemplatableBoolEnvVar("GH_AW_HIDE_OLDER_COMMENTS", data.SafeOutputs.AddComments.HideOlderComments)...) // Pass the allowed-reasons list configuration if len(data.SafeOutputs.AddComments.AllowedReasons) > 0 { reasonsJSON, err := json.Marshal(data.SafeOutputs.AddComments.AllowedReasons) @@ -151,6 +149,15 @@ func (c *Compiler) parseCommentsConfig(outputMap map[string]any) *AddCommentsCon addCommentLog.Print("Parsing add-comment configuration") + // Get config data for pre-processing before YAML unmarshaling + configData, _ := outputMap["add-comment"].(map[string]any) + + // Pre-process templatable bool fields + if err := preprocessBoolFieldAsString(configData, "hide-older-comments", addCommentLog); err != nil { + addCommentLog.Printf("Invalid hide-older-comments value: %v", err) + return nil + } + // Unmarshal into typed config struct var config AddCommentsConfig if err := unmarshalConfig(outputMap, "add-comment", &config, addCommentLog); err != nil { diff --git a/pkg/workflow/add_comment_target_repo_test.go b/pkg/workflow/add_comment_target_repo_test.go index 0d4feae180..6ba7bede49 100644 --- a/pkg/workflow/add_comment_target_repo_test.go +++ b/pkg/workflow/add_comment_target_repo_test.go @@ -98,7 +98,7 @@ func TestAddCommentsConfigHideOlderComments(t *testing.T) { tests := []struct { name string configMap map[string]any - expectedHideOlderComments bool + expectedHideOlderComments *string }{ { name: "hide-older-comments enabled", @@ -108,7 +108,7 @@ func TestAddCommentsConfigHideOlderComments(t *testing.T) { "hide-older-comments": true, }, }, - expectedHideOlderComments: true, + expectedHideOlderComments: testStringPtr("true"), }, { name: "hide-older-comments disabled", @@ -118,16 +118,16 @@ func TestAddCommentsConfigHideOlderComments(t *testing.T) { "hide-older-comments": false, }, }, - expectedHideOlderComments: false, + expectedHideOlderComments: testStringPtr("false"), }, { - name: "hide-older-comments not specified (default false)", + name: "hide-older-comments not specified (default nil)", configMap: map[string]any{ "add-comment": map[string]any{ "max": 1, }, }, - expectedHideOlderComments: false, + expectedHideOlderComments: nil, }, { name: "hide-older-comments with other fields", @@ -139,7 +139,7 @@ func TestAddCommentsConfigHideOlderComments(t *testing.T) { "hide-older-comments": true, }, }, - expectedHideOlderComments: true, + expectedHideOlderComments: testStringPtr("true"), }, } @@ -151,8 +151,16 @@ func TestAddCommentsConfigHideOlderComments(t *testing.T) { t.Fatal("Expected valid config, but got nil") } - if config.HideOlderComments != tt.expectedHideOlderComments { - t.Errorf("Expected HideOlderComments = %v, got %v", tt.expectedHideOlderComments, config.HideOlderComments) + if tt.expectedHideOlderComments == nil { + if config.HideOlderComments != nil { + t.Errorf("Expected HideOlderComments = nil, got %v", *config.HideOlderComments) + } + } else { + if config.HideOlderComments == nil { + t.Errorf("Expected HideOlderComments = %v, got nil", *tt.expectedHideOlderComments) + } else if *config.HideOlderComments != *tt.expectedHideOlderComments { + t.Errorf("Expected HideOlderComments = %v, got %v", *tt.expectedHideOlderComments, *config.HideOlderComments) + } } }) } diff --git a/pkg/workflow/assign_to_user.go b/pkg/workflow/assign_to_user.go index 38a6f4c6bd..79fcaf70b4 100644 --- a/pkg/workflow/assign_to_user.go +++ b/pkg/workflow/assign_to_user.go @@ -12,7 +12,7 @@ type AssignToUserConfig struct { SafeOutputTargetConfig `yaml:",inline"` Allowed []string `yaml:"allowed,omitempty"` // Optional list of allowed usernames. If omitted, any users are allowed. Blocked []string `yaml:"blocked,omitempty"` // Optional list of blocked usernames or patterns (e.g., "copilot", "*[bot]") - UnassignFirst bool `yaml:"unassign-first,omitempty"` // If true, unassign all current assignees before assigning new ones + UnassignFirst *string `yaml:"unassign-first,omitempty"` // If true, unassign all current assignees before assigning new ones } // parseAssignToUserConfig handles assign-to-user configuration @@ -24,6 +24,15 @@ func (c *Compiler) parseAssignToUserConfig(outputMap map[string]any) *AssignToUs assignToUserLog.Print("Parsing assign-to-user configuration") + // Get config data for pre-processing before YAML unmarshaling + configData, _ := outputMap["assign-to-user"].(map[string]any) + + // Pre-process templatable bool fields + if err := preprocessBoolFieldAsString(configData, "unassign-first", assignToUserLog); err != nil { + assignToUserLog.Printf("Invalid unassign-first value: %v", err) + return nil + } + // Unmarshal into typed config struct var config AssignToUserConfig if err := unmarshalConfig(outputMap, "assign-to-user", &config, assignToUserLog); err != nil { diff --git a/pkg/workflow/compile_outputs_pr_test.go b/pkg/workflow/compile_outputs_pr_test.go index e9604d1fd5..404994e052 100644 --- a/pkg/workflow/compile_outputs_pr_test.go +++ b/pkg/workflow/compile_outputs_pr_test.go @@ -649,7 +649,7 @@ This test verifies that auto-merge configuration is properly handled. } // Verify auto-merge is set to true - if !workflowData.SafeOutputs.CreatePullRequests.AutoMerge { + if workflowData.SafeOutputs.CreatePullRequests.AutoMerge == nil || *workflowData.SafeOutputs.CreatePullRequests.AutoMerge != "true" { t.Error("Expected auto-merge to be true") } diff --git a/pkg/workflow/compiler_safe_outputs_config.go b/pkg/workflow/compiler_safe_outputs_config.go index e26e0ab731..2d3aa7b5e4 100644 --- a/pkg/workflow/compiler_safe_outputs_config.go +++ b/pkg/workflow/compiler_safe_outputs_config.go @@ -9,14 +9,23 @@ import ( var compilerSafeOutputsConfigLog = logger.New("workflow:compiler_safe_outputs_config") -// getEffectiveFooter returns the effective footer value for a config -// If the local footer is set, use it; otherwise fall back to global footer -// Returns nil if neither is set (default to true in JavaScript) -func getEffectiveFooter(localFooter *bool, globalFooter *bool) *bool { +// getEffectiveFooterForTemplatable returns the effective footer as a templatable string. +// If the local string footer is set, use it; otherwise convert the global bool footer. +// Returns nil if neither is set (default to true in JavaScript). +func getEffectiveFooterForTemplatable(localFooter *string, globalFooter *bool) *string { if localFooter != nil { return localFooter } - return globalFooter + if globalFooter != nil { + var s string + if *globalFooter { + s = "true" + } else { + s = "false" + } + return &s + } + return nil } // getEffectiveFooterString returns the effective footer string value for a config. @@ -138,9 +147,9 @@ var handlerRegistry = map[string]handlerBuilder{ AddIfNotEmpty("title_prefix", c.TitlePrefix). AddStringSlice("assignees", c.Assignees). AddIfNotEmpty("target-repo", c.TargetRepoSlug). - AddIfTrue("group", c.Group). - AddIfTrue("close_older_issues", c.CloseOlderIssues). - AddBoolPtr("footer", getEffectiveFooter(c.Footer, cfg.Footer)). + AddTemplatableBool("group", c.Group). + AddTemplatableBool("close_older_issues", c.CloseOlderIssues). + AddTemplatableBool("footer", getEffectiveFooterForTemplatable(c.Footer, cfg.Footer)). Build() }, "add_comment": func(cfg *SafeOutputsConfig) map[string]any { @@ -151,7 +160,7 @@ var handlerRegistry = map[string]handlerBuilder{ return newHandlerConfigBuilder(). AddIfPositive("max", c.Max). AddIfNotEmpty("target", c.Target). - AddIfTrue("hide_older_comments", c.HideOlderComments). + AddTemplatableBool("hide_older_comments", c.HideOlderComments). AddIfNotEmpty("target-repo", c.TargetRepoSlug). AddStringSlice("allowed_repos", c.AllowedRepos). Build() @@ -168,12 +177,12 @@ var handlerRegistry = map[string]handlerBuilder{ AddStringSlice("labels", c.Labels). AddStringSlice("allowed_labels", c.AllowedLabels). AddStringSlice("allowed_repos", c.AllowedRepos). - AddIfTrue("close_older_discussions", c.CloseOlderDiscussions). + AddTemplatableBool("close_older_discussions", c.CloseOlderDiscussions). AddIfNotEmpty("required_category", c.RequiredCategory). AddIfPositive("expires", c.Expires). AddBoolPtr("fallback_to_issue", c.FallbackToIssue). AddIfNotEmpty("target-repo", c.TargetRepoSlug). - AddBoolPtr("footer", getEffectiveFooter(c.Footer, cfg.Footer)). + AddTemplatableBool("footer", getEffectiveFooterForTemplatable(c.Footer, cfg.Footer)). Build() }, "close_issue": func(cfg *SafeOutputsConfig) map[string]any { @@ -323,7 +332,7 @@ var handlerRegistry = map[string]handlerBuilder{ return builder. AddIfNotEmpty("target-repo", c.TargetRepoSlug). AddStringSlice("allowed_repos", c.AllowedRepos). - AddBoolPtr("footer", getEffectiveFooter(c.Footer, cfg.Footer)). + AddTemplatableBool("footer", getEffectiveFooterForTemplatable(c.Footer, cfg.Footer)). Build() }, "update_discussion": func(cfg *SafeOutputsConfig) map[string]any { @@ -348,7 +357,7 @@ var handlerRegistry = map[string]handlerBuilder{ AddStringSlice("allowed_labels", c.AllowedLabels). AddIfNotEmpty("target-repo", c.TargetRepoSlug). AddStringSlice("allowed_repos", c.AllowedRepos). - AddBoolPtr("footer", getEffectiveFooter(c.Footer, cfg.Footer)). + AddTemplatableBool("footer", getEffectiveFooterForTemplatable(c.Footer, cfg.Footer)). Build() }, "link_sub_issue": func(cfg *SafeOutputsConfig) map[string]any { @@ -373,7 +382,7 @@ var handlerRegistry = map[string]handlerBuilder{ c := cfg.UpdateRelease return newHandlerConfigBuilder(). AddIfPositive("max", c.Max). - AddBoolPtr("footer", getEffectiveFooter(c.Footer, cfg.Footer)). + AddTemplatableBool("footer", getEffectiveFooterForTemplatable(c.Footer, cfg.Footer)). Build() }, "create_pull_request_review_comment": func(cfg *SafeOutputsConfig) map[string]any { @@ -410,7 +419,7 @@ var handlerRegistry = map[string]handlerBuilder{ AddIfNotEmpty("target", c.Target). AddIfNotEmpty("target-repo", c.TargetRepoSlug). AddStringSlice("allowed_repos", c.AllowedRepos). - AddBoolPtr("footer", getEffectiveFooter(c.Footer, cfg.Footer)). + AddTemplatableBool("footer", getEffectiveFooterForTemplatable(c.Footer, cfg.Footer)). Build() }, "resolve_pull_request_review_thread": func(cfg *SafeOutputsConfig) map[string]any { @@ -438,13 +447,13 @@ var handlerRegistry = map[string]handlerBuilder{ AddStringSlice("reviewers", c.Reviewers). AddTemplatableBool("draft", c.Draft). AddIfNotEmpty("if_no_changes", c.IfNoChanges). - AddIfTrue("allow_empty", c.AllowEmpty). - AddIfTrue("auto_merge", c.AutoMerge). + AddTemplatableBool("allow_empty", c.AllowEmpty). + AddTemplatableBool("auto_merge", c.AutoMerge). AddIfPositive("expires", c.Expires). AddIfNotEmpty("target-repo", c.TargetRepoSlug). AddStringSlice("allowed_repos", c.AllowedRepos). AddDefault("max_patch_size", maxPatchSize). - AddBoolPtr("footer", getEffectiveFooter(c.Footer, cfg.Footer)). + AddTemplatableBool("footer", getEffectiveFooterForTemplatable(c.Footer, cfg.Footer)). AddBoolPtr("fallback_as_issue", c.FallbackAsIssue) // Add base_branch - use custom value if specified, otherwise use github.base_ref || github.ref_name // This handles PR contexts where github.ref_name is "123/merge" which is invalid as a target branch @@ -486,7 +495,7 @@ var handlerRegistry = map[string]handlerBuilder{ AddBoolPtrOrDefault("allow_title", c.Title, true). AddBoolPtrOrDefault("allow_body", c.Body, true). AddStringPtr("default_operation", c.Operation). - AddBoolPtr("footer", getEffectiveFooter(c.Footer, cfg.Footer)). + AddTemplatableBool("footer", getEffectiveFooterForTemplatable(c.Footer, cfg.Footer)). AddIfNotEmpty("target-repo", c.TargetRepoSlug). AddStringSlice("allowed_repos", c.AllowedRepos). Build() @@ -612,7 +621,7 @@ var handlerRegistry = map[string]handlerBuilder{ AddIfNotEmpty("target", c.Target). AddIfNotEmpty("target-repo", c.TargetRepoSlug). AddStringSlice("allowed_repos", c.AllowedRepos). - AddIfTrue("unassign_first", c.UnassignFirst). + AddTemplatableBool("unassign_first", c.UnassignFirst). Build() }, "unassign_from_user": func(cfg *SafeOutputsConfig) map[string]any { diff --git a/pkg/workflow/compiler_safe_outputs_config_test.go b/pkg/workflow/compiler_safe_outputs_config_test.go index 4b758a0979..15f87699d7 100644 --- a/pkg/workflow/compiler_safe_outputs_config_test.go +++ b/pkg/workflow/compiler_safe_outputs_config_test.go @@ -47,7 +47,7 @@ func TestAddHandlerManagerConfigEnvVar(t *testing.T) { Max: 3, }, Target: "issue", - HideOlderComments: true, + HideOlderComments: testStringPtr("true"), }, }, checkContains: []string{ @@ -66,7 +66,7 @@ func TestAddHandlerManagerConfigEnvVar(t *testing.T) { Category: "general", TitlePrefix: "[Discussion] ", Labels: []string{"ai"}, - CloseOlderDiscussions: true, + CloseOlderDiscussions: testStringPtr("true"), }, }, checkContains: []string{ @@ -134,7 +134,7 @@ func TestAddHandlerManagerConfigEnvVar(t *testing.T) { Labels: []string{"automated"}, Draft: testStringPtr("true"), IfNoChanges: "skip", - AllowEmpty: true, + AllowEmpty: testStringPtr("true"), Expires: 7, }, }, @@ -424,7 +424,7 @@ func TestHandlerConfigBooleanFields(t *testing.T) { name: "hide older comments", safeOutputs: &SafeOutputsConfig{ AddComments: &AddCommentsConfig{ - HideOlderComments: true, + HideOlderComments: testStringPtr("true"), }, }, checkField: "add_comment", @@ -435,7 +435,7 @@ func TestHandlerConfigBooleanFields(t *testing.T) { name: "close older discussions", safeOutputs: &SafeOutputsConfig{ CreateDiscussions: &CreateDiscussionsConfig{ - CloseOlderDiscussions: true, + CloseOlderDiscussions: testStringPtr("true"), }, }, checkField: "create_discussion", @@ -446,7 +446,7 @@ func TestHandlerConfigBooleanFields(t *testing.T) { name: "allow empty PR", safeOutputs: &SafeOutputsConfig{ CreatePullRequests: &CreatePullRequestsConfig{ - AllowEmpty: true, + AllowEmpty: testStringPtr("true"), }, }, checkField: "create_pull_request", @@ -972,7 +972,7 @@ func TestHandlerConfigAssignToUserWithUnassignFirst(t *testing.T) { BaseSafeOutputConfig: BaseSafeOutputConfig{ Max: 3, }, - UnassignFirst: true, + UnassignFirst: testStringPtr("true"), }, }, } diff --git a/pkg/workflow/create_discussion.go b/pkg/workflow/create_discussion.go index f7a3e2b28c..102a25e7a6 100644 --- a/pkg/workflow/create_discussion.go +++ b/pkg/workflow/create_discussion.go @@ -19,11 +19,11 @@ type CreateDiscussionsConfig struct { AllowedLabels []string `yaml:"allowed-labels,omitempty"` // Optional list of allowed labels. If omitted, any labels are allowed (including creating new ones). TargetRepoSlug string `yaml:"target-repo,omitempty"` // Target repository in format "owner/repo" for cross-repository discussions AllowedRepos []string `yaml:"allowed-repos,omitempty"` // List of additional repositories that discussions can be created in - CloseOlderDiscussions bool `yaml:"close-older-discussions,omitempty"` // When true, close older discussions with same title prefix or labels as outdated + CloseOlderDiscussions *string `yaml:"close-older-discussions,omitempty"` // When true, close older discussions with same title prefix or labels as outdated RequiredCategory string `yaml:"required-category,omitempty"` // Required category for matching when close-older-discussions is enabled Expires int `yaml:"expires,omitempty"` // Hours until the discussion expires and should be automatically closed FallbackToIssue *bool `yaml:"fallback-to-issue,omitempty"` // When true (default), fallback to create-issue if discussion creation fails due to permissions. - Footer *bool `yaml:"footer,omitempty"` // Controls whether AI-generated footer is added. When false, visible footer is omitted but XML markers are kept. + Footer *string `yaml:"footer,omitempty"` // Controls whether AI-generated footer is added. When false, visible footer is omitted but XML markers are kept. } // parseDiscussionsConfig handles create-discussion configuration @@ -41,6 +41,14 @@ func (c *Compiler) parseDiscussionsConfig(outputMap map[string]any) *CreateDiscu // Pre-process the expires field (convert to hours before unmarshaling) expiresDisabled := preprocessExpiresField(configData, discussionLog) + // Pre-process templatable bool fields + for _, field := range []string{"close-older-discussions", "footer"} { + if err := preprocessBoolFieldAsString(configData, field, discussionLog); err != nil { + discussionLog.Printf("Invalid %s value: %v", field, err) + return nil + } + } + // Unmarshal into typed config struct var config CreateDiscussionsConfig if err := unmarshalConfig(outputMap, "create-discussion", &config, discussionLog); err != nil { @@ -97,8 +105,8 @@ func (c *Compiler) parseDiscussionsConfig(outputMap map[string]any) *CreateDiscu if len(config.AllowedRepos) > 0 { discussionLog.Printf("Allowed repos configured: %v", config.AllowedRepos) } - if config.CloseOlderDiscussions { - discussionLog.Print("Close older discussions enabled") + if config.CloseOlderDiscussions != nil { + discussionLog.Print("Close older discussions flag set") if config.RequiredCategory != "" { discussionLog.Printf("Required category for close older discussions: %q", config.RequiredCategory) } @@ -129,10 +137,8 @@ func (c *Compiler) buildCreateOutputDiscussionJob(data *WorkflowData, mainJobNam customEnvVars = append(customEnvVars, buildLabelsEnvVar("GH_AW_DISCUSSION_ALLOWED_LABELS", data.SafeOutputs.CreateDiscussions.AllowedLabels)...) customEnvVars = append(customEnvVars, buildAllowedReposEnvVar("GH_AW_ALLOWED_REPOS", data.SafeOutputs.CreateDiscussions.AllowedRepos)...) - // Add close-older-discussions flag if enabled - if data.SafeOutputs.CreateDiscussions.CloseOlderDiscussions { - customEnvVars = append(customEnvVars, " GH_AW_CLOSE_OLDER_DISCUSSIONS: \"true\"\n") - } + // Add close-older-discussions flag if set + customEnvVars = append(customEnvVars, buildTemplatableBoolEnvVar("GH_AW_CLOSE_OLDER_DISCUSSIONS", data.SafeOutputs.CreateDiscussions.CloseOlderDiscussions)...) // Add expires value if set if data.SafeOutputs.CreateDiscussions.Expires > 0 { @@ -148,7 +154,7 @@ func (c *Compiler) buildCreateOutputDiscussionJob(data *WorkflowData, mainJobNam } // Add footer flag if explicitly set to false - if data.SafeOutputs.CreateDiscussions.Footer != nil && !*data.SafeOutputs.CreateDiscussions.Footer { + if data.SafeOutputs.CreateDiscussions.Footer != nil && *data.SafeOutputs.CreateDiscussions.Footer == "false" { customEnvVars = append(customEnvVars, " GH_AW_FOOTER: \"false\"\n") discussionLog.Print("Footer disabled - XML markers will be included but visible footer content will be omitted") } diff --git a/pkg/workflow/create_issue.go b/pkg/workflow/create_issue.go index a849cd16ee..940b60fcd3 100644 --- a/pkg/workflow/create_issue.go +++ b/pkg/workflow/create_issue.go @@ -17,10 +17,10 @@ type CreateIssuesConfig struct { Assignees []string `yaml:"assignees,omitempty"` // List of users/bots to assign the issue to TargetRepoSlug string `yaml:"target-repo,omitempty"` // Target repository in format "owner/repo" for cross-repository issues AllowedRepos []string `yaml:"allowed-repos,omitempty"` // List of additional repositories that issues can be created in - CloseOlderIssues bool `yaml:"close-older-issues,omitempty"` // When true, close older issues with same title prefix or labels as "not planned" + CloseOlderIssues *string `yaml:"close-older-issues,omitempty"` // When true, close older issues with same title prefix or labels as "not planned" Expires int `yaml:"expires,omitempty"` // Hours until the issue expires and should be automatically closed - Group bool `yaml:"group,omitempty"` // If true, group issues as sub-issues under a parent issue (workflow ID is used as group identifier) - Footer *bool `yaml:"footer,omitempty"` // Controls whether AI-generated footer is added. When false, visible footer is omitted but XML markers are kept. + Group *string `yaml:"group,omitempty"` // If true, group issues as sub-issues under a parent issue (workflow ID is used as group identifier) + Footer *string `yaml:"footer,omitempty"` // Controls whether AI-generated footer is added. When false, visible footer is omitted but XML markers are kept. } // parseIssuesConfig handles create-issue configuration @@ -38,6 +38,15 @@ func (c *Compiler) parseIssuesConfig(outputMap map[string]any) *CreateIssuesConf // Pre-process the expires field (convert to hours before unmarshaling) expiresDisabled := preprocessExpiresField(configData, createIssueLog) + // Pre-process templatable bool fields: convert literal booleans to strings so that + // GitHub Actions expression strings (e.g. "${{ inputs.close-older-issues }}") are also accepted. + for _, field := range []string{"close-older-issues", "group", "footer"} { + if err := preprocessBoolFieldAsString(configData, field, createIssueLog); err != nil { + createIssueLog.Printf("Invalid %s value: %v", field, err) + return nil + } + } + // Unmarshal into typed config struct var config CreateIssuesConfig if err := unmarshalConfig(outputMap, "create-issue", &config, createIssueLog); err != nil { @@ -150,19 +159,19 @@ func (c *Compiler) buildCreateOutputIssueJob(data *WorkflowData, mainJobName str } // Add group flag if set - if data.SafeOutputs.CreateIssues.Group { - customEnvVars = append(customEnvVars, " GH_AW_ISSUE_GROUP: \"true\"\n") - createIssueLog.Print("Issue grouping enabled - issues will be grouped as sub-issues under parent") + customEnvVars = append(customEnvVars, buildTemplatableBoolEnvVar("GH_AW_ISSUE_GROUP", data.SafeOutputs.CreateIssues.Group)...) + if data.SafeOutputs.CreateIssues.Group != nil { + createIssueLog.Print("Issue grouping flag set") } // Add close-older-issues flag if enabled - if data.SafeOutputs.CreateIssues.CloseOlderIssues { - customEnvVars = append(customEnvVars, " GH_AW_CLOSE_OLDER_ISSUES: \"true\"\n") - createIssueLog.Print("Close older issues enabled - older issues with same title prefix or labels will be closed") + customEnvVars = append(customEnvVars, buildTemplatableBoolEnvVar("GH_AW_CLOSE_OLDER_ISSUES", data.SafeOutputs.CreateIssues.CloseOlderIssues)...) + if data.SafeOutputs.CreateIssues.CloseOlderIssues != nil { + createIssueLog.Print("Close older issues flag set") } // Add footer flag if explicitly set to false - if data.SafeOutputs.CreateIssues.Footer != nil && !*data.SafeOutputs.CreateIssues.Footer { + if data.SafeOutputs.CreateIssues.Footer != nil && *data.SafeOutputs.CreateIssues.Footer == "false" { customEnvVars = append(customEnvVars, " GH_AW_FOOTER: \"false\"\n") createIssueLog.Print("Footer disabled - XML markers will be included but visible footer content will be omitted") } diff --git a/pkg/workflow/create_issue_group_test.go b/pkg/workflow/create_issue_group_test.go index c1ab54b20a..5954911dfe 100644 --- a/pkg/workflow/create_issue_group_test.go +++ b/pkg/workflow/create_issue_group_test.go @@ -14,10 +14,11 @@ import ( // TestCreateIssueGroupFieldParsing verifies that the group field is parsed correctly func TestCreateIssueGroupFieldParsing(t *testing.T) { + strPtr := func(s string) *string { return &s } tests := []struct { name string frontmatter string - expectedGroup bool + expectedGroup *string }{ { name: "group enabled with true", @@ -34,7 +35,7 @@ safe-outputs: --- Test content`, - expectedGroup: true, + expectedGroup: strPtr("true"), }, { name: "group disabled with false", @@ -51,10 +52,10 @@ safe-outputs: --- Test content`, - expectedGroup: false, + expectedGroup: strPtr("false"), }, { - name: "group not specified defaults to false", + name: "group not specified defaults to nil", frontmatter: `--- name: Test Workflow on: workflow_dispatch @@ -67,7 +68,7 @@ safe-outputs: --- Test content`, - expectedGroup: false, + expectedGroup: nil, }, } diff --git a/pkg/workflow/create_pull_request.go b/pkg/workflow/create_pull_request.go index 687c2960eb..f7878a7bb3 100644 --- a/pkg/workflow/create_pull_request.go +++ b/pkg/workflow/create_pull_request.go @@ -2,7 +2,6 @@ package workflow import ( "fmt" - "strings" "github.com/github/gh-aw/pkg/constants" "github.com/github/gh-aw/pkg/logger" @@ -27,13 +26,13 @@ type CreatePullRequestsConfig struct { Reviewers []string `yaml:"reviewers,omitempty"` // List of users/bots to assign as reviewers to the pull request Draft *string `yaml:"draft,omitempty"` // Pointer to distinguish between unset (nil), literal bool, and expression values IfNoChanges string `yaml:"if-no-changes,omitempty"` // Behavior when no changes to push: "warn" (default), "error", or "ignore" - AllowEmpty bool `yaml:"allow-empty,omitempty"` // Allow creating PR without patch file or with empty patch (useful for preparing feature branches) + AllowEmpty *string `yaml:"allow-empty,omitempty"` // Allow creating PR without patch file or with empty patch (useful for preparing feature branches) TargetRepoSlug string `yaml:"target-repo,omitempty"` // Target repository in format "owner/repo" for cross-repository pull requests AllowedRepos []string `yaml:"allowed-repos,omitempty"` // List of additional repositories that pull requests can be created in (additionally to the target-repo) Expires int `yaml:"expires,omitempty"` // Hours until the pull request expires and should be automatically closed (only for same-repo PRs) - AutoMerge bool `yaml:"auto-merge,omitempty"` // Enable auto-merge for the pull request when all required checks pass + AutoMerge *string `yaml:"auto-merge,omitempty"` // Enable auto-merge for the pull request when all required checks pass BaseBranch string `yaml:"base-branch,omitempty"` // Base branch for the pull request (defaults to github.ref_name if not specified) - Footer *bool `yaml:"footer,omitempty"` // Controls whether AI-generated footer is added. When false, visible footer is omitted but XML markers are kept. + Footer *string `yaml:"footer,omitempty"` // Controls whether AI-generated footer is added. When false, visible footer is omitted but XML markers are kept. FallbackAsIssue *bool `yaml:"fallback-as-issue,omitempty"` // When true (default), creates an issue if PR creation fails. When false, no fallback occurs and issues: write permission is not requested. } @@ -106,13 +105,7 @@ func (c *Compiler) buildCreateOutputPullRequestJob(data *WorkflowData, mainJobNa customEnvVars = append(customEnvVars, buildLabelsEnvVar("GH_AW_PR_ALLOWED_LABELS", data.SafeOutputs.CreatePullRequests.AllowedLabels)...) // Pass draft setting - default to true for backwards compatibility if data.SafeOutputs.CreatePullRequests.Draft != nil { - draftVal := *data.SafeOutputs.CreatePullRequests.Draft - if strings.HasPrefix(draftVal, "${{") { - // Expression value - embed unquoted so GitHub Actions evaluates it - customEnvVars = append(customEnvVars, fmt.Sprintf(" GH_AW_PR_DRAFT: %s\n", draftVal)) - } else { - customEnvVars = append(customEnvVars, fmt.Sprintf(" GH_AW_PR_DRAFT: %q\n", draftVal)) - } + customEnvVars = append(customEnvVars, buildTemplatableBoolEnvVar("GH_AW_PR_DRAFT", data.SafeOutputs.CreatePullRequests.Draft)...) } else { customEnvVars = append(customEnvVars, " GH_AW_PR_DRAFT: \"true\"\n") } @@ -125,10 +118,18 @@ func (c *Compiler) buildCreateOutputPullRequestJob(data *WorkflowData, mainJobNa customEnvVars = append(customEnvVars, fmt.Sprintf(" GH_AW_PR_IF_NO_CHANGES: %q\n", ifNoChanges)) // Pass the allow-empty configuration - customEnvVars = append(customEnvVars, fmt.Sprintf(" GH_AW_PR_ALLOW_EMPTY: %q\n", fmt.Sprintf("%t", data.SafeOutputs.CreatePullRequests.AllowEmpty))) + if data.SafeOutputs.CreatePullRequests.AllowEmpty != nil { + customEnvVars = append(customEnvVars, buildTemplatableBoolEnvVar("GH_AW_PR_ALLOW_EMPTY", data.SafeOutputs.CreatePullRequests.AllowEmpty)...) + } else { + customEnvVars = append(customEnvVars, " GH_AW_PR_ALLOW_EMPTY: \"false\"\n") + } // Pass the auto-merge configuration - customEnvVars = append(customEnvVars, fmt.Sprintf(" GH_AW_PR_AUTO_MERGE: %q\n", fmt.Sprintf("%t", data.SafeOutputs.CreatePullRequests.AutoMerge))) + if data.SafeOutputs.CreatePullRequests.AutoMerge != nil { + customEnvVars = append(customEnvVars, buildTemplatableBoolEnvVar("GH_AW_PR_AUTO_MERGE", data.SafeOutputs.CreatePullRequests.AutoMerge)...) + } else { + customEnvVars = append(customEnvVars, " GH_AW_PR_AUTO_MERGE: \"false\"\n") + } // Pass the fallback-as-issue configuration - default to true for backwards compatibility if data.SafeOutputs.CreatePullRequests.FallbackAsIssue != nil { @@ -157,7 +158,7 @@ func (c *Compiler) buildCreateOutputPullRequestJob(data *WorkflowData, mainJobNa } // Add footer flag if explicitly set to false - if data.SafeOutputs.CreatePullRequests.Footer != nil && !*data.SafeOutputs.CreatePullRequests.Footer { + if data.SafeOutputs.CreatePullRequests.Footer != nil && *data.SafeOutputs.CreatePullRequests.Footer == "false" { customEnvVars = append(customEnvVars, " GH_AW_FOOTER: \"false\"\n") createPRLog.Print("Footer disabled - XML markers will be included but visible footer content will be omitted") } @@ -266,9 +267,11 @@ func (c *Compiler) parsePullRequestsConfig(outputMap map[string]any) *CreatePull // Pre-process templatable bool fields: convert literal booleans to strings so that // GitHub Actions expression strings (e.g. "${{ inputs.draft-prs }}") are also accepted. - if err := preprocessBoolFieldAsString(configData, "draft", createPRLog); err != nil { - createPRLog.Printf("Invalid draft value: %v", err) - return nil + for _, field := range []string{"draft", "allow-empty", "auto-merge", "footer"} { + if err := preprocessBoolFieldAsString(configData, field, createPRLog); err != nil { + createPRLog.Printf("Invalid %s value: %v", field, err) + return nil + } } // Unmarshal into typed config struct diff --git a/pkg/workflow/noop.go b/pkg/workflow/noop.go index 029823ca87..c7af28aab6 100644 --- a/pkg/workflow/noop.go +++ b/pkg/workflow/noop.go @@ -9,7 +9,7 @@ var noopLog = logger.New("workflow:noop") // NoOpConfig holds configuration for no-op safe output (logging only) type NoOpConfig struct { BaseSafeOutputConfig `yaml:",inline"` - ReportAsIssue bool `yaml:"report-as-issue,omitempty"` // Controls whether noop runs are reported as issue comments (default: true) + ReportAsIssue *string `yaml:"report-as-issue,omitempty"` // Controls whether noop runs are reported as issue comments (default: true) } // parseNoOpConfig handles noop configuration @@ -30,26 +30,34 @@ func (c *Compiler) parseNoOpConfig(outputMap map[string]any) *NoOpConfig { // Set default max for noop messages noopConfig.Max = 1 // Set default report-as-issue to true - noopConfig.ReportAsIssue = true + trueVal := "true" + noopConfig.ReportAsIssue = &trueVal noopLog.Print("Noop enabled with default max=1, report-as-issue=true") return noopConfig } if configMap, ok := configData.(map[string]any); ok { + // Pre-process report-as-issue as a templatable bool + if err := preprocessBoolFieldAsString(configMap, "report-as-issue", noopLog); err != nil { + noopLog.Printf("Invalid report-as-issue value: %v", err) + return nil + } + // Parse common base fields with default max of 1 c.parseBaseSafeOutputConfig(configMap, &noopConfig.BaseSafeOutputConfig, 1) // Parse report-as-issue field with default of true - if reportAsIssue, ok := configMap["report-as-issue"].(bool); ok { - noopConfig.ReportAsIssue = reportAsIssue - noopLog.Printf("report-as-issue explicitly set to: %t", reportAsIssue) + if reportAsIssue, ok := configMap["report-as-issue"].(string); ok { + noopConfig.ReportAsIssue = &reportAsIssue + noopLog.Printf("report-as-issue explicitly set to: %s", reportAsIssue) } else { // Default to true - noopConfig.ReportAsIssue = true + trueVal := "true" + noopConfig.ReportAsIssue = &trueVal noopLog.Print("report-as-issue not specified, defaulting to true") } - noopLog.Printf("Parsed noop configuration: max=%d, report-as-issue=%t", noopConfig.Max, noopConfig.ReportAsIssue) + noopLog.Printf("Parsed noop configuration: max=%d, report-as-issue=%s", noopConfig.Max, *noopConfig.ReportAsIssue) } return noopConfig diff --git a/pkg/workflow/noop_test.go b/pkg/workflow/noop_test.go index 9cd2f759ae..4cdc8fae1b 100644 --- a/pkg/workflow/noop_test.go +++ b/pkg/workflow/noop_test.go @@ -14,14 +14,14 @@ func TestParseNoOpConfig(t *testing.T) { outputMap map[string]any expectedNil bool expectedMax int - expectedReport bool + expectedReport *string }{ { name: "noop not present", outputMap: map[string]any{}, expectedNil: true, expectedMax: 0, - expectedReport: false, + expectedReport: nil, }, { name: "noop explicitly disabled with false", @@ -30,7 +30,7 @@ func TestParseNoOpConfig(t *testing.T) { }, expectedNil: true, expectedMax: 0, - expectedReport: false, + expectedReport: nil, }, { name: "noop enabled with nil value", @@ -39,7 +39,7 @@ func TestParseNoOpConfig(t *testing.T) { }, expectedNil: false, expectedMax: 1, - expectedReport: true, + expectedReport: testStringPtr("true"), }, { name: "noop with empty config object", @@ -48,7 +48,7 @@ func TestParseNoOpConfig(t *testing.T) { }, expectedNil: false, expectedMax: 1, - expectedReport: true, + expectedReport: testStringPtr("true"), }, { name: "noop with max specified", @@ -59,7 +59,7 @@ func TestParseNoOpConfig(t *testing.T) { }, expectedNil: false, expectedMax: 5, - expectedReport: true, + expectedReport: testStringPtr("true"), }, { name: "noop with report-as-issue set to true", @@ -70,7 +70,7 @@ func TestParseNoOpConfig(t *testing.T) { }, expectedNil: false, expectedMax: 1, - expectedReport: true, + expectedReport: testStringPtr("true"), }, { name: "noop with report-as-issue set to false", @@ -81,7 +81,7 @@ func TestParseNoOpConfig(t *testing.T) { }, expectedNil: false, expectedMax: 1, - expectedReport: false, + expectedReport: testStringPtr("false"), }, { name: "noop with max and report-as-issue", @@ -93,7 +93,7 @@ func TestParseNoOpConfig(t *testing.T) { }, expectedNil: false, expectedMax: 3, - expectedReport: false, + expectedReport: testStringPtr("false"), }, { name: "noop with report-as-issue not specified defaults to true", @@ -104,7 +104,7 @@ func TestParseNoOpConfig(t *testing.T) { }, expectedNil: false, expectedMax: 2, - expectedReport: true, + expectedReport: testStringPtr("true"), }, } @@ -118,7 +118,11 @@ func TestParseNoOpConfig(t *testing.T) { } else { assert.NotNil(t, result, "Expected non-nil NoOpConfig") assert.Equal(t, tt.expectedMax, result.Max, "Max value mismatch") - assert.Equal(t, tt.expectedReport, result.ReportAsIssue, "ReportAsIssue value mismatch") + if tt.expectedReport == nil { + assert.Nil(t, result.ReportAsIssue, "ReportAsIssue value mismatch") + } else { + assert.Equal(t, *tt.expectedReport, *result.ReportAsIssue, "ReportAsIssue value mismatch") + } } }) } diff --git a/pkg/workflow/notify_comment.go b/pkg/workflow/notify_comment.go index 274daa3197..647baceab2 100644 --- a/pkg/workflow/notify_comment.go +++ b/pkg/workflow/notify_comment.go @@ -214,10 +214,9 @@ func (c *Compiler) buildConclusionJob(data *WorkflowData, mainJobName string, sa if data.SafeOutputs.NoOp != nil { noopMessageEnvVars = append(noopMessageEnvVars, " GH_AW_NOOP_MESSAGE: ${{ steps.noop.outputs.noop_message }}\n") // Pass the report-as-issue configuration - if data.SafeOutputs.NoOp.ReportAsIssue { + noopMessageEnvVars = append(noopMessageEnvVars, buildTemplatableBoolEnvVar("GH_AW_NOOP_REPORT_AS_ISSUE", data.SafeOutputs.NoOp.ReportAsIssue)...) + if data.SafeOutputs.NoOp.ReportAsIssue == nil { noopMessageEnvVars = append(noopMessageEnvVars, " GH_AW_NOOP_REPORT_AS_ISSUE: \"true\"\n") - } else { - noopMessageEnvVars = append(noopMessageEnvVars, " GH_AW_NOOP_REPORT_AS_ISSUE: \"false\"\n") } } diff --git a/pkg/workflow/reply_to_pr_review_comment.go b/pkg/workflow/reply_to_pr_review_comment.go index 0d0f05d086..9c9a923acf 100644 --- a/pkg/workflow/reply_to_pr_review_comment.go +++ b/pkg/workflow/reply_to_pr_review_comment.go @@ -11,7 +11,7 @@ var replyToPRReviewCommentLog = logger.New("workflow:reply_to_pr_review_comment" type ReplyToPullRequestReviewCommentConfig struct { BaseSafeOutputConfig `yaml:",inline"` SafeOutputTargetConfig `yaml:",inline"` - Footer *bool `yaml:"footer,omitempty"` // Whether to add AI-generated footer to replies + Footer *string `yaml:"footer,omitempty"` // Whether to add AI-generated footer to replies } // parseReplyToPullRequestReviewCommentConfig handles reply-to-pull-request-review-comment configuration @@ -51,8 +51,12 @@ func (c *Compiler) parseReplyToPullRequestReviewCommentConfig(outputMap map[stri } } - // Parse footer - if footer, ok := configMap["footer"].(bool); ok { + // Parse footer as templatable bool + if err := preprocessBoolFieldAsString(configMap, "footer", replyToPRReviewCommentLog); err != nil { + replyToPRReviewCommentLog.Printf("Invalid footer value: %v", err) + return nil + } + if footer, ok := configMap["footer"].(string); ok { config.Footer = &footer } diff --git a/pkg/workflow/safe_outputs_config.go b/pkg/workflow/safe_outputs_config.go index 7d64c542e2..f5740eea62 100644 --- a/pkg/workflow/safe_outputs_config.go +++ b/pkg/workflow/safe_outputs_config.go @@ -332,8 +332,9 @@ func (c *Compiler) extractSafeOutputsConfig(frontmatter map[string]any) *SafeOut // This ensures there's always a fallback for transparency if _, exists := outputMap["noop"]; !exists { config.NoOp = &NoOpConfig{} - config.NoOp.Max = 1 // Default max - config.NoOp.ReportAsIssue = true // Default to reporting to issue + config.NoOp.Max = 1 // Default max + trueVal := "true" + config.NoOp.ReportAsIssue = &trueVal // Default to reporting to issue } } diff --git a/pkg/workflow/safe_outputs_config_generation.go b/pkg/workflow/safe_outputs_config_generation.go index 1a55eeea65..94075dd4e1 100644 --- a/pkg/workflow/safe_outputs_config_generation.go +++ b/pkg/workflow/safe_outputs_config_generation.go @@ -68,7 +68,7 @@ func generateSafeOutputsConfig(data *WorkflowData) string { data.SafeOutputs.CreateIssues.AllowedLabels, ) // Add group flag if enabled - if data.SafeOutputs.CreateIssues.Group { + if data.SafeOutputs.CreateIssues.Group != nil && *data.SafeOutputs.CreateIssues.Group == "true" { config["group"] = true } // Add expires value if set (0 means explicitly disabled or not set) diff --git a/pkg/workflow/safe_outputs_config_generation_helpers.go b/pkg/workflow/safe_outputs_config_generation_helpers.go index 1204b78d9a..de1400563c 100644 --- a/pkg/workflow/safe_outputs_config_generation_helpers.go +++ b/pkg/workflow/safe_outputs_config_generation_helpers.go @@ -124,19 +124,19 @@ func generateAssignToAgentConfig(max int, defaultMax int, defaultAgent string, t } // generatePullRequestConfig creates a config with max, allowed_labels, allow_empty, auto_merge, and expires -func generatePullRequestConfig(max int, defaultMax int, allowedLabels []string, allowEmpty bool, autoMerge bool, expires int) map[string]any { - safeOutputsConfigGenLog.Printf("Generating pull request config: max=%d, allowEmpty=%t, autoMerge=%t, expires=%d, labels_count=%d", +func generatePullRequestConfig(max int, defaultMax int, allowedLabels []string, allowEmpty *string, autoMerge *string, expires int) map[string]any { + safeOutputsConfigGenLog.Printf("Generating pull request config: max=%d, allowEmpty=%v, autoMerge=%v, expires=%d, labels_count=%d", max, allowEmpty, autoMerge, expires, len(allowedLabels)) config := generateMaxConfig(max, defaultMax) if len(allowedLabels) > 0 { config["allowed_labels"] = allowedLabels } // Pass allow_empty flag to MCP server so it can skip patch generation - if allowEmpty { + if allowEmpty != nil && *allowEmpty == "true" { config["allow_empty"] = true } // Pass auto_merge flag to enable auto-merge for the pull request - if autoMerge { + if autoMerge != nil && *autoMerge == "true" { config["auto_merge"] = true } // Pass expires to configure pull request expiration diff --git a/pkg/workflow/safe_outputs_footer_test.go b/pkg/workflow/safe_outputs_footer_test.go index 739831b07e..4299f9e4c7 100644 --- a/pkg/workflow/safe_outputs_footer_test.go +++ b/pkg/workflow/safe_outputs_footer_test.go @@ -23,7 +23,7 @@ func TestFooterConfiguration(t *testing.T) { require.NotNil(t, config) require.NotNil(t, config.CreateIssues) require.NotNil(t, config.CreateIssues.Footer) - assert.False(t, *config.CreateIssues.Footer) + assert.Equal(t, "false", *config.CreateIssues.Footer) } func TestGlobalFooterConfiguration(t *testing.T) { @@ -110,7 +110,7 @@ func TestGlobalFooterConfiguration(t *testing.T) { require.NotNil(t, config.Footer) assert.False(t, *config.Footer, "Global footer should be false") require.NotNil(t, config.CreatePullRequests.Footer) - assert.True(t, *config.CreatePullRequests.Footer, "Local PR footer should override to true") + assert.Equal(t, "true", *config.CreatePullRequests.Footer, "Local PR footer should override to true") // Verify in handler config workflowData := &WorkflowData{ @@ -151,7 +151,7 @@ func TestFooterInHandlerConfig(t *testing.T) { SafeOutputs: &SafeOutputsConfig{ CreateIssues: &CreateIssuesConfig{ BaseSafeOutputConfig: BaseSafeOutputConfig{Max: 1}, - Footer: boolPtr(false), + Footer: testStringPtr("false"), }, }, } diff --git a/pkg/workflow/templatables.go b/pkg/workflow/templatables.go index 104830049b..d83219772c 100644 --- a/pkg/workflow/templatables.go +++ b/pkg/workflow/templatables.go @@ -62,6 +62,21 @@ func preprocessBoolFieldAsString(configData map[string]any, fieldName string, lo return nil } +// buildTemplatableBoolEnvVar returns a YAML environment variable entry for a +// templatable boolean field. If value is a GitHub Actions expression it is +// embedded unquoted so that GitHub Actions can evaluate it at runtime; +// otherwise the literal string is quoted. Returns nil if value is nil. +func buildTemplatableBoolEnvVar(envVarName string, value *string) []string { + if value == nil { + return nil + } + v := *value + if strings.HasPrefix(v, "${{") { + return []string{fmt.Sprintf(" %s: %s\n", envVarName, v)} + } + return []string{fmt.Sprintf(" %s: %q\n", envVarName, v)} +} + // AddTemplatableBool adds a templatable boolean field to the handler config. // // The stored JSON value depends on the content of *value: diff --git a/pkg/workflow/update_discussion.go b/pkg/workflow/update_discussion.go index f30bba41ab..e2dca03a2d 100644 --- a/pkg/workflow/update_discussion.go +++ b/pkg/workflow/update_discussion.go @@ -13,7 +13,7 @@ type UpdateDiscussionsConfig struct { 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 *bool `yaml:"footer,omitempty"` // Controls whether AI-generated footer is added. When false, visible footer is omitted but XML markers are kept. + 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 @@ -25,7 +25,7 @@ func (c *Compiler) parseUpdateDiscussionsConfig(outputMap map[string]any) *Updat {Name: "title", Mode: FieldParsingKeyExistence, Dest: &cfg.Title}, {Name: "body", Mode: FieldParsingKeyExistence, Dest: &cfg.Body}, {Name: "labels", Mode: FieldParsingKeyExistence, Dest: &cfg.Labels}, - {Name: "footer", Mode: FieldParsingBoolValue, Dest: &cfg.Footer}, + {Name: "footer", Mode: FieldParsingTemplatableBool, StringDest: &cfg.Footer}, } }, func(cm map[string]any, cfg *UpdateDiscussionsConfig) { diff --git a/pkg/workflow/update_entity_helpers.go b/pkg/workflow/update_entity_helpers.go index b0cc765164..1db316d23a 100644 --- a/pkg/workflow/update_entity_helpers.go +++ b/pkg/workflow/update_entity_helpers.go @@ -204,6 +204,10 @@ const ( // Special case: nil values are treated as true for backward compatibility. // Used by body/footer fields in all update entities. FieldParsingBoolValue + // FieldParsingTemplatableBool mode: Field accepts a literal boolean or a GitHub Actions + // expression string (e.g. "${{ inputs.show-footer }}"). The parsed value is stored as a + // *string in the StringDest field of UpdateEntityFieldSpec. + FieldParsingTemplatableBool ) // parseUpdateEntityBoolField is a generic helper that parses boolean fields from config maps @@ -256,11 +260,34 @@ func parseUpdateEntityBoolField(configMap map[string]any, fieldName string, mode } } +// parseUpdateEntityStringBoolField parses a FieldParsingTemplatableBool field from a config map. +// It pre-processes the value to normalise literal booleans to strings, then returns the value +// as *string. Returns nil when the field is absent. +func parseUpdateEntityStringBoolField(configMap map[string]any, fieldName string, log *logger.Logger) *string { + if configMap == nil { + return nil + } + if _, exists := configMap[fieldName]; !exists { + return nil + } + if err := preprocessBoolFieldAsString(configMap, fieldName, log); err != nil { + if log != nil { + log.Printf("Invalid %s value: %v", fieldName, err) + } + return nil + } + if strVal, ok := configMap[fieldName].(string); ok { + return &strVal + } + return nil +} + // UpdateEntityFieldSpec defines a boolean field to be parsed from config type UpdateEntityFieldSpec struct { - Name string // Field name in config (e.g., "title", "body", "status") - Mode FieldParsingMode // Parsing mode for this field - Dest **bool // Pointer to the destination field in the config struct + Name string // Field name in config (e.g., "title", "body", "status") + Mode FieldParsingMode // Parsing mode for this field + Dest **bool // Pointer to the destination field (used with FieldParsingKeyExistence / FieldParsingBoolValue) + StringDest **string // Pointer to the destination string field (used with FieldParsingTemplatableBool) } // UpdateEntityParseOptions holds options for parsing entity-specific configuration @@ -305,7 +332,15 @@ func (c *Compiler) parseUpdateEntityConfigWithFields( // Parse entity-specific bool fields according to specs for _, field := range opts.Fields { - *field.Dest = parseUpdateEntityBoolField(configMap, field.Name, field.Mode) + if field.Mode == FieldParsingTemplatableBool { + if field.StringDest != nil { + *field.StringDest = parseUpdateEntityStringBoolField(configMap, field.Name, opts.Logger) + } + } else { + if field.Dest != nil { + *field.Dest = parseUpdateEntityBoolField(configMap, field.Name, field.Mode) + } + } } // Call custom parser if provided (e.g., for AllowedLabels in discussions) diff --git a/pkg/workflow/update_issue.go b/pkg/workflow/update_issue.go index dc7954bee1..8e585c2ebd 100644 --- a/pkg/workflow/update_issue.go +++ b/pkg/workflow/update_issue.go @@ -9,11 +9,11 @@ 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 *bool `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 + 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 @@ -25,7 +25,7 @@ func (c *Compiler) parseUpdateIssuesConfig(outputMap map[string]any) *UpdateIssu {Name: "status", Mode: FieldParsingKeyExistence, Dest: &cfg.Status}, {Name: "title", Mode: FieldParsingKeyExistence, Dest: &cfg.Title}, {Name: "body", Mode: FieldParsingBoolValue, Dest: &cfg.Body}, - {Name: "footer", Mode: FieldParsingBoolValue, Dest: &cfg.Footer}, + {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.go b/pkg/workflow/update_pull_request.go index 1186745822..4ec772b966 100644 --- a/pkg/workflow/update_pull_request.go +++ b/pkg/workflow/update_pull_request.go @@ -12,7 +12,7 @@ type UpdatePullRequestsConfig struct { 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 *bool `yaml:"footer,omitempty"` // Controls whether AI-generated footer is added. When false, visible footer is omitted. + Footer *string `yaml:"footer,omitempty"` // Controls whether AI-generated footer is added. When false, visible footer is omitted. } // parseUpdatePullRequestsConfig handles update-pull-request configuration @@ -25,7 +25,7 @@ func (c *Compiler) parseUpdatePullRequestsConfig(outputMap map[string]any) *Upda return []UpdateEntityFieldSpec{ {Name: "title", Mode: FieldParsingBoolValue, Dest: &cfg.Title}, {Name: "body", Mode: FieldParsingBoolValue, Dest: &cfg.Body}, - {Name: "footer", Mode: FieldParsingBoolValue, Dest: &cfg.Footer}, + {Name: "footer", Mode: FieldParsingTemplatableBool, StringDest: &cfg.Footer}, } }, func(configMap map[string]any, cfg *UpdatePullRequestsConfig) { // Parse operation field diff --git a/pkg/workflow/update_release.go b/pkg/workflow/update_release.go index 4784d861cb..88e93118cb 100644 --- a/pkg/workflow/update_release.go +++ b/pkg/workflow/update_release.go @@ -9,7 +9,7 @@ var updateReleaseLog = logger.New("workflow:update_release") // UpdateReleaseConfig holds configuration for updating GitHub releases from agent output type UpdateReleaseConfig struct { UpdateEntityConfig `yaml:",inline"` - Footer *bool `yaml:"footer,omitempty"` // Controls whether AI-generated footer is added. When false, visible footer is omitted but XML markers are kept. + Footer *string `yaml:"footer,omitempty"` // Controls whether AI-generated footer is added. When false, visible footer is omitted but XML markers are kept. } // parseUpdateReleaseConfig handles update-release configuration @@ -18,7 +18,7 @@ func (c *Compiler) parseUpdateReleaseConfig(outputMap map[string]any) *UpdateRel UpdateEntityRelease, "update-release", updateReleaseLog, func(cfg *UpdateReleaseConfig) []UpdateEntityFieldSpec { return []UpdateEntityFieldSpec{ - {Name: "footer", Mode: FieldParsingBoolValue, Dest: &cfg.Footer}, + {Name: "footer", Mode: FieldParsingTemplatableBool, StringDest: &cfg.Footer}, } }, nil) }