From 5aee041195c6130c7e4eb3aa42183d8a5c5aa258 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 28 Jul 2026 15:59:17 +0000 Subject: [PATCH 1/2] Initial plan From f49f175d97d122c81f4e8e97d0f7af5a57e62007 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 28 Jul 2026 16:15:27 +0000 Subject: [PATCH 2/2] refactor: consolidate security scanner flag helpers into flags.go Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/add_command.go | 8 +--- pkg/cli/add_command_test.go | 9 ----- pkg/cli/add_wizard_command.go | 8 +--- pkg/cli/add_wizard_command_test.go | 9 ----- pkg/cli/deploy_command.go | 8 +--- pkg/cli/deploy_command_test.go | 9 ----- pkg/cli/flags.go | 18 +++++++++ pkg/cli/flags_test.go | 65 ++++++++++++++++++++++++++++++ pkg/cli/trial_command.go | 12 ++---- pkg/cli/trial_command_test.go | 11 ----- pkg/cli/update_command.go | 12 ++---- pkg/cli/update_command_test.go | 23 ----------- 12 files changed, 95 insertions(+), 97 deletions(-) diff --git a/pkg/cli/add_command.go b/pkg/cli/add_command.go index a06acf1490b..44758f6049a 100644 --- a/pkg/cli/add_command.go +++ b/pkg/cli/add_command.go @@ -128,9 +128,7 @@ func runAddCommand(cmd *cobra.Command, args []string, validateEngine func(string workflowDir, _ := cmd.Flags().GetString("dir") noStopAfter, _ := cmd.Flags().GetBool("no-stop-after") stopAfter, _ := cmd.Flags().GetString("stop-after") - disableSecurityScanner, _ := cmd.Flags().GetBool("no-security-scanner") - disableSecurityScannerLegacy, _ := cmd.Flags().GetBool("disable-security-scanner") - disableSecurityScanner = disableSecurityScanner || disableSecurityScannerLegacy + disableSecurityScanner := resolveDeprecatedBoolFlag(cmd, "no-security-scanner", "disable-security-scanner") if nameFlag != "" && len(args) > 1 { return errors.New("--name flag cannot be used when adding multiple workflows at once") @@ -219,9 +217,7 @@ func registerAddCommandFlags(cmd *cobra.Command) { cmd.Flags().String("stop-after", "", "Override stop-after value in the workflow (e.g., '+48h', '2025-12-31 23:59:59')") // Add no-security-scanner flag to add command (--disable-security-scanner is kept as a deprecated alias) - cmd.Flags().Bool("no-security-scanner", false, "Skip security scanning of workflow markdown content") - cmd.Flags().Bool("disable-security-scanner", false, "Skip security scanning of workflow markdown content") - _ = cmd.Flags().MarkDeprecated("disable-security-scanner", "use --no-security-scanner instead") + addSecurityScannerFlag(cmd) // Register completions for add command RegisterEngineFlagCompletion(cmd) diff --git a/pkg/cli/add_command_test.go b/pkg/cli/add_command_test.go index 413f4da2dbe..f22ae73c02f 100644 --- a/pkg/cli/add_command_test.go +++ b/pkg/cli/add_command_test.go @@ -92,15 +92,6 @@ func TestNewAddCommand_MentionsEnterpriseSourceResolution(t *testing.T) { assert.Contains(t, cmd.Long, "Use full https://github.com/... source URLs for other public github.com workflows.") } -func TestNewAddCommand_DeprecatesDisableSecurityScannerFlag(t *testing.T) { - cmd := NewAddCommand(validateEngineStub) - require.NotNil(t, cmd) - - flag := cmd.Flags().Lookup("disable-security-scanner") - require.NotNil(t, flag, "add command should keep --disable-security-scanner as a deprecated alias") - assert.Equal(t, "use --no-security-scanner instead", flag.Deprecated) -} - func TestAddWorkflows(t *testing.T) { tests := []struct { name string diff --git a/pkg/cli/add_wizard_command.go b/pkg/cli/add_wizard_command.go index c35d5aef2f7..b0613619592 100644 --- a/pkg/cli/add_wizard_command.go +++ b/pkg/cli/add_wizard_command.go @@ -73,9 +73,7 @@ Note: To create a new workflow from scratch, use the 'new' command instead.`, skipSecretLegacy, _ := cmd.Flags().GetBool("skip-secret") skipSecret := noSecret || skipSecretLegacy appendText, _ := cmd.Flags().GetString("append") - disableSecurityScanner, _ := cmd.Flags().GetBool("no-security-scanner") - disableSecurityScannerLegacy, _ := cmd.Flags().GetBool("disable-security-scanner") - disableSecurityScanner = disableSecurityScanner || disableSecurityScannerLegacy + disableSecurityScanner := resolveDeprecatedBoolFlag(cmd, "no-security-scanner", "disable-security-scanner") addWizardLog.Printf("Starting add-wizard: workflows=%v, engine=%s, verbose=%v", workflows, engineOverride, verbose) @@ -131,9 +129,7 @@ Note: To create a new workflow from scratch, use the 'new' command instead.`, // Add no-security-scanner flag (--disable-security-scanner is kept as a deprecated alias // for consistency with add and other install entry points) - cmd.Flags().Bool("no-security-scanner", false, "Skip security scanning of workflow markdown content") - cmd.Flags().Bool("disable-security-scanner", false, "Skip security scanning of workflow markdown content") - _ = cmd.Flags().MarkDeprecated("disable-security-scanner", "use --no-security-scanner instead") + addSecurityScannerFlag(cmd) // Register completions RegisterEngineFlagCompletion(cmd) diff --git a/pkg/cli/add_wizard_command_test.go b/pkg/cli/add_wizard_command_test.go index 0f1488bc7c5..40849a54565 100644 --- a/pkg/cli/add_wizard_command_test.go +++ b/pkg/cli/add_wizard_command_test.go @@ -39,15 +39,6 @@ func TestAddWizardCommand_FlagUsageMatchesAddCommand(t *testing.T) { } } -func TestAddWizardCommand_DeprecatesDisableSecurityScannerFlag(t *testing.T) { - cmd := NewAddWizardCommand(validateEngineStub) - require.NotNil(t, cmd) - - flag := cmd.Flags().Lookup("disable-security-scanner") - require.NotNil(t, flag, "add-wizard command should keep --disable-security-scanner as a deprecated alias") - assert.Equal(t, "use --no-security-scanner instead", flag.Deprecated) -} - func TestAddWizardCommand_ExamplesMentionNewFlags(t *testing.T) { cmd := NewAddWizardCommand(func(string) error { return nil }) require.NotNil(t, cmd) diff --git a/pkg/cli/deploy_command.go b/pkg/cli/deploy_command.go index 1cfa77b1ece..66ff023fd91 100644 --- a/pkg/cli/deploy_command.go +++ b/pkg/cli/deploy_command.go @@ -99,9 +99,7 @@ func registerDeployFlags(cmd *cobra.Command) { cmd.Flags().StringP("dir", "d", "", "Workflow directory (default: $GH_AW_WORKFLOWS_DIR or .github/workflows)") cmd.Flags().Bool("no-stop-after", false, "Remove any stop-after field from the workflow") cmd.Flags().String("stop-after", "", "Override stop-after value in the workflow (e.g., '+48h', '2025-12-31 23:59:59')") - cmd.Flags().Bool("no-security-scanner", false, "Skip security scanning of workflow markdown content") - cmd.Flags().Bool("disable-security-scanner", false, "Skip security scanning of workflow markdown content") - _ = cmd.Flags().MarkDeprecated("disable-security-scanner", "use --no-security-scanner instead") + addSecurityScannerFlag(cmd) cmd.Flags().String("cool-down", defaultDeployCooldown, coolDownFlagUsage) cmd.Flags().String("org", "", "Deploy workflows across repositories in an organization") cmd.Flags().StringSlice("repos", nil, "Limit --org mode to repositories matching one or more glob patterns") @@ -149,9 +147,7 @@ func parseDeployCommandOptions(cmd *cobra.Command, workflows []string, validateE workflowDir, _ := cmd.Flags().GetString("dir") noStopAfter, _ := cmd.Flags().GetBool("no-stop-after") stopAfter, _ := cmd.Flags().GetString("stop-after") - disableSecurityScanner, _ := cmd.Flags().GetBool("no-security-scanner") - disableSecurityScannerLegacy, _ := cmd.Flags().GetBool("disable-security-scanner") - disableSecurityScanner = disableSecurityScanner || disableSecurityScannerLegacy + disableSecurityScanner := resolveDeprecatedBoolFlag(cmd, "no-security-scanner", "disable-security-scanner") coolDownStr, _ := cmd.Flags().GetString("cool-down") if nameFlag != "" && len(workflows) > 1 { diff --git a/pkg/cli/deploy_command_test.go b/pkg/cli/deploy_command_test.go index 6af7f7ccd0a..8f30ffe0a8b 100644 --- a/pkg/cli/deploy_command_test.go +++ b/pkg/cli/deploy_command_test.go @@ -72,15 +72,6 @@ func TestNewDeployCommand_CoolDownFlagUsageMatchesUpdate(t *testing.T) { assert.Equal(t, coolDownFlagUsage, coolDownFlag.Usage) } -func TestNewDeployCommand_DeprecatesDisableSecurityScannerFlag(t *testing.T) { - cmd := NewDeployCommand(func(string) error { return nil }) - require.NotNil(t, cmd) - - flag := cmd.Flags().Lookup("disable-security-scanner") - require.NotNil(t, flag, "deploy command should keep --disable-security-scanner as a deprecated alias") - assert.Equal(t, "use --no-security-scanner instead", flag.Deprecated) -} - func TestNewDeployCommand_RequiresRepoFlag(t *testing.T) { cmd := NewDeployCommand(func(string) error { return nil }) require.NotNil(t, cmd) diff --git a/pkg/cli/flags.go b/pkg/cli/flags.go index a371846842f..b082ae5d5e0 100644 --- a/pkg/cli/flags.go +++ b/pkg/cli/flags.go @@ -38,3 +38,21 @@ func addOutputFlag(cmd *cobra.Command, defaultValue string) { func addJSONFlag(cmd *cobra.Command) { cmd.Flags().BoolP("json", "j", false, "Output results in JSON format") } + +// addSecurityScannerFlag adds the --no-security-scanner flag and its deprecated +// --disable-security-scanner alias to a command. +func addSecurityScannerFlag(cmd *cobra.Command) { + cmd.Flags().Bool("no-security-scanner", false, "Skip security scanning of workflow markdown content") + cmd.Flags().Bool("disable-security-scanner", false, "Skip security scanning of workflow markdown content") + _ = cmd.Flags().MarkDeprecated("disable-security-scanner", "use --no-security-scanner instead") +} + +// resolveDeprecatedBoolFlag returns true if either the newName flag or the +// deprecated oldName flag is set on cmd. It is intended for cases where a flag +// has been renamed: callers register both names and use this helper to collapse +// them into a single effective value. +func resolveDeprecatedBoolFlag(cmd *cobra.Command, newName, oldName string) bool { + newVal, _ := cmd.Flags().GetBool(newName) + oldVal, _ := cmd.Flags().GetBool(oldName) + return newVal || oldVal +} diff --git a/pkg/cli/flags_test.go b/pkg/cli/flags_test.go index 627b4f3dbcd..b7aadf8dbe1 100644 --- a/pkg/cli/flags_test.go +++ b/pkg/cli/flags_test.go @@ -300,3 +300,68 @@ func TestEngineFlagUsageText(t *testing.T) { t.Errorf("Unexpected --engine filter usage text: %s", filterFlag.Usage) } } + +func TestAddSecurityScannerFlag(t *testing.T) { + t.Parallel() + + cmd := &cobra.Command{Use: "test"} + addSecurityScannerFlag(cmd) + + primary := cmd.Flags().Lookup("no-security-scanner") + if primary == nil { + t.Fatal("addSecurityScannerFlag should register --no-security-scanner") + } + if primary.Usage != "Skip security scanning of workflow markdown content" { + t.Errorf("Unexpected --no-security-scanner usage: %s", primary.Usage) + } + + deprecated := cmd.Flags().Lookup("disable-security-scanner") + if deprecated == nil { + t.Fatal("addSecurityScannerFlag should register --disable-security-scanner as a deprecated alias") + } + if deprecated.Deprecated != "use --no-security-scanner instead" { + t.Errorf("Expected deprecation message 'use --no-security-scanner instead', got %q", deprecated.Deprecated) + } +} + +func TestResolveDeprecatedBoolFlag(t *testing.T) { + t.Parallel() + + setup := func() *cobra.Command { + cmd := &cobra.Command{Use: "test"} + cmd.Flags().Bool("new-flag", false, "new flag") + cmd.Flags().Bool("old-flag", false, "old flag") + _ = cmd.Flags().MarkDeprecated("old-flag", "use --new-flag instead") + return cmd + } + + t.Run("both false returns false", func(t *testing.T) { + t.Parallel() + cmd := setup() + if resolveDeprecatedBoolFlag(cmd, "new-flag", "old-flag") { + t.Error("expected false when both flags are unset") + } + }) + + t.Run("new flag true returns true", func(t *testing.T) { + t.Parallel() + cmd := setup() + if err := cmd.Flags().Set("new-flag", "true"); err != nil { + t.Fatalf("failed to set new-flag: %v", err) + } + if !resolveDeprecatedBoolFlag(cmd, "new-flag", "old-flag") { + t.Error("expected true when new flag is set") + } + }) + + t.Run("old flag true returns true", func(t *testing.T) { + t.Parallel() + cmd := setup() + if err := cmd.Flags().Set("old-flag", "true"); err != nil { + t.Fatalf("failed to set old-flag: %v", err) + } + if !resolveDeprecatedBoolFlag(cmd, "new-flag", "old-flag") { + t.Error("expected true when deprecated old flag is set") + } + }) +} diff --git a/pkg/cli/trial_command.go b/pkg/cli/trial_command.go index 43faf587486..8829c1208e4 100644 --- a/pkg/cli/trial_command.go +++ b/pkg/cli/trial_command.go @@ -55,9 +55,7 @@ Trial results are saved both locally (in the trials/ directory) and in the host cloneRepoSpec, _ := cmd.Flags().GetString("clone-repo") hostRepoSpec, _ := cmd.Flags().GetString("host-repo") deleteHostRepo, _ := cmd.Flags().GetBool("delete-host-repo-after") - legacyForceDelete, _ := cmd.Flags().GetBool("force-delete-host-repo-before") - deleteHostRepoBefore, _ := cmd.Flags().GetBool("delete-host-repo-before") - forceDeleteHostRepo := legacyForceDelete || deleteHostRepoBefore + forceDeleteHostRepo := resolveDeprecatedBoolFlag(cmd, "delete-host-repo-before", "force-delete-host-repo-before") yes, _ := cmd.Flags().GetBool("yes") dryRun, _ := cmd.Flags().GetBool("dry-run") jsonOutput, _ := cmd.Flags().GetBool("json") @@ -68,9 +66,7 @@ Trial results are saved both locally (in the trials/ directory) and in the host engineOverride, _ := cmd.Flags().GetString("engine") appendText, _ := cmd.Flags().GetString("append") verbose, _ := cmd.Root().PersistentFlags().GetBool("verbose") - disableSecurityScanner, _ := cmd.Flags().GetBool("no-security-scanner") - disableSecurityScannerLegacy, _ := cmd.Flags().GetBool("disable-security-scanner") - disableSecurityScanner = disableSecurityScanner || disableSecurityScannerLegacy + disableSecurityScanner := resolveDeprecatedBoolFlag(cmd, "no-security-scanner", "disable-security-scanner") if err := validateEngine(engineOverride); err != nil { trialLog.Printf("Engine validation failed: engine=%s, err=%v", engineOverride, err) @@ -126,9 +122,7 @@ Trial results are saved both locally (in the trials/ directory) and in the host addEngineFlag(cmd) addJSONFlag(cmd) cmd.Flags().String("append", "", "Append extra content to the end of the agentic workflow on installation") - cmd.Flags().Bool("no-security-scanner", false, "Skip security scanning of workflow markdown content") - cmd.Flags().Bool("disable-security-scanner", false, "Skip security scanning of workflow markdown content") - _ = cmd.Flags().MarkDeprecated("disable-security-scanner", "use --no-security-scanner instead") + addSecurityScannerFlag(cmd) cmd.MarkFlagsMutuallyExclusive("logical-repo", "clone-repo") return cmd diff --git a/pkg/cli/trial_command_test.go b/pkg/cli/trial_command_test.go index f5e4bbc4377..a7575888c94 100644 --- a/pkg/cli/trial_command_test.go +++ b/pkg/cli/trial_command_test.go @@ -9,8 +9,6 @@ import ( "testing" "github.com/github/gh-aw/pkg/testutil" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) func TestNewTrialCommandCloneRepoFlagDescription(t *testing.T) { @@ -47,15 +45,6 @@ func TestNewTrialCommandNoArgsErrorIncludesExample(t *testing.T) { } } -func TestNewTrialCommand_DeprecatesDisableSecurityScannerFlag(t *testing.T) { - cmd := NewTrialCommand(func(string) error { return nil }) - require.NotNil(t, cmd) - - flag := cmd.Flags().Lookup("disable-security-scanner") - require.NotNil(t, flag, "trial command should keep --disable-security-scanner as a deprecated alias") - assert.Equal(t, "use --no-security-scanner instead", flag.Deprecated) -} - // Test the host repo slug processing logic with dot notation func TestHostRepoSlugProcessing(t *testing.T) { testCases := []struct { diff --git a/pkg/cli/update_command.go b/pkg/cli/update_command.go index 617935dbaeb..948f87a8a90 100644 --- a/pkg/cli/update_command.go +++ b/pkg/cli/update_command.go @@ -78,14 +78,10 @@ Note: In GitHub Enterprise repos, shorthand source specs resolve on your enterpr noStopAfter, _ := cmd.Flags().GetBool("no-stop-after") stopAfter, _ := cmd.Flags().GetString("stop-after") noMergeFlag, _ := cmd.Flags().GetBool("no-merge") - disableReleaseBump, _ := cmd.Flags().GetBool("no-release-bump") - disableReleaseBumpLegacy, _ := cmd.Flags().GetBool("disable-release-bump") - disableReleaseBump = disableReleaseBump || disableReleaseBumpLegacy + disableReleaseBump := resolveDeprecatedBoolFlag(cmd, "no-release-bump", "disable-release-bump") noCompile, _ := cmd.Flags().GetBool("no-compile") noRedirect, _ := cmd.Flags().GetBool("no-redirect") - disableSecurityScanner, _ := cmd.Flags().GetBool("no-security-scanner") - disableSecurityScannerLegacy, _ := cmd.Flags().GetBool("disable-security-scanner") - disableSecurityScanner = disableSecurityScanner || disableSecurityScannerLegacy + disableSecurityScanner := resolveDeprecatedBoolFlag(cmd, "no-security-scanner", "disable-security-scanner") approveFlag, _ := cmd.Flags().GetBool("approve") createPRFlag, _ := cmd.Flags().GetBool("create-pull-request") prFlagAlias, _ := cmd.Flags().GetBool("pr") @@ -175,9 +171,7 @@ Note: In GitHub Enterprise repos, shorthand source specs resolve on your enterpr cmd.Flags().Bool("no-release-bump", false, "Restrict automatic major version bumps to core actions/* only (non-core actions are left as-is)") cmd.Flags().Bool("disable-release-bump", false, "Restrict automatic major version bumps to core actions/* only (non-core actions are left as-is)") _ = cmd.Flags().MarkDeprecated("disable-release-bump", "use --no-release-bump instead") - cmd.Flags().Bool("no-security-scanner", false, "Skip security scanning of workflow markdown content") - cmd.Flags().Bool("disable-security-scanner", false, "Skip security scanning of workflow markdown content") - _ = cmd.Flags().MarkDeprecated("disable-security-scanner", "use --no-security-scanner instead") + addSecurityScannerFlag(cmd) cmd.Flags().Bool("approve", false, "Approve all safe update changes. When strict mode is active (the default), the compiler emits warnings for new restricted secrets or unapproved action additions/removals not present in the existing gh-aw-manifest. Use this flag to approve and skip safe update enforcement") cmd.Flags().Bool("no-compile", false, "Skip recompiling workflows during update (do not modify lock files)") cmd.Flags().Bool("no-redirect", false, "Refuse updates when redirect frontmatter is present") diff --git a/pkg/cli/update_command_test.go b/pkg/cli/update_command_test.go index 893a9c2e6a1..1fb229bd78f 100644 --- a/pkg/cli/update_command_test.go +++ b/pkg/cli/update_command_test.go @@ -82,29 +82,6 @@ This is the base content.` } } -func TestNewUpdateCommand_HasDisableSecurityScannerFlag(t *testing.T) { - cmd := NewUpdateCommand(func(string) error { return nil }) - require.NotNil(t, cmd, "update command should be created") - - flag := cmd.Flags().Lookup("no-security-scanner") - require.NotNil(t, flag, "update command should register --no-security-scanner") - assert.Equal(t, "Skip security scanning of workflow markdown content", flag.Usage, "flag help text should match add/trial wording") - - // Deprecated alias should still be registered - deprecated := cmd.Flags().Lookup("disable-security-scanner") - require.NotNil(t, deprecated, "update command should keep --disable-security-scanner as a deprecated alias") - assert.Equal(t, "use --no-security-scanner instead", deprecated.Deprecated) -} - -func TestNewUpdateCommand_DeprecatesDisableReleaseBumpFlag(t *testing.T) { - cmd := NewUpdateCommand(func(string) error { return nil }) - require.NotNil(t, cmd) - - flag := cmd.Flags().Lookup("disable-release-bump") - require.NotNil(t, flag, "update command should keep --disable-release-bump as a deprecated alias") - assert.Equal(t, "use --no-release-bump instead", flag.Deprecated) -} - func TestNewUpdateCommand_CoolDownFlagUsage(t *testing.T) { cmd := NewUpdateCommand(func(string) error { return nil }) require.NotNil(t, cmd)