From 69adf5e5ed2967366542d7bb302e4d93241cfcfc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 28 Jul 2026 06:08:24 +0000 Subject: [PATCH 1/2] Initial plan From 4cc92f0493957bb86886d1898d75bbd00cd9f08e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 28 Jul 2026 06:28:10 +0000 Subject: [PATCH 2/2] Refactor non-workflow large functions in parser, linters, and CLI Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- cmd/gh-aw/main.go | 859 ++++++++---------- pkg/linters/registry.go | 124 +-- .../stringbytesroundtrip.go | 103 +-- pkg/parser/remote_download_file.go | 100 +- 4 files changed, 576 insertions(+), 610 deletions(-) diff --git a/cmd/gh-aw/main.go b/cmd/gh-aw/main.go index 3683a5a0966..b1584ef3040 100644 --- a/cmd/gh-aw/main.go +++ b/cmd/gh-aw/main.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "io" "os" "os/signal" "sort" @@ -285,137 +286,174 @@ 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") - disableModelsDevLookup, _ := cmd.Flags().GetBool("no-models-dev-lookup") - 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 - } + RunE: runCompileCommand, +} - finishCompileUpdateCheck := cli.StartCompileUpdateCheck(cmd.Context(), noCheckUpdate, verbose) - defer finishCompileUpdateCheck() +func runCompileCommand(cmd *cobra.Command, args []string) error { + flags, err := readCompileFlags(cmd) + if err != nil { + return err + } + if err := validateEngine(flags.EngineOverride); err != nil { + return err + } - // 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 - } - } + finishCompileUpdateCheck := cli.StartCompileUpdateCheck(cmd.Context(), flags.NoCheckUpdate, flags.Verbose) + defer finishCompileUpdateCheck() - // 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, - DisableModelsDevLookup: disableModelsDevLookup, - 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 - } + if err := runCompileFixIfRequested(args, flags); err != nil { + return err + } + if _, err := cli.CompileWorkflows(cmd.Context(), buildCompileConfig(args, flags)); 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 +} + +func runCompileFixIfRequested(args []string, flags compileFlagValues) error { + if !flags.Fix { return nil - }, + } + return cli.RunFix(cli.FixConfig{ + WorkflowIDs: args, + Write: true, + Verbose: flags.Verbose, + WorkflowDir: flags.Dir, + }) +} + +type compileFlagValues struct { + EngineOverride, ActionMode, ActionTag, ActionsRepo, Dir, WorkflowsDir, LogicalRepo string + ScheduleSeed, PriorManifestFile string + Validate, Watch, NoEmit, Purge, Strict, Trial, Dependabot, ForceOverwrite bool + RefreshStopTime, ForceRefreshActionPins, AllowActionRefs bool + Zizmor, Poutine, Actionlint, RunnerGuard, Syft, Grype, Grant, Yamllint bool + JSONOutput, ShowAllErrors, Fix, Stats, FailFast, NoCheckUpdate bool + Staged, Approve, ValidateImages, DisableModelsDevLookup, GHESCompat, Verbose bool + UseSamples bool +} + +func readCompileFlags(cmd *cobra.Command) (compileFlagValues, error) { + flags := compileFlagValues{ + EngineOverride: flagString(cmd, "engine"), + ActionMode: flagString(cmd, "action-mode"), + ActionTag: flagString(cmd, "action-tag"), + ActionsRepo: flagString(cmd, "actions-repo"), + Validate: flagBool(cmd, "validate"), + Watch: flagBool(cmd, "watch"), + Dir: flagString(cmd, "dir"), + WorkflowsDir: flagString(cmd, "workflows-dir"), + NoEmit: flagBool(cmd, "no-emit"), + Purge: flagBool(cmd, "purge"), + Strict: flagBool(cmd, "strict"), + Trial: flagBool(cmd, "trial"), + LogicalRepo: flagString(cmd, "logical-repo"), + Dependabot: flagBool(cmd, "dependabot"), + ForceOverwrite: flagBool(cmd, "force"), + RefreshStopTime: flagBool(cmd, "refresh-stop-time"), + ForceRefreshActionPins: flagBool(cmd, "force-refresh-action-pins"), + AllowActionRefs: flagBool(cmd, "allow-action-refs"), + Zizmor: flagBool(cmd, "zizmor"), + Poutine: flagBool(cmd, "poutine"), + Actionlint: flagBool(cmd, "actionlint"), + RunnerGuard: flagBool(cmd, "runner-guard"), + Syft: flagBool(cmd, "syft"), + Grype: flagBool(cmd, "grype"), + Grant: flagBool(cmd, "grant"), + Yamllint: flagBool(cmd, "yamllint"), + JSONOutput: flagBool(cmd, "json"), + ShowAllErrors: flagBool(cmd, "show-all"), + Fix: flagBool(cmd, "fix"), + Stats: flagBool(cmd, "stats"), + FailFast: flagBool(cmd, "fail-fast"), + NoCheckUpdate: flagBool(cmd, "no-check-update"), + ScheduleSeed: flagString(cmd, "schedule-seed"), + Staged: flagBool(cmd, "staged"), + Approve: flagBool(cmd, "approve"), + ValidateImages: flagBool(cmd, "validate-images"), + DisableModelsDevLookup: flagBool(cmd, "no-models-dev-lookup"), + PriorManifestFile: flagString(cmd, "prior-manifest-file"), + GHESCompat: flagBool(cmd, "ghes"), + Verbose: flagBool(cmd, "verbose"), + UseSamples: flagBool(cmd, "use-samples"), + } + return resolveCompileGhAwRef(cmd, flags) +} + +func resolveCompileGhAwRef(cmd *cobra.Command, flags compileFlagValues) (compileFlagValues, error) { + ghAwRef := flagString(cmd, "gh-aw-ref") + if ghAwRef == "" { + return flags, nil + } + resolvedRef, err := workflow.ResolveGhAwRef(cmd.Context(), ghAwRef) + if err != nil { + return compileFlagValues{}, fmt.Errorf("--gh-aw-ref: %w", err) + } + flags.ActionMode = string(workflow.ActionModeRelease) + flags.ActionTag = resolvedRef + return flags, nil +} + +func buildCompileConfig(args []string, flags compileFlagValues) cli.CompileConfig { + workflowDir := flags.Dir + if flags.WorkflowsDir != "" { + workflowDir = flags.WorkflowsDir + } + return cli.CompileConfig{ + MarkdownFiles: args, + Verbose: flags.Verbose, + EngineOverride: flags.EngineOverride, + ActionMode: flags.ActionMode, + ActionTag: flags.ActionTag, + ActionsRepo: flags.ActionsRepo, + Validate: flags.Validate, + Watch: flags.Watch, + WorkflowDir: workflowDir, + SkipInstructions: false, // Deprecated field, kept for backward compatibility + NoEmit: flags.NoEmit, + Purge: flags.Purge, + TrialMode: flags.Trial, + TrialLogicalRepoSlug: flags.LogicalRepo, + Strict: flags.Strict, + Dependabot: flags.Dependabot, + ForceOverwrite: flags.ForceOverwrite, + RefreshStopTime: flags.RefreshStopTime, + ForceRefreshActionPins: flags.ForceRefreshActionPins, + AllowActionRefs: flags.AllowActionRefs, + Zizmor: flags.Zizmor, + Poutine: flags.Poutine, + Actionlint: flags.Actionlint, + RunnerGuard: flags.RunnerGuard, + Syft: flags.Syft, + Grype: flags.Grype, + Grant: flags.Grant, + Yamllint: flags.Yamllint, + JSONOutput: flags.JSONOutput, + ShowAllErrors: flags.ShowAllErrors, + Stats: flags.Stats, + FailFast: flags.FailFast, + ScheduleSeed: flags.ScheduleSeed, + Staged: flags.Staged, + Approve: flags.Approve, + ValidateImages: flags.ValidateImages, + DisableModelsDevLookup: flags.DisableModelsDevLookup, + PriorManifestFile: flags.PriorManifestFile, + GHESCompat: flags.GHESCompat, + UseSamples: flags.UseSamples, + } +} + +func flagString(cmd *cobra.Command, name string) string { + value, _ := cmd.Flags().GetString(name) + return value +} + +func flagBool(cmd *cobra.Command, name string) bool { + value, _ := cmd.Flags().GetBool(name) + return value } var runCmd = &cobra.Command{ @@ -514,52 +552,29 @@ 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:", - }) + configureRootCommand() + configureRootUsage() + rootCmd.SetHelpCommand(newCustomHelpCommand()) + + commands := createCommandBundle() + configureCommandFlags() + assignCommandGroups(commands) + addCommandsToRoot(commands) + normalizeSubcommandHelpFlags() +} - // Add global verbose flag to root command +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") - - // Add global banner flag to root command rootCmd.PersistentFlags().BoolVar(&bannerFlag, "banner", false, "Display ASCII logo banner with purple GitHub color theme") - - // Set output to stderr for consistency with CLI logging guidelines rootCmd.SetOut(os.Stderr) - - // 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 - - // 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 - - // Set version template to match the version subcommand format rootCmd.SetVersionTemplate(string(constants.CLIExtensionPrefix) + " version {{.Version}}\n") - - // 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) @@ -568,165 +583,216 @@ func init() { if f := rootCmd.Flags().Lookup("version"); f != nil { f.Usage = "Print the current version" } +} - // 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())) - } - 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) - } - } - } - } - } - 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")) +func configureRootUsage() { + rootCmd.SetUsageFunc(printUsageWithGhAwPath) +} + +func printUsageWithGhAwPath(cmd *cobra.Command) error { + out := cmd.OutOrStderr() + fmt.Fprint(out, "Usage:") + if cmd.Runnable() { + fmt.Fprintf(out, "\n %s", fixUsagePath(cmd.UseLine())) + } + if cmd.HasAvailableSubCommands() { + fmt.Fprintf(out, "\n %s [command]", fixUsagePath(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) + } + printUsageCommandList(out, 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", fixUsagePath(cmd.CommandPath())) + } else { + fmt.Fprintln(out) + } + return nil +} + +func printUsageCommandList(out io.Writer, cmd *cobra.Command) { + if !cmd.HasAvailableSubCommands() { + return + } + cmds := cmd.Commands() + colFmt := fmt.Sprintf("\n %%-%ds %%s", usageCommandColumnWidth(cmds)) + if len(cmd.Groups()) == 0 { + fmt.Fprint(out, "\n\nAvailable Commands:") + printUsageCommandsForGroup(out, cmds, colFmt, "") + return + } + for _, group := range cmd.Groups() { + fmt.Fprintf(out, "\n\n%s", group.Title) + printUsageCommandsForGroup(out, cmds, colFmt, group.ID) + } + if !cmd.AllChildCommandsHaveGroup() { + fmt.Fprint(out, "\n\nAdditional Commands:") + printUsageCommandsForGroup(out, cmds, colFmt, "") + } +} + +func usageCommandColumnWidth(cmds []*cobra.Command) int { + colWidth := 0 + for _, sub := range cmds { + if (sub.IsAvailableCommand() || sub.Name() == "help") && len(sub.Name()) > colWidth { + colWidth = len(sub.Name()) } - 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) + } + return colWidth +} + +func printUsageCommandsForGroup(out io.Writer, cmds []*cobra.Command, colFmt, groupID string) { + for _, sub := range cmds { + if sub.GroupID != groupID || (!sub.IsAvailableCommand() && sub.Name() != "help") { + continue } - return nil - }) + fmt.Fprintf(out, colFmt, sub.Name(), sub.Short) + } +} - // Create custom help command that supports "all" subcommand - customHelpCmd := &cobra.Command{ +func fixUsagePath(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 +} + +func newCustomHelpCommand() *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: runCustomHelpCommand, } +} - // 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) +func runCustomHelpCommand(_ *cobra.Command, args []string) error { + if len(args) == 1 && args[0] == "all" { + printAllCommandsHelp() + return nil + } + cmd, _, err := rootCmd.Find(args) + if cmd == nil || err != nil { + return fmt.Errorf("unknown help topic [%#q]", args) + } + cmd.InitDefaultHelpFlag() // make possible 'help' flag to be shown + return cmd.Help() +} - // Create and setup deploy command - deployCmd := cli.NewDeployCommand(validateEngine) +func printAllCommandsHelp() { + 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/")) +} - // Create and setup trial command - trialCmd := cli.NewTrialCommand(validateEngine) +type commandBundle struct { + addCmd, addWizardCmd, updateCmd, deployCmd, trialCmd, initCmd *cobra.Command + statusCmd, listCmd, mcpCmd, logsCmd, auditCmd, viewCmd *cobra.Command + healthCmd, outcomesCmd, mcpServerCmd, prCmd, secretsCmd, fixCmd *cobra.Command + upgradeCmd, completionCmd, hashCmd, projectCmd, doctorCmd *cobra.Command + checksCmd, validateCmd, lintCmd, domainsCmd, experimentsCmd *cobra.Command + forecastCmd, envCmd *cobra.Command +} - // Create and setup init command +func createCommandBundle() commandBundle { initCmd := cli.NewInitCommand() cli.RegisterEngineFlagCompletion(initCmd) + return commandBundle{ + addCmd: cli.NewAddCommand(validateEngine), + addWizardCmd: cli.NewAddWizardCommand(validateEngine), + updateCmd: cli.NewUpdateCommand(validateEngine), + deployCmd: cli.NewDeployCommand(validateEngine), + trialCmd: cli.NewTrialCommand(validateEngine), + initCmd: initCmd, + 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(), + } +} - // Add flags to new command +func configureCommandFlags() { + configureNewCommandFlags() + configureCompileCommandFlags() + configureRemoveAndToggleFlags() + configureRunCommandFlags() + cli.RegisterEngineFlagCompletion(newCmd) + cli.RegisterEngineFlagCompletion(runCmd) + compileCmd.ValidArgsFunction = cli.CompleteWorkflowNames + removeCmd.ValidArgsFunction = cli.CompleteWorkflowNames + enableCmd.ValidArgsFunction = cli.CompleteWorkflowNames + disableCmd.ValidArgsFunction = cli.CompleteWorkflowNames + runCmd.ValidArgsFunction = cli.CompleteWorkflowNames + cli.RegisterEngineFlagCompletion(compileCmd) + cli.RegisterDirFlagCompletion(compileCmd, "dir") + cli.RegisterDirFlagCompletion(removeCmd, "dir") +} + +func configureNewCommandFlags() { 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 +func configureCompileCommandFlags() { + configureCompileCoreFlags() + configureCompileScannerFlags() + configureCompileAdvancedFlags() + if err := compileCmd.Flags().MarkHidden("prior-manifest-file"); err != nil { + _ = err + } + compileCmd.MarkFlagsMutuallyExclusive("dir", "workflows-dir") + compileCmd.MarkFlagsMutuallyExclusive("gh-aw-ref", "action-tag") + compileCmd.MarkFlagsMutuallyExclusive("gh-aw-ref", "action-mode") +} + +func configureCompileCoreFlags() { 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") @@ -749,6 +815,9 @@ Use "` + string(constants.CLIExtensionPrefix) + ` help all" to show help for all compileCmd.Flags().Bool("refresh-stop-time", false, "Force regeneration of stop-after times instead of preserving existing values from lock files") compileCmd.Flags().Bool("force-refresh-action-pins", false, "Force refresh of action pins by clearing the cache and resolving all action SHAs from GitHub API") compileCmd.Flags().Bool("allow-action-refs", false, "Allow unresolved action refs and emit warnings instead of failing validation") +} + +func configureCompileScannerFlags() { compileCmd.Flags().Bool("zizmor", false, "Run zizmor security scanner on generated .lock.yml files") compileCmd.Flags().Bool("poutine", false, "Run poutine security scanner on generated .lock.yml files") compileCmd.Flags().Bool("actionlint", false, "Run actionlint linter on generated .lock.yml files") @@ -757,6 +826,9 @@ Use "` + string(constants.CLIExtensionPrefix) + ` help all" to show help for all compileCmd.Flags().Bool("grype", false, "Run grype vulnerability scanner on container images referenced in compiled .lock.yml files (uses Docker image "+cli.GrypeImage+")") compileCmd.Flags().Bool("grant", false, "Run grant license scanner on container images referenced in compiled .lock.yml files (uses Docker image "+cli.GrantImage+")") compileCmd.Flags().Bool("yamllint", false, "Run yamllint YAML linter on generated .lock.yml files (uses Docker image "+cli.YamllintImage+")") +} + +func configureCompileAdvancedFlags() { compileCmd.Flags().Bool("fix", false, "Apply automatic codemod fixes to workflows before compiling") compileCmd.Flags().BoolP("json", "j", false, "Output results in JSON format") compileCmd.Flags().Bool("show-all", false, "Display all compilation errors instead of only the highest-priority subset (default: top 5)") @@ -770,40 +842,18 @@ Use "` + string(constants.CLIExtensionPrefix) + ` help all" to show help for all compileCmd.Flags().Bool("no-models-dev-lookup", false, "Disable compile-time models.dev pricing lookup for models missing from the embedded catalog") 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") - 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 configureRemoveAndToggleFlags() { 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 +func configureRunCommandFlags() { 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) @@ -816,139 +866,36 @@ 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) +} + +func assignCommandGroups(commands commandBundle) { + commands.initCmd.GroupID, newCmd.GroupID, commands.addCmd.GroupID, commands.addWizardCmd.GroupID = "setup", "setup", "setup", "setup" + removeCmd.GroupID, commands.updateCmd.GroupID, commands.deployCmd.GroupID, commands.upgradeCmd.GroupID = "setup", "setup", "setup", "setup" + commands.secretsCmd.GroupID, commands.envCmd.GroupID, commands.doctorCmd.GroupID = "setup", "setup", "setup" + compileCmd.GroupID, commands.validateCmd.GroupID, commands.lintCmd.GroupID = "development", "development", "development" + commands.mcpCmd.GroupID, commands.fixCmd.GroupID, commands.domainsCmd.GroupID = "development", "development", "development" + runCmd.GroupID, enableCmd.GroupID, disableCmd.GroupID, commands.trialCmd.GroupID = "execution", "execution", "execution", "execution" + commands.logsCmd.GroupID, commands.auditCmd.GroupID, commands.viewCmd.GroupID = "analysis", "analysis", "analysis" + commands.healthCmd.GroupID, commands.outcomesCmd.GroupID, commands.checksCmd.GroupID = "analysis", "analysis", "analysis" + commands.statusCmd.GroupID, commands.listCmd.GroupID, commands.experimentsCmd.GroupID, commands.forecastCmd.GroupID = "analysis", "analysis", "analysis", "analysis" + commands.mcpServerCmd.GroupID, commands.prCmd.GroupID, commands.completionCmd.GroupID = "utilities", "utilities", "utilities" + commands.hashCmd.GroupID, commands.projectCmd.GroupID = "utilities", "utilities" +} + +func addCommandsToRoot(commands commandBundle) { + rootCmd.AddCommand(commands.addCmd, commands.addWizardCmd, commands.updateCmd, commands.deployCmd, commands.upgradeCmd, commands.trialCmd, newCmd, commands.initCmd) + rootCmd.AddCommand(runCmd, removeCmd, commands.statusCmd, commands.listCmd, enableCmd, disableCmd, commands.logsCmd, commands.auditCmd) + rootCmd.AddCommand(commands.viewCmd, commands.healthCmd, commands.outcomesCmd, commands.checksCmd, commands.mcpCmd, commands.mcpServerCmd, commands.prCmd, versionCmd) + rootCmd.AddCommand(commands.secretsCmd, commands.fixCmd, compileCmd, commands.validateCmd, commands.lintCmd, commands.completionCmd, commands.hashCmd, commands.projectCmd) + rootCmd.AddCommand(commands.doctorCmd, commands.domainsCmd, commands.experimentsCmd, commands.forecastCmd, commands.envCmd) +} - // 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]"). +func normalizeSubcommandHelpFlags() { var fixSubCmdHelpFlags func(cmd *cobra.Command) fixSubCmdHelpFlags = 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:] } diff --git a/pkg/linters/registry.go b/pkg/linters/registry.go index b81f1bd1ebf..273162c4086 100644 --- a/pkg/linters/registry.go +++ b/pkg/linters/registry.go @@ -70,65 +70,67 @@ import ( // analyzers. Use it to drive multichecker.Main, test assertions, and // doc-completeness checks so that every consumer stays in sync automatically. func All() []*analysis.Analyzer { - return []*analysis.Analyzer{ - appendbytestring.Analyzer, - appendoneelement.Analyzer, - bytesbufferstring.Analyzer, - bytescomparestring.Analyzer, - contextcancelnotdeferred.Analyzer, - ctxbackground.Analyzer, - deferinloop.Analyzer, - errormessage.Analyzer, - errortypeassertion.Analyzer, - fprintlnsprintf.Analyzer, - errstringmatch.Analyzer, - errorfwrapv.Analyzer, - execcommandwithoutcontext.Analyzer, - excessivefuncparams.Analyzer, - fileclosenotdeferred.Analyzer, - fmterrorfnoverbs.Analyzer, - hardcodedfilepath.Analyzer, - httpnoctx.Analyzer, - httprespbodyclose.Analyzer, - ioutildeprecated.Analyzer, - httpstatuscode.Analyzer, - largefunc.Analyzer, - logfatallibrary.Analyzer, - manualmutexunlock.Analyzer, - mapclearloop.Analyzer, - mapdeletecheck.Analyzer, - nilctxpassed.Analyzer, - osexitinlibrary.Analyzer, - osgetenvlibrary.Analyzer, - ossetenvlibrary.Analyzer, - panicinlibrarycode.Analyzer, - rawloginlib.Analyzer, - regexpcompileinfunction.Analyzer, - ssljson.Analyzer, - seenmapbool.Analyzer, - sortslice.Analyzer, - sprintferrdot.Analyzer, - sprintferrorsnew.Analyzer, - sprintfbool.Analyzer, - sprintfint.Analyzer, - strconvparseignorederror.Analyzer, - stringbytesroundtrip.Analyzer, - stringreplaceminusone.Analyzer, - stringsconcatloop.Analyzer, - stringsindexcontains.Analyzer, - stringsindexhasprefix.Analyzer, - stringsjoinone.Analyzer, - stringscountcontains.Analyzer, - jsonmarshalignoredeerror.Analyzer, - lenstringzero.Analyzer, - lenstringsplit.Analyzer, - timeafterleak.Analyzer, - timesleepnocontext.Analyzer, - timenowsub.Analyzer, - tolowerequalfold.Analyzer, - trimleftright.Analyzer, - uncheckedtypeassertion.Analyzer, - wgdonenotdeferred.Analyzer, - writebytestring.Analyzer, - } + return append([]*analysis.Analyzer(nil), allAnalyzers...) +} + +var allAnalyzers = []*analysis.Analyzer{ + appendbytestring.Analyzer, + appendoneelement.Analyzer, + bytesbufferstring.Analyzer, + bytescomparestring.Analyzer, + contextcancelnotdeferred.Analyzer, + ctxbackground.Analyzer, + deferinloop.Analyzer, + errormessage.Analyzer, + errortypeassertion.Analyzer, + fprintlnsprintf.Analyzer, + errstringmatch.Analyzer, + errorfwrapv.Analyzer, + execcommandwithoutcontext.Analyzer, + excessivefuncparams.Analyzer, + fileclosenotdeferred.Analyzer, + fmterrorfnoverbs.Analyzer, + hardcodedfilepath.Analyzer, + httpnoctx.Analyzer, + httprespbodyclose.Analyzer, + ioutildeprecated.Analyzer, + httpstatuscode.Analyzer, + largefunc.Analyzer, + logfatallibrary.Analyzer, + manualmutexunlock.Analyzer, + mapclearloop.Analyzer, + mapdeletecheck.Analyzer, + nilctxpassed.Analyzer, + osexitinlibrary.Analyzer, + osgetenvlibrary.Analyzer, + ossetenvlibrary.Analyzer, + panicinlibrarycode.Analyzer, + rawloginlib.Analyzer, + regexpcompileinfunction.Analyzer, + ssljson.Analyzer, + seenmapbool.Analyzer, + sortslice.Analyzer, + sprintferrdot.Analyzer, + sprintferrorsnew.Analyzer, + sprintfbool.Analyzer, + sprintfint.Analyzer, + strconvparseignorederror.Analyzer, + stringbytesroundtrip.Analyzer, + stringreplaceminusone.Analyzer, + stringsconcatloop.Analyzer, + stringsindexcontains.Analyzer, + stringsindexhasprefix.Analyzer, + stringsjoinone.Analyzer, + stringscountcontains.Analyzer, + jsonmarshalignoredeerror.Analyzer, + lenstringzero.Analyzer, + lenstringsplit.Analyzer, + timeafterleak.Analyzer, + timesleepnocontext.Analyzer, + timenowsub.Analyzer, + tolowerequalfold.Analyzer, + trimleftright.Analyzer, + uncheckedtypeassertion.Analyzer, + wgdonenotdeferred.Analyzer, + writebytestring.Analyzer, } diff --git a/pkg/linters/stringbytesroundtrip/stringbytesroundtrip.go b/pkg/linters/stringbytesroundtrip/stringbytesroundtrip.go index a19341e6533..0a57b39e24c 100644 --- a/pkg/linters/stringbytesroundtrip/stringbytesroundtrip.go +++ b/pkg/linters/stringbytesroundtrip/stringbytesroundtrip.go @@ -53,82 +53,79 @@ func run(pass *analysis.Pass) (any, error) { // string/[]byte round-trip (string([]byte(s))) or a wasteful two-copy clone // ([]byte(string(b))) and reports a diagnostic if so. func analyzeRoundTrip(pass *analysis.Pass, n ast.Node, generatedFiles filecheck.GeneratedIndex, noLintIndex nolint.DirectiveIndex) { - outer, ok := n.(*ast.CallExpr) + outer, inner, ok := roundTripCalls(n) + if !ok || shouldSkipRoundTrip(pass, outer, generatedFiles, noLintIndex) { + return + } + outerUnderlying, innerUnderlying, innerArgUnderlying, ok := roundTripUnderlyingTypes(pass, outer, inner) if !ok { return } - // Must be a type conversion (single argument, no ellipsis). - if len(outer.Args) != 1 || outer.Ellipsis.IsValid() { + if reportRedundantRoundTrip(pass, outer, inner, outerUnderlying, innerUnderlying, innerArgUnderlying) { return } + reportWastefulCloneRoundTrip(pass, outer, inner, outerUnderlying, innerUnderlying, innerArgUnderlying) +} + +func roundTripCalls(n ast.Node) (*ast.CallExpr, *ast.CallExpr, bool) { + outer, ok := n.(*ast.CallExpr) + if !ok || len(outer.Args) != 1 || outer.Ellipsis.IsValid() { + return nil, nil, false + } + inner, ok := outer.Args[0].(*ast.CallExpr) + if !ok || len(inner.Args) != 1 || inner.Ellipsis.IsValid() { + return nil, nil, false + } + return outer, inner, true +} +func shouldSkipRoundTrip(pass *analysis.Pass, outer *ast.CallExpr, generatedFiles filecheck.GeneratedIndex, noLintIndex nolint.DirectiveIndex) bool { pos := pass.Fset.PositionFor(outer.Pos(), false) if filecheck.ShouldSkipFilename(pos.Filename, generatedFiles) { - return - } - if nolint.HasDirectiveForLinter(pos, noLintIndex, "stringbytesroundtrip") { - return + return true } + return nolint.HasDirectiveForLinter(pos, noLintIndex, "stringbytesroundtrip") +} - // Must be a type conversion, not a function call. +func roundTripUnderlyingTypes(pass *analysis.Pass, outer, inner *ast.CallExpr) (types.Type, types.Type, types.Type, bool) { outerFunInfo, ok := pass.TypesInfo.Types[outer.Fun] if !ok || !outerFunInfo.IsType() { - return + return nil, nil, nil, false } - - outerType := pass.TypesInfo.TypeOf(outer) - if outerType == nil { - return - } - - inner, ok := outer.Args[0].(*ast.CallExpr) - if !ok { - return - } - if len(inner.Args) != 1 || inner.Ellipsis.IsValid() { - return - } - - // The inner call must also be a type conversion, not a function call. innerFunInfo, ok := pass.TypesInfo.Types[inner.Fun] if !ok || !innerFunInfo.IsType() { - return + return nil, nil, nil, false } - + outerType := pass.TypesInfo.TypeOf(outer) innerType := pass.TypesInfo.TypeOf(inner) - if innerType == nil { - return - } innerArgType := pass.TypesInfo.TypeOf(inner.Args[0]) - if innerArgType == nil { - return + if outerType == nil || innerType == nil || innerArgType == nil { + return nil, nil, nil, false } + return outerType.Underlying(), innerType.Underlying(), innerArgType.Underlying(), true +} - outerUnderlying := outerType.Underlying() - innerUnderlying := innerType.Underlying() - innerArgUnderlying := innerArgType.Underlying() - - // Check string([]byte(s)) where s is already a string. - if isStringType(outerUnderlying) && isByteSliceType(innerUnderlying) && isStringType(innerArgUnderlying) { - argText := astutil.NodeText(pass.Fset, inner.Args[0]) - pass.ReportRangef(outer, - "string([]byte(%s)) is a redundant round-trip; the inner []byte conversion copies the string unnecessarily", - argText, - ) - return +func reportRedundantRoundTrip(pass *analysis.Pass, outer, inner *ast.CallExpr, outerUnderlying, innerUnderlying, innerArgUnderlying types.Type) bool { + if !isStringType(outerUnderlying) || !isByteSliceType(innerUnderlying) || !isStringType(innerArgUnderlying) { + return false } + argText := astutil.NodeText(pass.Fset, inner.Args[0]) + pass.ReportRangef(outer, + "string([]byte(%s)) is a redundant round-trip; the inner []byte conversion copies the string unnecessarily", + argText, + ) + return true +} - // Check []byte(string(b)) where b is already a []byte. - // This is the defensive-copy idiom: the result is a non-aliasing copy, not - // a no-op. The diagnostic is therefore not "redundant" but "wasteful": - // two memory copies are made when one would suffice. - if isByteSliceType(outerUnderlying) && isStringType(innerUnderlying) && isByteSliceType(innerArgUnderlying) { - argText := astutil.NodeText(pass.Fset, inner.Args[0]) - pass.ReportRangef(outer, - "[]byte(string(%s)) makes two copies to clone %s; use slices.Clone(%s) or bytes.Clone(%s) for a single-copy independent slice", - argText, argText, argText, argText, - ) +func reportWastefulCloneRoundTrip(pass *analysis.Pass, outer, inner *ast.CallExpr, outerUnderlying, innerUnderlying, innerArgUnderlying types.Type) { + if !isByteSliceType(outerUnderlying) || !isStringType(innerUnderlying) || !isByteSliceType(innerArgUnderlying) { + return } + argText := astutil.NodeText(pass.Fset, inner.Args[0]) + pass.ReportRangef(outer, + "[]byte(string(%s)) makes two copies to clone %s; use slices.Clone(%s) or bytes.Clone(%s) for a single-copy independent slice", + argText, argText, argText, argText, + ) } func isStringType(t types.Type) bool { diff --git a/pkg/parser/remote_download_file.go b/pkg/parser/remote_download_file.go index 381f2dd4b80..93cec217407 100644 --- a/pkg/parser/remote_download_file.go +++ b/pkg/parser/remote_download_file.go @@ -383,61 +383,83 @@ func downloadFileViaRawURL(ctx context.Context, owner, repo, filePath, ref strin func downloadFileViaGitClone(ctx context.Context, owner, repo, path, ref, host string) ([]byte, error) { remoteLog.Printf("Attempting git clone fallback for %s/%s/%s@%s", owner, repo, path, ref) + if err := validateGitCloneFallbackInputs(ref, path); err != nil { + return nil, err + } + tmpDir, err := createGitCloneTempDir() + if err != nil { + return nil, err + } + defer os.RemoveAll(tmpDir) + + repoURL := getRepoGitURL(owner, repo, host) + if len(ref) == 40 && gitutil.IsHexString(ref) { + if err := cloneAndCheckoutSHA(ctx, repoURL, tmpDir, ref); err != nil { + return nil, err + } + } else if err := cloneBranchOrTag(ctx, repoURL, tmpDir, ref); err != nil { + return nil, err + } + + content, err := readFileFromClone(tmpDir, path) + if err != nil { + return nil, err + } + + remoteLog.Printf("Successfully downloaded file via git clone: %s/%s/%s@%s", owner, repo, path, ref) + return content, nil +} + +func validateGitCloneFallbackInputs(ref, path string) error { if err := gitutil.ValidateGitRef(ref); err != nil { - return nil, fmt.Errorf("refusing git clone fallback: %w", err) + return fmt.Errorf("refusing git clone fallback: %w", err) } if err := gitutil.ValidateGitPath(path); err != nil { - return nil, fmt.Errorf("refusing git clone fallback: %w", err) + return fmt.Errorf("refusing git clone fallback: %w", err) } + return nil +} - // Create a temporary directory for the shallow clone +func createGitCloneTempDir() (string, error) { tmpDir, err := os.MkdirTemp("", "gh-aw-git-clone-*") if err != nil { - return nil, fmt.Errorf("failed to create temp directory: %w", err) + return "", fmt.Errorf("failed to create temp directory: %w", err) } - defer os.RemoveAll(tmpDir) + return tmpDir, nil +} - var githubHost string +func getRepoGitURL(owner, repo, host string) string { + githubHost := GetGitHubHostForRepo(owner, repo) if host != "" { githubHost = "https://" + host - } else { - githubHost = GetGitHubHostForRepo(owner, repo) } - repoURL := fmt.Sprintf("%s/%s/%s.git", githubHost, owner, repo) + return fmt.Sprintf("%s/%s/%s.git", githubHost, owner, repo) +} - // Check if ref is a SHA (40 hex characters) - isSHA := len(ref) == 40 && gitutil.IsHexString(ref) - - var cloneCmd *exec.Cmd - if isSHA { - // For SHA refs, we need to clone without --branch and then checkout the specific commit - // Clone with minimal depth and no branch specified - cloneCmd = exec.CommandContext(ctx, "git", "clone", "--depth", "1", "--no-single-branch", repoURL, tmpDir) - if output, err := cloneCmd.CombinedOutput(); err != nil { - // Try without --no-single-branch if the first attempt fails - remoteLog.Printf("Clone with --no-single-branch failed, trying full clone: %s", string(output)) - cloneCmd = exec.CommandContext(ctx, "git", "clone", repoURL, tmpDir) - if output, err := cloneCmd.CombinedOutput(); err != nil { - return nil, fmt.Errorf("failed to clone repository: %w\nOutput: %s", err, string(output)) - } +func cloneAndCheckoutSHA(ctx context.Context, repoURL, tmpDir, ref string) error { + if output, err := exec.CommandContext(ctx, "git", "clone", "--depth", "1", "--no-single-branch", repoURL, tmpDir).CombinedOutput(); err != nil { + remoteLog.Printf("Clone with --no-single-branch failed, trying full clone: %s", string(output)) + if output, err := exec.CommandContext(ctx, "git", "clone", repoURL, tmpDir).CombinedOutput(); err != nil { + return fmt.Errorf("failed to clone repository: %w\nOutput: %s", err, string(output)) } + } + // ValidateGitRef above guarantees ref does not start with '-', so it remains a revision argument. + if output, err := exec.CommandContext(ctx, "git", "-C", tmpDir, "checkout", "--detach", ref).CombinedOutput(); err != nil { + return fmt.Errorf("failed to checkout commit %s: %w\nOutput: %s", ref, err, string(output)) + } + return nil +} - // Now checkout the specific commit in detached HEAD mode. ValidateGitRef above - // guarantees ref does not start with '-', so it remains a revision argument. - checkoutCmd := exec.CommandContext(ctx, "git", "-C", tmpDir, "checkout", "--detach", ref) - if output, err := checkoutCmd.CombinedOutput(); err != nil { - return nil, fmt.Errorf("failed to checkout commit %s: %w\nOutput: %s", ref, err, string(output)) - } - } else { - // For branch/tag refs, use --branch flag; the value is passed via a separate - // argument slot and cannot be confused with a flag because it follows --branch. - cloneCmd = exec.CommandContext(ctx, "git", "clone", "--depth", "1", "--branch", ref, repoURL, tmpDir) - if output, err := cloneCmd.CombinedOutput(); err != nil { - return nil, fmt.Errorf("failed to clone repository: %w\nOutput: %s", err, string(output)) - } +func cloneBranchOrTag(ctx context.Context, repoURL, tmpDir, ref string) error { + // For branch/tag refs, use --branch flag; the value is passed via a separate + // argument slot and cannot be confused with a flag because it follows --branch. + if output, err := exec.CommandContext(ctx, "git", "clone", "--depth", "1", "--branch", ref, repoURL, tmpDir).CombinedOutput(); err != nil { + return fmt.Errorf("failed to clone repository: %w\nOutput: %s", err, string(output)) } + return nil +} - // Read the file from the cloned repository +func readFileFromClone(tmpDir, path string) ([]byte, error) { filePath := filepath.Join(tmpDir, path) if err := fileutil.ValidatePathWithinBase(tmpDir, filePath); err != nil { return nil, fmt.Errorf("refusing to read file outside clone directory: %w", err) @@ -446,7 +468,5 @@ func downloadFileViaGitClone(ctx context.Context, owner, repo, path, ref, host s if err != nil { return nil, fmt.Errorf("failed to read file from cloned repository: %w", err) } - - remoteLog.Printf("Successfully downloaded file via git clone: %s/%s/%s@%s", owner, repo, path, ref) return content, nil }