Convert boolean safe output fields to templatable bools#17653
Conversation
|
@copilot do not convert CreateDiscussionsConfig.FallbackToIssue |
- CloseOlderIssues/Group/Footer in CreateIssuesConfig: bool/*bool → *string - HideOlderComments in AddCommentsConfig: bool → *string - CloseOlderDiscussions/FallbackToIssue/Footer in CreateDiscussionsConfig: bool/*bool → *string - AllowEmpty/AutoMerge/Footer in CreatePullRequestsConfig: bool/*bool → *string - UnassignFirst in AssignToUserConfig: bool → *string - ReportAsIssue in NoOpConfig: bool → *string - Footer in UpdateIssuesConfig/UpdatePullRequestsConfig/UpdateDiscussionsConfig/UpdateReleaseConfig/ReplyToPRReviewCommentConfig: *bool → *string - Add FieldParsingTemplatableBool mode to update_entity_helpers.go - Add getEffectiveFooterForTemplatable helper (replaces getEffectiveFooter) - Update handler registry to use AddTemplatableBool - Update build job functions to emit expressions or quoted strings - Update all JS handlers to use parseBoolTemplatable from templatable.cjs Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Reverted in 2f346c7 — Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
| 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 fallbackToIssue = parseBoolTemplatable(config.fallback_to_issue, true); |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
pkg/workflow/add_comment.go
Outdated
| customEnvVars = append(customEnvVars, " GH_AW_HIDE_OLDER_COMMENTS: \"true\"\n") | ||
| if data.SafeOutputs.AddComments.HideOlderComments != nil { | ||
| hoc := *data.SafeOutputs.AddComments.HideOlderComments | ||
| if strings.HasPrefix(hoc, "${{") { |
There was a problem hiding this comment.
@copilot move this check to a helper function and refactor
There was a problem hiding this comment.
Done in 49471ab — added buildTemplatableBoolEnvVar(envVarName string, value *string) []string to templatables.go and replaced all inline strings.HasPrefix("${{") blocks in add_comment.go, create_issue.go, create_discussion.go, create_pull_request.go, and notify_comment.go with calls to this helper.
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Converts several “safe” boolean safe-output config fields from Go bool/*bool to templatable *string values, enabling GitHub Actions expressions (e.g. ${{ inputs.flag }}) to control runtime behavior while keeping permission-affecting booleans as real booleans.
Changes:
- Introduced a templatable-bool parsing mode for update-* entities and refactored multiple configs to use
*string+ templatable parsing. - Added a shared
buildTemplatableBoolEnvVarhelper and migrated job env-var emission to support expression values. - Updated JS handlers to parse templatable booleans consistently via
parseBoolTemplatable, and refreshed affected tests/lock output.
Reviewed changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/update_release.go | Switch footer to templatable bool parsing for update-release. |
| pkg/workflow/update_pull_request.go | Switch footer to templatable bool parsing for update-pull-request. |
| pkg/workflow/update_issue.go | Switch footer to templatable bool parsing for update-issue. |
| pkg/workflow/update_entity_helpers.go | Add FieldParsingTemplatableBool mode, string parsing helper, and spec plumbing. |
| pkg/workflow/update_discussion.go | Switch footer to templatable bool parsing for update-discussion. |
| pkg/workflow/templatables.go | Add buildTemplatableBoolEnvVar helper for YAML env emission of templatable bools. |
| pkg/workflow/safe_outputs_footer_test.go | Update footer-related assertions for *string templatable bools. |
| pkg/workflow/safe_outputs_config_generation_helpers.go | Update PR config generator signature to accept templatable bools (now *string). |
| pkg/workflow/safe_outputs_config_generation.go | Update generation logic for templatable group field. |
| pkg/workflow/safe_outputs_config.go | Change noop default report-as-issue representation to templatable *string. |
| pkg/workflow/reply_to_pr_review_comment.go | Parse reply footer as templatable bool (*string). |
| pkg/workflow/notify_comment.go | Emit noop report-as-issue as templatable env var. |
| pkg/workflow/noop_test.go | Update noop tests for report-as-issue now being *string. |
| pkg/workflow/noop.go | Change noop report-as-issue to templatable *string with preprocessing. |
| pkg/workflow/create_pull_request.go | Convert allow-empty/auto-merge/footer to templatable bools; use shared env-var helper. |
| pkg/workflow/create_issue.go | Convert close-older-issues/group/footer to templatable bools; preprocess before unmarshal. |
| pkg/workflow/create_discussion.go | Convert close-older-discussions/footer to templatable bools; preprocess before unmarshal. |
| pkg/workflow/compiler_safe_outputs_config_test.go | Update config builder tests for templatable boolean fields. |
| pkg/workflow/compiler_safe_outputs_config.go | Route templatable bool fields through AddTemplatableBool and new effective-footer helper. |
| pkg/workflow/compile_outputs_pr_test.go | Update PR auto-merge test for templatable bool representation. |
| pkg/workflow/assign_to_user.go | Convert unassign-first to templatable bool and preprocess before unmarshal. |
| pkg/workflow/add_comment_target_repo_test.go | Update hide-older-comments tests for *string templatable bool. |
| pkg/workflow/add_comment.go | Convert hide-older-comments to templatable bool; use shared env-var helper + preprocessing. |
| actions/setup/js/update_release.cjs | Use parseBoolTemplatable for footer inclusion. |
| actions/setup/js/update_pull_request.cjs | Use parseBoolTemplatable for footer inclusion. |
| actions/setup/js/update_issue.cjs | Use parseBoolTemplatable for footer inclusion. |
| actions/setup/js/update_discussion.cjs | Use parseBoolTemplatable for footer inclusion. |
| actions/setup/js/reply_to_pr_review_comment.cjs | Use parseBoolTemplatable for footer inclusion. |
| actions/setup/js/create_pull_request.cjs | Use parseBoolTemplatable for allow_empty/auto_merge/footer. |
| actions/setup/js/create_issue.cjs | Use parseBoolTemplatable for group/close_older_issues/footer. |
| actions/setup/js/create_discussion.cjs | Use parseBoolTemplatable for close_older_discussions/footer. |
| actions/setup/js/assign_to_user.cjs | Use parseBoolTemplatable for unassign_first. |
| actions/setup/js/add_comment.cjs | Use parseBoolTemplatable for hide_older_comments. |
| .github/workflows/weekly-safe-outputs-spec-review.lock.yml | Refresh handler-config JSON to include the new templatable fields. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // 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 |
There was a problem hiding this comment.
generatePullRequestConfig only sets allow_empty/auto_merge when the value is the literal string "true". If the config uses a GitHub Actions expression (e.g. "${{ inputs.allow_empty }}"), the field is silently omitted from config.json, which means downstream runtime components that rely on config.json (e.g. patch generation / has_patch handling) will treat it as disabled even when the expression evaluates to true. Consider emitting the field for all non-nil values using the same templatable-bool encoding as handler config ("true"→true, "false"→false, expression→string) so runtime code can interpret it correctly.
See below for a potential fix:
// Pass allow_empty flag to MCP server so it can skip patch generation.
// Use templatable-bool encoding: "true" -> true, "false" -> false, expression -> string.
if allowEmpty != nil {
switch *allowEmpty {
case "true":
config["allow_empty"] = true
case "false":
config["allow_empty"] = false
default:
config["allow_empty"] = *allowEmpty
}
}
// Pass auto_merge flag to enable auto-merge for the pull request.
// Use templatable-bool encoding: "true" -> true, "false" -> false, expression -> string.
if autoMerge != nil {
switch *autoMerge {
case "true":
config["auto_merge"] = true
case "false":
config["auto_merge"] = false
default:
config["auto_merge"] = *autoMerge
}
*stringtemplatable boolsCreateDiscussionsConfig.FallbackToIssueas*bool(Go side)fallbackToIssueincreate_discussion.cjsto originalconfig.fallback_to_issue !== falsepatternbuildTemplatableBoolEnvVarhelper intotemplatables.goand refactor allstrings.HasPrefix("${{")call sites inadd_comment.go,create_issue.go,create_discussion.go,create_pull_request.go,notify_comment.goOriginal prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.