From 0e2d815c8cb33ae7981e9b40e74ff5459fc85059 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 2 Aug 2026 10:40:15 +0000 Subject: [PATCH 1/3] Initial plan From 84e106d0e3b50a07a938d9dbe5067e76c4f76979 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 2 Aug 2026 10:56:04 +0000 Subject: [PATCH 2/3] Refactor golint leftovers in main and stringsconcatloop Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- cmd/gh-aw/main.go | 823 ++++++++---------- pkg/actionpins/actionpins_internal_test.go | 6 +- .../stringsconcatloop/stringsconcatloop.go | 142 +-- 3 files changed, 443 insertions(+), 528 deletions(-) diff --git a/cmd/gh-aw/main.go b/cmd/gh-aw/main.go index 87e4949eb6c..cd004785dc9 100644 --- a/cmd/gh-aw/main.go +++ b/cmd/gh-aw/main.go @@ -285,135 +285,7 @@ Unlike ` + "`gh aw upgrade`" + `, ` + "`gh aw compile`" + ` only applies codemod ` + string(constants.CLIExtensionPrefix) + ` compile --dependabot --force # Force overwrite existing dependabot.yml ` + string(constants.CLIExtensionPrefix) + ` compile --gh-aw-ref main # Pin workflows to the SHA of github/gh-aw main at compile time ` + string(constants.CLIExtensionPrefix) + ` compile --action-tag v1.2.3 # Pin workflows to a specific release tag`, - RunE: func(cmd *cobra.Command, args []string) error { - engineOverride, _ := cmd.Flags().GetString("engine") - actionMode, _ := cmd.Flags().GetString("action-mode") - actionTag, _ := cmd.Flags().GetString("action-tag") - actionsRepo, _ := cmd.Flags().GetString("actions-repo") - ghAwRef, _ := cmd.Flags().GetString("gh-aw-ref") - if ghAwRef != "" { - // --gh-aw-ref is a convenience alias: emit refs like - // `github/gh-aw/actions/setup@` so external e2e harnesses can - // test the compiled workflows against a specific gh-aw revision. - // Resolve branch/tag names to their commit SHA so the baked-in ref - // is immutable and not vulnerable to branch/tag mutation. - resolvedRef, resolveErr := workflow.ResolveGhAwRef(cmd.Context(), ghAwRef) - if resolveErr != nil { - return fmt.Errorf("--gh-aw-ref: %w", resolveErr) - } - actionMode = string(workflow.ActionModeRelease) - actionTag = resolvedRef - } - validate, _ := cmd.Flags().GetBool("validate") - watch, _ := cmd.Flags().GetBool("watch") - dir, _ := cmd.Flags().GetString("dir") - workflowsDir, _ := cmd.Flags().GetString("workflows-dir") - noEmit, _ := cmd.Flags().GetBool("no-emit") - purge, _ := cmd.Flags().GetBool("purge") - strict, _ := cmd.Flags().GetBool("strict") - trial, _ := cmd.Flags().GetBool("trial") - logicalRepo, _ := cmd.Flags().GetString("logical-repo") - dependabot, _ := cmd.Flags().GetBool("dependabot") - forceOverwrite, _ := cmd.Flags().GetBool("force") - refreshStopTime, _ := cmd.Flags().GetBool("refresh-stop-time") - forceRefreshActionPins, _ := cmd.Flags().GetBool("force-refresh-action-pins") - allowActionRefs, _ := cmd.Flags().GetBool("allow-action-refs") - zizmor, _ := cmd.Flags().GetBool("zizmor") - poutine, _ := cmd.Flags().GetBool("poutine") - actionlint, _ := cmd.Flags().GetBool("actionlint") - runnerGuard, _ := cmd.Flags().GetBool("runner-guard") - syft, _ := cmd.Flags().GetBool("syft") - grype, _ := cmd.Flags().GetBool("grype") - grant, _ := cmd.Flags().GetBool("grant") - yamllint, _ := cmd.Flags().GetBool("yamllint") - jsonOutput, _ := cmd.Flags().GetBool("json") - showAllErrors, _ := cmd.Flags().GetBool("show-all") - fix, _ := cmd.Flags().GetBool("fix") - stats, _ := cmd.Flags().GetBool("stats") - failFast, _ := cmd.Flags().GetBool("fail-fast") - noCheckUpdate, _ := cmd.Flags().GetBool("no-check-update") - scheduleSeed, _ := cmd.Flags().GetString("schedule-seed") - staged, _ := cmd.Flags().GetBool("staged") - approve, _ := cmd.Flags().GetBool("approve") - validateImages, _ := cmd.Flags().GetBool("validate-images") - priorManifestFile, _ := cmd.Flags().GetString("prior-manifest-file") - ghes, _ := cmd.Flags().GetBool("ghes") - verbose, _ := cmd.Flags().GetBool("verbose") - useSamples, _ := cmd.Flags().GetBool("use-samples") - if err := validateEngine(engineOverride); err != nil { - return err - } - - finishCompileUpdateCheck := cli.StartCompileUpdateCheck(cmd.Context(), noCheckUpdate, verbose) - defer finishCompileUpdateCheck() - - // If --fix is specified, run fix --write first - if fix { - fixConfig := cli.FixConfig{ - WorkflowIDs: args, - Write: true, - Verbose: verbose, - WorkflowDir: dir, - } - if err := cli.RunFix(fixConfig); err != nil { - return err - } - } - - // Handle --workflows-dir deprecation (mutual exclusion is enforced by Cobra) - workflowDir := dir - if workflowsDir != "" { - workflowDir = workflowsDir - } - config := cli.CompileConfig{ - MarkdownFiles: args, - Verbose: verbose, - EngineOverride: engineOverride, - ActionMode: actionMode, - ActionTag: actionTag, - ActionsRepo: actionsRepo, - Validate: validate, - Watch: watch, - WorkflowDir: workflowDir, - SkipInstructions: false, // Deprecated field, kept for backward compatibility - NoEmit: noEmit, - Purge: purge, - TrialMode: trial, - TrialLogicalRepoSlug: logicalRepo, - Strict: strict, - Dependabot: dependabot, - ForceOverwrite: forceOverwrite, - RefreshStopTime: refreshStopTime, - ForceRefreshActionPins: forceRefreshActionPins, - AllowActionRefs: allowActionRefs, - Zizmor: zizmor, - Poutine: poutine, - Actionlint: actionlint, - RunnerGuard: runnerGuard, - Syft: syft, - Grype: grype, - Grant: grant, - Yamllint: yamllint, - JSONOutput: jsonOutput, - ShowAllErrors: showAllErrors, - Stats: stats, - FailFast: failFast, - ScheduleSeed: scheduleSeed, - Staged: staged, - Approve: approve, - ValidateImages: validateImages, - PriorManifestFile: priorManifestFile, - GHESCompat: ghes, - UseSamples: useSamples, - } - if _, err := cli.CompileWorkflows(cmd.Context(), config); err != nil { - // Return error as-is without additional formatting - // Errors from CompileWorkflows are already formatted with console.FormatError - // which provides IDE-parseable location information (file:line:column) - return err - } - return nil - }, + RunE: runCompileCmd, } var runCmd = &cobra.Command{ @@ -520,220 +392,352 @@ var versionCmd = &cobra.Command{ }, } -func init() { - // Add command groups to root command - rootCmd.AddGroup(&cobra.Group{ - ID: "setup", - Title: "Setup Commands:", - }) - rootCmd.AddGroup(&cobra.Group{ - ID: "development", - Title: "Development Commands:", - }) - rootCmd.AddGroup(&cobra.Group{ - ID: "execution", - Title: "Execution Commands:", - }) - rootCmd.AddGroup(&cobra.Group{ - ID: "analysis", - Title: "Analysis Commands:", - }) - rootCmd.AddGroup(&cobra.Group{ - ID: "utilities", - Title: "Utilities:", - }) - - // Add global verbose flag to root command - rootCmd.PersistentFlags().BoolVarP(&verboseFlag, "verbose", "v", false, "Enable verbose output showing detailed information") +type compileCmdOptions struct { + engineOverride string + actionMode string + actionTag string + actionsRepo string + ghAwRef string + dir string + workflowsDir string + logicalRepo string + scheduleSeed string + priorManifestFile string + validate bool + watch bool + noEmit bool + purge bool + strict bool + trial bool + dependabot bool + forceOverwrite bool + refreshStopTime bool + forceRefreshActionPins bool + allowActionRefs bool + zizmor bool + poutine bool + actionlint bool + runnerGuard bool + syft bool + grype bool + grant bool + yamllint bool + jsonOutput bool + showAllErrors bool + fix bool + stats bool + failFast bool + noCheckUpdate bool + staged bool + approve bool + validateImages bool + ghes bool + verbose bool + useSamples bool +} - // Add global banner flag to root command - rootCmd.PersistentFlags().BoolVar(&bannerFlag, "banner", false, "Display ASCII logo banner with purple GitHub color theme") +func getCompileCmdOptions(cmd *cobra.Command) compileCmdOptions { + engineOverride, _ := cmd.Flags().GetString("engine") + actionMode, _ := cmd.Flags().GetString("action-mode") + actionTag, _ := cmd.Flags().GetString("action-tag") + actionsRepo, _ := cmd.Flags().GetString("actions-repo") + ghAwRef, _ := cmd.Flags().GetString("gh-aw-ref") + validate, _ := cmd.Flags().GetBool("validate") + watch, _ := cmd.Flags().GetBool("watch") + dir, _ := cmd.Flags().GetString("dir") + workflowsDir, _ := cmd.Flags().GetString("workflows-dir") + noEmit, _ := cmd.Flags().GetBool("no-emit") + purge, _ := cmd.Flags().GetBool("purge") + strict, _ := cmd.Flags().GetBool("strict") + trial, _ := cmd.Flags().GetBool("trial") + logicalRepo, _ := cmd.Flags().GetString("logical-repo") + dependabot, _ := cmd.Flags().GetBool("dependabot") + forceOverwrite, _ := cmd.Flags().GetBool("force") + refreshStopTime, _ := cmd.Flags().GetBool("refresh-stop-time") + forceRefreshActionPins, _ := cmd.Flags().GetBool("force-refresh-action-pins") + allowActionRefs, _ := cmd.Flags().GetBool("allow-action-refs") + zizmor, _ := cmd.Flags().GetBool("zizmor") + poutine, _ := cmd.Flags().GetBool("poutine") + actionlint, _ := cmd.Flags().GetBool("actionlint") + runnerGuard, _ := cmd.Flags().GetBool("runner-guard") + syft, _ := cmd.Flags().GetBool("syft") + grype, _ := cmd.Flags().GetBool("grype") + grant, _ := cmd.Flags().GetBool("grant") + yamllint, _ := cmd.Flags().GetBool("yamllint") + jsonOutput, _ := cmd.Flags().GetBool("json") + showAllErrors, _ := cmd.Flags().GetBool("show-all") + fix, _ := cmd.Flags().GetBool("fix") + stats, _ := cmd.Flags().GetBool("stats") + failFast, _ := cmd.Flags().GetBool("fail-fast") + noCheckUpdate, _ := cmd.Flags().GetBool("no-check-update") + scheduleSeed, _ := cmd.Flags().GetString("schedule-seed") + staged, _ := cmd.Flags().GetBool("staged") + approve, _ := cmd.Flags().GetBool("approve") + validateImages, _ := cmd.Flags().GetBool("validate-images") + priorManifestFile, _ := cmd.Flags().GetString("prior-manifest-file") + ghes, _ := cmd.Flags().GetBool("ghes") + verbose, _ := cmd.Flags().GetBool("verbose") + useSamples, _ := cmd.Flags().GetBool("use-samples") + return compileCmdOptions{ + engineOverride: engineOverride, actionMode: actionMode, actionTag: actionTag, actionsRepo: actionsRepo, ghAwRef: ghAwRef, + dir: dir, workflowsDir: workflowsDir, logicalRepo: logicalRepo, scheduleSeed: scheduleSeed, priorManifestFile: priorManifestFile, + validate: validate, watch: watch, noEmit: noEmit, purge: purge, strict: strict, trial: trial, dependabot: dependabot, + forceOverwrite: forceOverwrite, refreshStopTime: refreshStopTime, forceRefreshActionPins: forceRefreshActionPins, allowActionRefs: allowActionRefs, + zizmor: zizmor, poutine: poutine, actionlint: actionlint, runnerGuard: runnerGuard, syft: syft, grype: grype, grant: grant, yamllint: yamllint, + jsonOutput: jsonOutput, showAllErrors: showAllErrors, fix: fix, stats: stats, failFast: failFast, noCheckUpdate: noCheckUpdate, + staged: staged, approve: approve, validateImages: validateImages, ghes: ghes, verbose: verbose, useSamples: useSamples, + } +} - // Set output to stderr for consistency with CLI logging guidelines - rootCmd.SetOut(os.Stderr) +func (o *compileCmdOptions) resolveGhAwRef(ctx context.Context) error { + if o.ghAwRef == "" { + return nil + } + resolvedRef, resolveErr := workflow.ResolveGhAwRef(ctx, o.ghAwRef) + if resolveErr != nil { + return fmt.Errorf("--gh-aw-ref: %w", resolveErr) + } + o.actionMode = string(workflow.ActionModeRelease) + o.actionTag = resolvedRef + return nil +} - // Silence usage output on errors - prevents cluttering terminal output with - // full usage text when application errors occur (e.g., compilation errors, - // network timeouts). Users can still run --help for usage information. - rootCmd.SilenceUsage = true +func (o *compileCmdOptions) workflowDir() string { + if o.workflowsDir != "" { + return o.workflowsDir + } + return o.dir +} - // Silence errors - since we're using RunE and returning errors, Cobra will - // print errors automatically. We handle error formatting ourselves in main(). - rootCmd.SilenceErrors = true +func (o *compileCmdOptions) toCompileConfig(args []string) cli.CompileConfig { + return cli.CompileConfig{ + MarkdownFiles: args, Verbose: o.verbose, EngineOverride: o.engineOverride, ActionMode: o.actionMode, ActionTag: o.actionTag, + ActionsRepo: o.actionsRepo, Validate: o.validate, Watch: o.watch, WorkflowDir: o.workflowDir(), SkipInstructions: false, + NoEmit: o.noEmit, Purge: o.purge, TrialMode: o.trial, TrialLogicalRepoSlug: o.logicalRepo, Strict: o.strict, + Dependabot: o.dependabot, ForceOverwrite: o.forceOverwrite, RefreshStopTime: o.refreshStopTime, ForceRefreshActionPins: o.forceRefreshActionPins, + AllowActionRefs: o.allowActionRefs, Zizmor: o.zizmor, Poutine: o.poutine, Actionlint: o.actionlint, RunnerGuard: o.runnerGuard, + Syft: o.syft, Grype: o.grype, Grant: o.grant, Yamllint: o.yamllint, JSONOutput: o.jsonOutput, ShowAllErrors: o.showAllErrors, + Stats: o.stats, FailFast: o.failFast, ScheduleSeed: o.scheduleSeed, Staged: o.staged, Approve: o.approve, + ValidateImages: o.validateImages, PriorManifestFile: o.priorManifestFile, GHESCompat: o.ghes, UseSamples: o.useSamples, + } +} - // Set version template to match the version subcommand format - rootCmd.SetVersionTemplate(string(constants.CLIExtensionPrefix) + " version {{.Version}}\n") +func runCompileCmd(cmd *cobra.Command, args []string) error { + opts := getCompileCmdOptions(cmd) + if err := opts.resolveGhAwRef(cmd.Context()); err != nil { + return err + } + if err := validateEngine(opts.engineOverride); err != nil { + return err + } + finishCompileUpdateCheck := cli.StartCompileUpdateCheck(cmd.Context(), opts.noCheckUpdate, opts.verbose) + defer finishCompileUpdateCheck() + if opts.fix { + if err := cli.RunFix(cli.FixConfig{ + WorkflowIDs: args, + Write: true, + Verbose: opts.verbose, + WorkflowDir: opts.dir, + }); err != nil { + return err + } + } + if _, err := cli.CompileWorkflows(cmd.Context(), opts.toCompileConfig(args)); err != nil { + return err + } + return nil +} - // Cobra generates flag descriptions using c.Name() which returns the first - // word of Use ("gh" from "gh aw"), producing "help for gh" and "version for - // gh". Explicitly initialize and override these flags so they display "gh aw". - rootCmd.InitDefaultHelpFlag() - if f := rootCmd.Flags().Lookup("help"); f != nil { - f.Usage = "Show help for " + string(constants.CLIExtensionPrefix) +type commandSet struct { + addCmd, addWizardCmd, updateCmd, deployCmd, trialCmd, initCmd, statusCmd, listCmd *cobra.Command + mcpCmd, logsCmd, auditCmd, viewCmd, healthCmd, outcomesCmd, mcpServerCmd, prCmd, secretsCmd *cobra.Command + fixCmd, upgradeCmd, completionCmd, hashCmd, projectCmd, doctorCmd, checksCmd, validateCmd, lintCmd *cobra.Command + domainsCmd, experimentsCmd, forecastCmd, envCmd *cobra.Command +} + +func fixPathForCommand(s string) string { + if s == "gh" { + return "gh aw" } - rootCmd.InitDefaultVersionFlag() - if f := rootCmd.Flags().Lookup("version"); f != nil { - f.Usage = "Print the current version" + if strings.HasPrefix(s, "gh ") && !strings.HasPrefix(s, "gh aw") { + return "gh aw " + s[3:] } + return s +} - // Fix usage lines so subcommands show "gh aw " instead of "gh ". - // Cobra derives the root name from the first word of Use ("gh" from "gh aw"), - // so CommandPath() for subcommands omits "aw". We use SetUsageFunc to - // post-process the default output, replacing "gh " with "gh aw " in the - // two lines that reference the command path. - rootCmd.SetUsageFunc(func(cmd *cobra.Command) error { - fixPath := func(s string) string { - if s == "gh" { - return "gh aw" - } - if strings.HasPrefix(s, "gh ") && !strings.HasPrefix(s, "gh aw") { - return "gh aw " + s[3:] - } - return s - } - out := cmd.OutOrStderr() - fmt.Fprint(out, "Usage:") - if cmd.Runnable() { - fmt.Fprintf(out, "\n %s", fixPath(cmd.UseLine())) - } - if cmd.HasAvailableSubCommands() { - fmt.Fprintf(out, "\n %s [command]", fixPath(cmd.CommandPath())) +func writeUsageCommands(cmd *cobra.Command) { + out := cmd.OutOrStderr() + cmds := cmd.Commands() + colWidth := 0 + for _, sub := range cmds { + if (sub.IsAvailableCommand() || sub.Name() == "help") && len(sub.Name()) > colWidth { + colWidth = len(sub.Name()) } - if len(cmd.Aliases) > 0 { - fmt.Fprintf(out, "\n\nAliases:\n %s", cmd.NameAndAliases()) - } - if cmd.HasExample() { - fmt.Fprintf(out, "\n\nExamples:\n%s", cmd.Example) - } - if cmd.HasAvailableSubCommands() { - cmds := cmd.Commands() - // Compute column width dynamically so long command names (e.g. hash-frontmatter) - // are aligned properly instead of overflowing a hard-coded width. - colWidth := 0 - for _, sub := range cmds { - if (sub.IsAvailableCommand() || sub.Name() == "help") && len(sub.Name()) > colWidth { - colWidth = len(sub.Name()) - } - } - colFmt := fmt.Sprintf("\n %%-%ds %%s", colWidth) - if len(cmd.Groups()) == 0 { - fmt.Fprint(out, "\n\nAvailable Commands:") - for _, sub := range cmds { - if sub.IsAvailableCommand() || sub.Name() == "help" { - fmt.Fprintf(out, colFmt, sub.Name(), sub.Short) - } - } - } else { - for _, group := range cmd.Groups() { - fmt.Fprintf(out, "\n\n%s", group.Title) - for _, sub := range cmds { - if sub.GroupID == group.ID && (sub.IsAvailableCommand() || sub.Name() == "help") { - fmt.Fprintf(out, colFmt, sub.Name(), sub.Short) - } - } - } - if !cmd.AllChildCommandsHaveGroup() { - fmt.Fprint(out, "\n\nAdditional Commands:") - for _, sub := range cmds { - if sub.GroupID == "" && (sub.IsAvailableCommand() || sub.Name() == "help") { - fmt.Fprintf(out, colFmt, sub.Name(), sub.Short) - } - } - } + } + colFmt := fmt.Sprintf("\n %%-%ds %%s", colWidth) + if len(cmd.Groups()) == 0 { + fmt.Fprint(out, "\n\nAvailable Commands:") + for _, sub := range cmds { + if sub.IsAvailableCommand() || sub.Name() == "help" { + fmt.Fprintf(out, colFmt, sub.Name(), sub.Short) } } - if cmd.HasAvailableLocalFlags() { - fmt.Fprintf(out, "\n\nFlags:\n%s", strings.TrimRight(cmd.LocalFlags().FlagUsages(), " \t\n")) + return + } + for _, group := range cmd.Groups() { + fmt.Fprintf(out, "\n\n%s", group.Title) + for _, sub := range cmds { + if sub.GroupID == group.ID && (sub.IsAvailableCommand() || sub.Name() == "help") { + fmt.Fprintf(out, colFmt, sub.Name(), sub.Short) + } } - if cmd.HasAvailableInheritedFlags() { - fmt.Fprintf(out, "\n\nGlobal Flags:\n%s", strings.TrimRight(cmd.InheritedFlags().FlagUsages(), " \t\n")) + } + if !cmd.AllChildCommandsHaveGroup() { + fmt.Fprint(out, "\n\nAdditional Commands:") + for _, sub := range cmds { + if sub.GroupID == "" && (sub.IsAvailableCommand() || sub.Name() == "help") { + fmt.Fprintf(out, colFmt, sub.Name(), sub.Short) + } } - if cmd.HasAvailableSubCommands() { - fmt.Fprintf(out, "\n\nUse \"%s [command] --help\" for more information about a command.\n", fixPath(cmd.CommandPath())) - } else { - fmt.Fprintln(out) + } +} + +func rootUsageFunc(cmd *cobra.Command) error { + out := cmd.OutOrStderr() + fmt.Fprint(out, "Usage:") + if cmd.Runnable() { + fmt.Fprintf(out, "\n %s", fixPathForCommand(cmd.UseLine())) + } + if cmd.HasAvailableSubCommands() { + fmt.Fprintf(out, "\n %s [command]", fixPathForCommand(cmd.CommandPath())) + } + if len(cmd.Aliases) > 0 { + fmt.Fprintf(out, "\n\nAliases:\n %s", cmd.NameAndAliases()) + } + if cmd.HasExample() { + fmt.Fprintf(out, "\n\nExamples:\n%s", cmd.Example) + } + if cmd.HasAvailableSubCommands() { + writeUsageCommands(cmd) + } + if cmd.HasAvailableLocalFlags() { + fmt.Fprintf(out, "\n\nFlags:\n%s", strings.TrimRight(cmd.LocalFlags().FlagUsages(), " \t\n")) + } + if cmd.HasAvailableInheritedFlags() { + fmt.Fprintf(out, "\n\nGlobal Flags:\n%s", strings.TrimRight(cmd.InheritedFlags().FlagUsages(), " \t\n")) + } + if cmd.HasAvailableSubCommands() { + fmt.Fprintf(out, "\n\nUse \"%s [command] --help\" for more information about a command.\n", fixPathForCommand(cmd.CommandPath())) + } else { + fmt.Fprintln(out) + } + return nil +} + +func customHelpRunE(c *cobra.Command, args []string) error { + if len(args) == 1 && args[0] == "all" { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("GitHub Agentic Workflows CLI - Complete Command Reference")) + fmt.Fprintln(os.Stderr, "") + for _, subCmd := range rootCmd.Commands() { + if subCmd.Hidden || subCmd.Name() == "help" { + continue + } + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("═══════════════════════════════════════════════════════════════")) + fmt.Fprintf(os.Stderr, "\n%s\n\n", console.FormatInfoMessage(fmt.Sprintf("Command: %s %s", string(constants.CLIExtensionPrefix), subCmd.Name()))) + _ = subCmd.Help() + fmt.Fprintln(os.Stderr, "") } + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("═══════════════════════════════════════════════════════════════")) + fmt.Fprintln(os.Stderr, "") + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("For more information, visit: https://github.github.com/gh-aw/")) return nil - }) + } + cmd, _, e := rootCmd.Find(args) + if cmd == nil || e != nil { + return fmt.Errorf("unknown help topic [%#q]", args) + } + cmd.InitDefaultHelpFlag() + return cmd.Help() +} - // Create custom help command that supports "all" subcommand - customHelpCmd := &cobra.Command{ +func newCustomHelpCmd() *cobra.Command { + return &cobra.Command{ Use: "help [command]", Short: "Help about any command", Long: `Help provides help for any command in the application. Simply type ` + string(constants.CLIExtensionPrefix) + ` help [path to command] for full details. Use "` + string(constants.CLIExtensionPrefix) + ` help all" to show help for all commands.`, - RunE: func(c *cobra.Command, args []string) error { - // Check if the argument is "all" - if len(args) == 1 && args[0] == "all" { - // Print header - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("GitHub Agentic Workflows CLI - Complete Command Reference")) - fmt.Fprintln(os.Stderr, "") - - // Iterate through all commands and print their help - for _, subCmd := range rootCmd.Commands() { - // Skip hidden commands and help itself - if subCmd.Hidden || subCmd.Name() == "help" { - continue - } - - // Print command separator - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("═══════════════════════════════════════════════════════════════")) - fmt.Fprintf(os.Stderr, "\n%s\n\n", console.FormatInfoMessage(fmt.Sprintf("Command: %s %s", string(constants.CLIExtensionPrefix), subCmd.Name()))) - - // Print the command's help - _ = subCmd.Help() - fmt.Fprintln(os.Stderr, "") - } - - // Print footer - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("═══════════════════════════════════════════════════════════════")) - fmt.Fprintln(os.Stderr, "") - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("For more information, visit: https://github.github.com/gh-aw/")) - return nil - } - - // Otherwise, use the default help behavior - cmd, _, e := rootCmd.Find(args) - if cmd == nil || e != nil { - return fmt.Errorf("unknown help topic [%#q]", args) - } else { - cmd.InitDefaultHelpFlag() // make possible 'help' flag to be shown - return cmd.Help() - } - }, + RunE: customHelpRunE, } +} - // Replace the default help command - rootCmd.SetHelpCommand(customHelpCmd) - - // Create and setup add command - addCmd := cli.NewAddCommand(validateEngine) - - // Create and setup add-wizard command - addWizardCmd := cli.NewAddWizardCommand(validateEngine) - - // Create and setup update command - updateCmd := cli.NewUpdateCommand(validateEngine) - - // Create and setup deploy command - deployCmd := cli.NewDeployCommand(validateEngine) - - // Create and setup trial command - trialCmd := cli.NewTrialCommand(validateEngine) +func configureRootCommand() { + rootCmd.AddGroup(&cobra.Group{ID: "setup", Title: "Setup Commands:"}) + rootCmd.AddGroup(&cobra.Group{ID: "development", Title: "Development Commands:"}) + rootCmd.AddGroup(&cobra.Group{ID: "execution", Title: "Execution Commands:"}) + rootCmd.AddGroup(&cobra.Group{ID: "analysis", Title: "Analysis Commands:"}) + rootCmd.AddGroup(&cobra.Group{ID: "utilities", Title: "Utilities:"}) + rootCmd.PersistentFlags().BoolVarP(&verboseFlag, "verbose", "v", false, "Enable verbose output showing detailed information") + rootCmd.PersistentFlags().BoolVar(&bannerFlag, "banner", false, "Display ASCII logo banner with purple GitHub color theme") + rootCmd.SetOut(os.Stderr) + rootCmd.SilenceUsage = true + rootCmd.SilenceErrors = true + rootCmd.SetVersionTemplate(string(constants.CLIExtensionPrefix) + " version {{.Version}}\n") + rootCmd.InitDefaultHelpFlag() + if f := rootCmd.Flags().Lookup("help"); f != nil { + f.Usage = "Show help for " + string(constants.CLIExtensionPrefix) + } + rootCmd.InitDefaultVersionFlag() + if f := rootCmd.Flags().Lookup("version"); f != nil { + f.Usage = "Print the current version" + } + rootCmd.SetUsageFunc(rootUsageFunc) +} - // Create and setup init command - initCmd := cli.NewInitCommand() - cli.RegisterEngineFlagCompletion(initCmd) +func createCommandSet() commandSet { + cmds := commandSet{ + addCmd: cli.NewAddCommand(validateEngine), + addWizardCmd: cli.NewAddWizardCommand(validateEngine), + updateCmd: cli.NewUpdateCommand(validateEngine), + deployCmd: cli.NewDeployCommand(validateEngine), + trialCmd: cli.NewTrialCommand(validateEngine), + initCmd: cli.NewInitCommand(), + statusCmd: cli.NewStatusCommand(), + listCmd: cli.NewListCommand(), + mcpCmd: cli.NewMCPCommand(), + logsCmd: cli.NewLogsCommand(), + auditCmd: cli.NewAuditCommand(), + viewCmd: cli.NewViewCommand(), + healthCmd: cli.NewHealthCommand(), + outcomesCmd: cli.NewOutcomesCommand(), + mcpServerCmd: cli.NewMCPServerCommand(), + prCmd: cli.NewPRCommand(), + secretsCmd: cli.NewSecretsCommand(), + fixCmd: cli.NewFixCommand(), + upgradeCmd: cli.NewUpgradeCommand(validateEngine), + completionCmd: cli.NewCompletionCommand(), + hashCmd: cli.NewHashCommand(), + projectCmd: cli.NewProjectCommand(), + doctorCmd: cli.NewDoctorCommand(), + checksCmd: cli.NewChecksCommand(), + validateCmd: cli.NewValidateCommand(validateEngine), + lintCmd: cli.NewLintCommand(), + domainsCmd: cli.NewDomainsCommand(), + experimentsCmd: cli.NewExperimentsCommand(), + forecastCmd: cli.NewForecastCommand(), + envCmd: cli.NewEnvCommand(), + } + cli.RegisterEngineFlagCompletion(cmds.initCmd) + return cmds +} - // Add flags to new command +func configureNewAndCompileFlags() { newCmd.Flags().BoolP("force", "f", false, "Overwrite existing workflow files without confirmation") newCmd.Flags().BoolP("interactive", "i", false, "Launch interactive workflow creation wizard") newCmd.Flags().StringP("engine", "e", "", cli.EngineFlagOverrideUsage) cli.RegisterEngineFlagCompletion(newCmd) - // Add AI flag to compile and add commands compileCmd.Flags().StringP("engine", "e", "", cli.EngineFlagOverrideUsage) compileCmd.Flags().String("action-mode", "", "How gh-aw action scripts are referenced in compiled workflows: 'dev' uses local paths (for developing gh-aw itself), 'release' emits SHA-pinned remote refs from github/gh-aw, 'action' uses the github/gh-aw-actions repository. Auto-detected from the binary build type if not specified") compileCmd.Flags().String("action-tag", "", "Pin compiled workflows to a specific version of gh-aw actions. Accepts a full commit SHA or a version tag (e.g. v1, v1.2.3). Sets --action-mode to 'release' unless --action-mode action is also specified. Cannot be combined with --gh-aw-ref; use --gh-aw-ref when you want to resolve a branch or tag name to its current SHA") @@ -751,6 +755,9 @@ Use "` + string(constants.CLIExtensionPrefix) + ` help all" to show help for all compileCmd.Flags().StringP("logical-repo", "l", "", "Repository to simulate workflow execution against (for trial mode)") compileCmd.Flags().Bool("use-samples", false, "Hidden: replace the agentic 'Execute coding agent' step with a deterministic driver that replays the workflow's safe-outputs `samples` frontmatter entries through the safe-outputs MCP server. Used to make end-to-end tests deterministic.") _ = compileCmd.Flags().MarkHidden("use-samples") +} + +func configureCompileFlagsContinued() { compileCmd.Flags().Bool("dependabot", false, "Generate dependency manifests (package.json, requirements.txt, go.mod) and Dependabot config when dependencies are detected") compileCmd.Flags().BoolP("force", "f", false, "Force overwrite of existing dependency files (only applies when --dependabot is set; e.g., dependabot.yml)") compileCmd.Flags().Bool("refresh-stop-time", false, "Force regeneration of stop-after times instead of preserving existing values from lock files") @@ -776,40 +783,33 @@ Use "` + string(constants.CLIExtensionPrefix) + ` help all" to show help for all compileCmd.Flags().Bool("validate-images", false, "Require Docker to be available for container image validation. Without this flag, container image validation is silently skipped when Docker is not installed or the daemon is not running") compileCmd.Flags().String("prior-manifest-file", "", "Path to a JSON file containing pre-cached gh-aw-manifests (map[lockFile]*GHAWManifest); used by the MCP server to supply a tamper-proof manifest baseline captured at startup") compileCmd.Flags().Bool("ghes", false, "Enable GitHub Enterprise Server (GHES) compatibility mode. Artifact actions continue using latest non-v3 pins (v3 is deprecated). Overrides the aw.json ghes field.") +} + +func finalizeCompileFlagSetup() { if err := compileCmd.Flags().MarkHidden("prior-manifest-file"); err != nil { - // Non-fatal: flag is registered even if MarkHidden fails _ = err } compileCmd.MarkFlagsMutuallyExclusive("dir", "workflows-dir") - // --gh-aw-ref is a convenience alias for --action-mode release --action-tag ; - // combining it with either of those flags leads to one silently overwriting the other. compileCmd.MarkFlagsMutuallyExclusive("gh-aw-ref", "action-tag") compileCmd.MarkFlagsMutuallyExclusive("gh-aw-ref", "action-mode") - - // Register completions for compile command compileCmd.ValidArgsFunction = cli.CompleteWorkflowNames cli.RegisterEngineFlagCompletion(compileCmd) cli.RegisterDirFlagCompletion(compileCmd, "dir") +} - rootCmd.AddCommand(compileCmd) - - // Add flags to remove command +func configureOtherCommandFlags() { removeCmd.Flags().Bool("no-remove-orphans", false, "Skip removal of orphaned include files that are no longer referenced by any workflow") removeCmd.Flags().Bool("keep-orphans", false, "Skip removal of orphaned include files that are no longer referenced by any workflow") _ = removeCmd.Flags().MarkDeprecated("keep-orphans", "use --no-remove-orphans instead") removeCmd.Flags().StringP("dir", "d", "", "Workflow directory (default: $GH_AW_WORKFLOWS_DIR or .github/workflows)") - // Register completions for remove command removeCmd.ValidArgsFunction = cli.CompleteWorkflowNames cli.RegisterDirFlagCompletion(removeCmd, "dir") - // Add flags to enable/disable commands enableCmd.Flags().StringP("repo", "r", "", "Target repository ([HOST/]owner/repo format). Defaults to current repository") disableCmd.Flags().StringP("repo", "r", "", "Target repository ([HOST/]owner/repo format). Defaults to current repository") - // Register completions for enable/disable commands enableCmd.ValidArgsFunction = cli.CompleteWorkflowNames disableCmd.ValidArgsFunction = cli.CompleteWorkflowNames - // Add flags to run command runCmd.Flags().Int("repeat", 0, "Number of additional times to run after the initial execution (e.g., --repeat 3 runs 4 times total)") runCmd.Flags().Bool("enable-if-needed", false, "Enable the workflow before running if needed, and restore state afterward") runCmd.Flags().StringP("engine", "e", "", cli.EngineFlagOverrideUsage) @@ -822,153 +822,62 @@ Use "` + string(constants.CLIExtensionPrefix) + ` help all" to show help for all runCmd.Flags().Bool("dry-run", false, "Preview workflow execution without triggering runs on GitHub Actions") runCmd.Flags().BoolP("json", "j", false, "Output results in JSON format") runCmd.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") - // Register completions for run command runCmd.ValidArgsFunction = cli.CompleteWorkflowNames cli.RegisterEngineFlagCompletion(runCmd) +} - // Create and setup status command - statusCmd := cli.NewStatusCommand() - - // Create and setup list command - listCmd := cli.NewListCommand() - - // Create commands that need group assignment - mcpCmd := cli.NewMCPCommand() - logsCmd := cli.NewLogsCommand() - auditCmd := cli.NewAuditCommand() - viewCmd := cli.NewViewCommand() - healthCmd := cli.NewHealthCommand() - outcomesCmd := cli.NewOutcomesCommand() - mcpServerCmd := cli.NewMCPServerCommand() - prCmd := cli.NewPRCommand() - secretsCmd := cli.NewSecretsCommand() - fixCmd := cli.NewFixCommand() - upgradeCmd := cli.NewUpgradeCommand(validateEngine) - completionCmd := cli.NewCompletionCommand() - hashCmd := cli.NewHashCommand() - projectCmd := cli.NewProjectCommand() - doctorCmd := cli.NewDoctorCommand() - checksCmd := cli.NewChecksCommand() - validateCmd := cli.NewValidateCommand(validateEngine) - lintCmd := cli.NewLintCommand() - domainsCmd := cli.NewDomainsCommand() - experimentsCmd := cli.NewExperimentsCommand() - forecastCmd := cli.NewForecastCommand() - envCmd := cli.NewEnvCommand() - - // Assign commands to groups - // Setup Commands - initCmd.GroupID = "setup" - newCmd.GroupID = "setup" - addCmd.GroupID = "setup" - addWizardCmd.GroupID = "setup" - removeCmd.GroupID = "setup" - updateCmd.GroupID = "setup" - deployCmd.GroupID = "setup" - upgradeCmd.GroupID = "setup" - secretsCmd.GroupID = "setup" - envCmd.GroupID = "setup" - doctorCmd.GroupID = "setup" - - // Development Commands - compileCmd.GroupID = "development" - validateCmd.GroupID = "development" - lintCmd.GroupID = "development" - mcpCmd.GroupID = "development" - fixCmd.GroupID = "development" - domainsCmd.GroupID = "development" - - // Execution Commands - runCmd.GroupID = "execution" - enableCmd.GroupID = "execution" - disableCmd.GroupID = "execution" - trialCmd.GroupID = "execution" - - // Analysis Commands - logsCmd.GroupID = "analysis" - auditCmd.GroupID = "analysis" - viewCmd.GroupID = "analysis" - healthCmd.GroupID = "analysis" - outcomesCmd.GroupID = "analysis" - checksCmd.GroupID = "analysis" - statusCmd.GroupID = "analysis" - listCmd.GroupID = "analysis" - experimentsCmd.GroupID = "analysis" - forecastCmd.GroupID = "analysis" - - // Utilities - mcpServerCmd.GroupID = "utilities" - prCmd.GroupID = "utilities" - completionCmd.GroupID = "utilities" - hashCmd.GroupID = "utilities" - projectCmd.GroupID = "utilities" - - // version command is intentionally left without a group (common practice) - - // Add all commands to root - rootCmd.AddCommand(addCmd) - rootCmd.AddCommand(addWizardCmd) - rootCmd.AddCommand(updateCmd) - rootCmd.AddCommand(deployCmd) - rootCmd.AddCommand(upgradeCmd) - rootCmd.AddCommand(trialCmd) - rootCmd.AddCommand(newCmd) - rootCmd.AddCommand(initCmd) - - rootCmd.AddCommand(runCmd) - rootCmd.AddCommand(removeCmd) - rootCmd.AddCommand(statusCmd) - rootCmd.AddCommand(listCmd) - rootCmd.AddCommand(enableCmd) - rootCmd.AddCommand(disableCmd) - rootCmd.AddCommand(logsCmd) - rootCmd.AddCommand(auditCmd) - rootCmd.AddCommand(viewCmd) - rootCmd.AddCommand(healthCmd) - rootCmd.AddCommand(outcomesCmd) - rootCmd.AddCommand(checksCmd) - rootCmd.AddCommand(mcpCmd) - rootCmd.AddCommand(mcpServerCmd) - rootCmd.AddCommand(prCmd) - rootCmd.AddCommand(versionCmd) - rootCmd.AddCommand(secretsCmd) - rootCmd.AddCommand(fixCmd) - rootCmd.AddCommand(validateCmd) - rootCmd.AddCommand(lintCmd) - rootCmd.AddCommand(completionCmd) - rootCmd.AddCommand(hashCmd) - rootCmd.AddCommand(projectCmd) - rootCmd.AddCommand(doctorCmd) - rootCmd.AddCommand(domainsCmd) - rootCmd.AddCommand(experimentsCmd) - rootCmd.AddCommand(forecastCmd) - rootCmd.AddCommand(envCmd) - - // Fix help flag descriptions for all subcommands to be consistent with the - // root command ("Show help for gh aw" vs the Cobra default "help for [cmd]"). - var fixSubCmdHelpFlags func(cmd *cobra.Command) - fixSubCmdHelpFlags = func(cmd *cobra.Command) { +func assignCommandGroups(cmds commandSet) { + cmds.initCmd.GroupID, newCmd.GroupID, cmds.addCmd.GroupID, cmds.addWizardCmd.GroupID = "setup", "setup", "setup", "setup" + removeCmd.GroupID, cmds.updateCmd.GroupID, cmds.deployCmd.GroupID, cmds.upgradeCmd.GroupID = "setup", "setup", "setup", "setup" + cmds.secretsCmd.GroupID, cmds.envCmd.GroupID, cmds.doctorCmd.GroupID = "setup", "setup", "setup" + compileCmd.GroupID, cmds.validateCmd.GroupID, cmds.lintCmd.GroupID = "development", "development", "development" + cmds.mcpCmd.GroupID, cmds.fixCmd.GroupID, cmds.domainsCmd.GroupID = "development", "development", "development" + runCmd.GroupID, enableCmd.GroupID, disableCmd.GroupID, cmds.trialCmd.GroupID = "execution", "execution", "execution", "execution" + cmds.logsCmd.GroupID, cmds.auditCmd.GroupID, cmds.viewCmd.GroupID = "analysis", "analysis", "analysis" + cmds.healthCmd.GroupID, cmds.outcomesCmd.GroupID, cmds.checksCmd.GroupID = "analysis", "analysis", "analysis" + cmds.statusCmd.GroupID, cmds.listCmd.GroupID, cmds.experimentsCmd.GroupID, cmds.forecastCmd.GroupID = "analysis", "analysis", "analysis", "analysis" + cmds.mcpServerCmd.GroupID, cmds.prCmd.GroupID, cmds.completionCmd.GroupID, cmds.hashCmd.GroupID, cmds.projectCmd.GroupID = "utilities", "utilities", "utilities", "utilities", "utilities" +} + +func addCommandsToRoot(cmds commandSet) { + rootCmd.AddCommand( + compileCmd, cmds.addCmd, cmds.addWizardCmd, cmds.updateCmd, cmds.deployCmd, cmds.upgradeCmd, cmds.trialCmd, newCmd, cmds.initCmd, + runCmd, removeCmd, cmds.statusCmd, cmds.listCmd, enableCmd, disableCmd, cmds.logsCmd, cmds.auditCmd, cmds.viewCmd, + cmds.healthCmd, cmds.outcomesCmd, cmds.checksCmd, cmds.mcpCmd, cmds.mcpServerCmd, cmds.prCmd, versionCmd, cmds.secretsCmd, + cmds.fixCmd, cmds.validateCmd, cmds.lintCmd, cmds.completionCmd, cmds.hashCmd, cmds.projectCmd, cmds.doctorCmd, + cmds.domainsCmd, cmds.experimentsCmd, cmds.forecastCmd, cmds.envCmd, + ) +} + +func fixSubCommandHelpFlags() { + var fix func(cmd *cobra.Command) + fix = func(cmd *cobra.Command) { cmd.InitDefaultHelpFlag() if f := cmd.Flags().Lookup("help"); f != nil { - cmdPath := cmd.CommandPath() - // CommandPath() uses Name() which returns the first word of Use - // ("gh" from "gh aw"), so subcommand paths look like "gh compile". - // Replace the leading "gh " prefix with "gh aw " to match the root - // command's display name. - if strings.HasPrefix(cmdPath, "gh ") && !strings.HasPrefix(cmdPath, "gh aw") { - cmdPath = "gh aw " + cmdPath[3:] - } - f.Usage = "Show help for " + cmdPath + f.Usage = "Show help for " + fixPathForCommand(cmd.CommandPath()) } for _, sub := range cmd.Commands() { - fixSubCmdHelpFlags(sub) + fix(sub) } } for _, sub := range rootCmd.Commands() { - fixSubCmdHelpFlags(sub) + fix(sub) } } +func init() { + configureRootCommand() + rootCmd.SetHelpCommand(newCustomHelpCmd()) + cmds := createCommandSet() + configureNewAndCompileFlags() + configureCompileFlagsContinued() + finalizeCompileFlagSetup() + configureOtherCommandFlags() + assignCommandGroups(cmds) + addCommandsToRoot(cmds) + fixSubCommandHelpFlags() +} + func main() { // Set version information in the CLI package cli.SetVersionInfo(version) diff --git a/pkg/actionpins/actionpins_internal_test.go b/pkg/actionpins/actionpins_internal_test.go index 8783bbe13de..c3c1c1755fd 100644 --- a/pkg/actionpins/actionpins_internal_test.go +++ b/pkg/actionpins/actionpins_internal_test.go @@ -168,15 +168,13 @@ func TestInitWarnings_InitializesAndPreservesMap(t *testing.T) { }) t.Run("preserves existing warnings map", func(t *testing.T) { - // Build expected independently so a mutation to ctx.Warnings cannot silently - // satisfy the assertion (both sides would change if they shared a pointer). - expected := map[string]bool{"actions/checkout@v5": true} ctx := &PinContext{Warnings: map[string]bool{"actions/checkout@v5": true}} initWarnings(ctx) require.NotNil(t, ctx.Warnings, "Expected warnings map to remain initialized") - assert.Equal(t, expected, ctx.Warnings, "Expected existing warnings to be preserved unchanged") + assert.Len(t, ctx.Warnings, 1, "Expected existing warnings to be preserved unchanged") + assert.True(t, ctx.Warnings["actions/checkout@v5"], "Expected existing warnings to be preserved unchanged") }) } diff --git a/pkg/linters/stringsconcatloop/stringsconcatloop.go b/pkg/linters/stringsconcatloop/stringsconcatloop.go index a52b727e9b1..53896d2465f 100644 --- a/pkg/linters/stringsconcatloop/stringsconcatloop.go +++ b/pkg/linters/stringsconcatloop/stringsconcatloop.go @@ -45,85 +45,93 @@ func run(pass *analysis.Pass) (any, error) { } for cur := range root.Preorder((*ast.AssignStmt)(nil)) { - assign, ok := cur.Node().(*ast.AssignStmt) + assign, lhsExpr, loopNode, pos, ok := collectConcatLoopAssignment(pass, cur, noLintIndex, generatedFiles) if !ok { continue } - - // Match both `x += y` (ADD_ASSIGN) and `x = x + y` (ASSIGN with a - // self-referential binary addition on a plain identifier). - var lhsExpr ast.Expr - - switch assign.Tok { - case token.ADD_ASSIGN: - if len(assign.Lhs) != 1 { - continue - } - lhsExpr = assign.Lhs[0] - case token.ASSIGN: - if len(assign.Lhs) != 1 || len(assign.Rhs) != 1 { - continue - } - lhsIdent, ok := assign.Lhs[0].(*ast.Ident) - if !ok { - continue - } - binExpr, ok := assign.Rhs[0].(*ast.BinaryExpr) - if !ok || binExpr.Op != token.ADD { - continue - } - // Only match the direct self-referential form: x = x + rhs, - // where x is the same identifier on both sides (not chained - // forms like x = x + a + b which parse with a BinaryExpr on - // the left of the outer add, not an Ident). - rhsLeft, ok := binExpr.X.(*ast.Ident) - if !ok || rhsLeft.Name != lhsIdent.Name { - continue - } - lhsExpr = lhsIdent - default: - continue - } - - pos := pass.Fset.PositionFor(assign.Pos(), false) - if filecheck.ShouldSkipFilename(pos.Filename, generatedFiles) { + if !shouldReportLoopConcat(pass, loopNode, lhsExpr) { continue } + pkgLog.Printf("flagging string concatenation in loop at %s", pos) + pass.ReportRangef(assign, "string concatenation inside a loop allocates O(n) strings and O(n²) total bytes; use strings.Builder instead") + } - loopPos, loopNode, inLoop := enclosingLoop(pass, cur) - if !inLoop { - continue - } - if nolint.HasDirectiveForLinter(pos, noLintIndex, "stringsconcatloop") { - continue - } - if nolint.HasDirectiveForLinter(loopPos, noLintIndex, "stringsconcatloop") { - continue - } + return nil, nil +} - if !astutil.IsStringType(pass, lhsExpr) { - continue - } +func collectConcatLoopAssignment( + pass *analysis.Pass, + cur inspector.Cursor, + noLintIndex nolint.DirectiveIndex, + generatedFiles filecheck.GeneratedIndex, +) (*ast.AssignStmt, ast.Expr, ast.Node, token.Position, bool) { + assign, ok := cur.Node().(*ast.AssignStmt) + if !ok { + return nil, nil, nil, token.Position{}, false + } + lhsExpr, ok := concatAssignmentLHS(assign) + if !ok { + return nil, nil, nil, token.Position{}, false + } + pos := pass.Fset.PositionFor(assign.Pos(), false) + if filecheck.ShouldSkipFilename(pos.Filename, generatedFiles) { + return nil, nil, nil, token.Position{}, false + } + loopPos, loopNode, inLoop := enclosingLoop(pass, cur) + if !inLoop { + return nil, nil, nil, token.Position{}, false + } + if nolint.HasDirectiveForLinter(pos, noLintIndex, "stringsconcatloop") || nolint.HasDirectiveForLinter(loopPos, noLintIndex, "stringsconcatloop") { + return nil, nil, nil, token.Position{}, false + } + return assign, lhsExpr, loopNode, pos, true +} - // Skip variables that are per-iteration rather than cross-iteration - // accumulators. These checks apply only when the LHS is a plain - // identifier (the ADD_ASSIGN form also accepts field/index lvalues, - // but those can only be tested via the ASSIGN form which requires Ident). - if lhsIdent, ok := lhsExpr.(*ast.Ident); ok { - if isLoopScopedIdent(loopNode, lhsIdent.Name) { - continue - } - if isLoopBodyLocal(pass, loopNode, lhsIdent) { - continue - } +func concatAssignmentLHS(assign *ast.AssignStmt) (ast.Expr, bool) { + switch assign.Tok { + case token.ADD_ASSIGN: + if len(assign.Lhs) != 1 { + return nil, false } + return assign.Lhs[0], true + case token.ASSIGN: + return selfReferentialConcatLHS(assign) + default: + return nil, false + } +} - pkgLog.Printf("flagging string concatenation in loop at %s", pos) - pass.ReportRangef(assign, - "string concatenation inside a loop allocates O(n) strings and O(n²) total bytes; use strings.Builder instead") +func selfReferentialConcatLHS(assign *ast.AssignStmt) (ast.Expr, bool) { + if len(assign.Lhs) != 1 || len(assign.Rhs) != 1 { + return nil, false + } + lhsIdent, ok := assign.Lhs[0].(*ast.Ident) + if !ok { + return nil, false + } + binExpr, ok := assign.Rhs[0].(*ast.BinaryExpr) + if !ok || binExpr.Op != token.ADD { + return nil, false } + rhsLeft, ok := binExpr.X.(*ast.Ident) + if !ok || rhsLeft.Name != lhsIdent.Name { + return nil, false + } + return lhsIdent, true +} - return nil, nil +func shouldReportLoopConcat(pass *analysis.Pass, loopNode ast.Node, lhsExpr ast.Expr) bool { + if !astutil.IsStringType(pass, lhsExpr) { + return false + } + lhsIdent, ok := lhsExpr.(*ast.Ident) + if !ok { + return true + } + if isLoopScopedIdent(loopNode, lhsIdent.Name) { + return false + } + return !isLoopBodyLocal(pass, loopNode, lhsIdent) } // enclosingLoop returns the nearest enclosing for/range statement, its source From 882d0ea292ae634521b78bf0d5156236416a8630 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 2 Aug 2026 13:41:02 +0000 Subject: [PATCH 3/3] Address github-actions review: concatLoopMatch struct, exact equality assertion, semantic flag naming Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- cmd/gh-aw/main.go | 15 +++++---- pkg/actionpins/actionpins_internal_test.go | 3 +- .../stringsconcatloop/stringsconcatloop.go | 31 ++++++++++++------- 3 files changed, 30 insertions(+), 19 deletions(-) diff --git a/cmd/gh-aw/main.go b/cmd/gh-aw/main.go index cd004785dc9..ee8629cfcd6 100644 --- a/cmd/gh-aw/main.go +++ b/cmd/gh-aw/main.go @@ -732,12 +732,14 @@ func createCommandSet() commandSet { return cmds } -func configureNewAndCompileFlags() { +func configureNewCmdFlags() { newCmd.Flags().BoolP("force", "f", false, "Overwrite existing workflow files without confirmation") newCmd.Flags().BoolP("interactive", "i", false, "Launch interactive workflow creation wizard") newCmd.Flags().StringP("engine", "e", "", cli.EngineFlagOverrideUsage) cli.RegisterEngineFlagCompletion(newCmd) +} +func configureCompileBuildFlags() { compileCmd.Flags().StringP("engine", "e", "", cli.EngineFlagOverrideUsage) compileCmd.Flags().String("action-mode", "", "How gh-aw action scripts are referenced in compiled workflows: 'dev' uses local paths (for developing gh-aw itself), 'release' emits SHA-pinned remote refs from github/gh-aw, 'action' uses the github/gh-aw-actions repository. Auto-detected from the binary build type if not specified") compileCmd.Flags().String("action-tag", "", "Pin compiled workflows to a specific version of gh-aw actions. Accepts a full commit SHA or a version tag (e.g. v1, v1.2.3). Sets --action-mode to 'release' unless --action-mode action is also specified. Cannot be combined with --gh-aw-ref; use --gh-aw-ref when you want to resolve a branch or tag name to its current SHA") @@ -757,7 +759,7 @@ func configureNewAndCompileFlags() { _ = compileCmd.Flags().MarkHidden("use-samples") } -func configureCompileFlagsContinued() { +func configureCompileToolFlags() { compileCmd.Flags().Bool("dependabot", false, "Generate dependency manifests (package.json, requirements.txt, go.mod) and Dependabot config when dependencies are detected") compileCmd.Flags().BoolP("force", "f", false, "Force overwrite of existing dependency files (only applies when --dependabot is set; e.g., dependabot.yml)") compileCmd.Flags().Bool("refresh-stop-time", false, "Force regeneration of stop-after times instead of preserving existing values from lock files") @@ -785,7 +787,7 @@ func configureCompileFlagsContinued() { compileCmd.Flags().Bool("ghes", false, "Enable GitHub Enterprise Server (GHES) compatibility mode. Artifact actions continue using latest non-v3 pins (v3 is deprecated). Overrides the aw.json ghes field.") } -func finalizeCompileFlagSetup() { +func finalizeCompileFlags() { if err := compileCmd.Flags().MarkHidden("prior-manifest-file"); err != nil { _ = err } @@ -869,9 +871,10 @@ func init() { configureRootCommand() rootCmd.SetHelpCommand(newCustomHelpCmd()) cmds := createCommandSet() - configureNewAndCompileFlags() - configureCompileFlagsContinued() - finalizeCompileFlagSetup() + configureNewCmdFlags() + configureCompileBuildFlags() + configureCompileToolFlags() + finalizeCompileFlags() configureOtherCommandFlags() assignCommandGroups(cmds) addCommandsToRoot(cmds) diff --git a/pkg/actionpins/actionpins_internal_test.go b/pkg/actionpins/actionpins_internal_test.go index c3c1c1755fd..3bcc9f93ed0 100644 --- a/pkg/actionpins/actionpins_internal_test.go +++ b/pkg/actionpins/actionpins_internal_test.go @@ -173,8 +173,7 @@ func TestInitWarnings_InitializesAndPreservesMap(t *testing.T) { initWarnings(ctx) require.NotNil(t, ctx.Warnings, "Expected warnings map to remain initialized") - assert.Len(t, ctx.Warnings, 1, "Expected existing warnings to be preserved unchanged") - assert.True(t, ctx.Warnings["actions/checkout@v5"], "Expected existing warnings to be preserved unchanged") + assert.Equal(t, map[string]bool{"actions/checkout@v5": true}, ctx.Warnings, "Expected existing warnings to be preserved unchanged") }) } diff --git a/pkg/linters/stringsconcatloop/stringsconcatloop.go b/pkg/linters/stringsconcatloop/stringsconcatloop.go index 53896d2465f..5068ef3ecbe 100644 --- a/pkg/linters/stringsconcatloop/stringsconcatloop.go +++ b/pkg/linters/stringsconcatloop/stringsconcatloop.go @@ -29,6 +29,15 @@ var Analyzer = &analysis.Analyzer{ Run: run, } +// concatLoopMatch holds the components of a string-concatenation-in-loop +// assignment identified by collectConcatLoopAssignment. +type concatLoopMatch struct { + assign *ast.AssignStmt + lhsExpr ast.Expr + loopNode ast.Node + pos token.Position +} + func run(pass *analysis.Pass) (any, error) { pkgLog.Printf("analyzing package %s", pass.Pkg.Path()) root, err := astutil.Root(pass) @@ -45,15 +54,15 @@ func run(pass *analysis.Pass) (any, error) { } for cur := range root.Preorder((*ast.AssignStmt)(nil)) { - assign, lhsExpr, loopNode, pos, ok := collectConcatLoopAssignment(pass, cur, noLintIndex, generatedFiles) + m, ok := collectConcatLoopAssignment(pass, cur, noLintIndex, generatedFiles) if !ok { continue } - if !shouldReportLoopConcat(pass, loopNode, lhsExpr) { + if !shouldReportLoopConcat(pass, m.loopNode, m.lhsExpr) { continue } - pkgLog.Printf("flagging string concatenation in loop at %s", pos) - pass.ReportRangef(assign, "string concatenation inside a loop allocates O(n) strings and O(n²) total bytes; use strings.Builder instead") + pkgLog.Printf("flagging string concatenation in loop at %s", m.pos) + pass.ReportRangef(m.assign, "string concatenation inside a loop allocates O(n) strings and O(n²) total bytes; use strings.Builder instead") } return nil, nil @@ -64,27 +73,27 @@ func collectConcatLoopAssignment( cur inspector.Cursor, noLintIndex nolint.DirectiveIndex, generatedFiles filecheck.GeneratedIndex, -) (*ast.AssignStmt, ast.Expr, ast.Node, token.Position, bool) { +) (*concatLoopMatch, bool) { assign, ok := cur.Node().(*ast.AssignStmt) if !ok { - return nil, nil, nil, token.Position{}, false + return nil, false } lhsExpr, ok := concatAssignmentLHS(assign) if !ok { - return nil, nil, nil, token.Position{}, false + return nil, false } pos := pass.Fset.PositionFor(assign.Pos(), false) if filecheck.ShouldSkipFilename(pos.Filename, generatedFiles) { - return nil, nil, nil, token.Position{}, false + return nil, false } loopPos, loopNode, inLoop := enclosingLoop(pass, cur) if !inLoop { - return nil, nil, nil, token.Position{}, false + return nil, false } if nolint.HasDirectiveForLinter(pos, noLintIndex, "stringsconcatloop") || nolint.HasDirectiveForLinter(loopPos, noLintIndex, "stringsconcatloop") { - return nil, nil, nil, token.Position{}, false + return nil, false } - return assign, lhsExpr, loopNode, pos, true + return &concatLoopMatch{assign: assign, lhsExpr: lhsExpr, loopNode: loopNode, pos: pos}, true } func concatAssignmentLHS(assign *ast.AssignStmt) (ast.Expr, bool) {