diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4e07196a9..466ccace4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -84,7 +84,7 @@ jobs: run: make lint - name: Run golangci-lint - uses: golangci/golangci-lint-action@ec5d18412c0aeab7936cb16880d708ba2a64e1ae # v6.2.0 + uses: golangci/golangci-lint-action@ba0d7d2ec06a0ea1cb5fa41b2e4a3ab91d21278a # v9.3.0 with: version: v2.8.0 args: --timeout=5m diff --git a/.golangci.yml b/.golangci.yml index 7bf920493..a8d4d67a4 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -19,38 +19,34 @@ linters: - revive - testifylint - gosec - -linters-settings: - testifylint: - enable-all: true # Configuration for disabled linters is kept for reference if they are re-enabled - errcheck: - exclude-functions: - - (*os.File).Close - - (*os.File).Sync - - os.Chdir - - os.Chmod - - os.Chtimes - - os.MkdirAll - - os.Remove - - os.RemoveAll - - os.WriteFile - gocritic: - enable-all: true - disabled-checks: - - ifElseChain - - singleCaseSwitch - - appendAssign - - unlambda - - elseif - - assignOp - - argOrder - - dupBranchBody - - deprecatedComment - - commentFormatting - - badCall - -issues: - exclude-generated: lax - exclude-dirs-use-default: false - exclude-use-default: false + settings: + testifylint: + enable-all: true + errcheck: + exclude-functions: + - (*os.File).Close + - (*os.File).Sync + - os.Chdir + - os.Chmod + - os.Chtimes + - os.MkdirAll + - os.Remove + - os.RemoveAll + - os.WriteFile + gocritic: + enable-all: true + disabled-checks: + - ifElseChain + - singleCaseSwitch + - appendAssign + - unlambda + - elseif + - assignOp + - argOrder + - dupBranchBody + - deprecatedComment + - commentFormatting + - badCall + exclusions: + generated: lax diff --git a/internal/cmd/completion.go b/internal/cmd/completion.go index f177e11fe..2a795b4ac 100644 --- a/internal/cmd/completion.go +++ b/internal/cmd/completion.go @@ -2,7 +2,6 @@ package cmd import ( "fmt" - "os" "github.com/spf13/cobra" ) @@ -51,15 +50,16 @@ PowerShell: ValidArgs: []string{"bash", "zsh", "fish", "powershell"}, Args: cobra.MatchAll(cobra.ExactArgs(1), cobra.OnlyValidArgs), RunE: func(cmd *cobra.Command, args []string) error { + out := cmd.OutOrStdout() switch args[0] { case "bash": - return cmd.Root().GenBashCompletionV2(os.Stdout, true) + return cmd.Root().GenBashCompletionV2(out, true) case "zsh": - return cmd.Root().GenZshCompletion(os.Stdout) + return cmd.Root().GenZshCompletion(out) case "fish": - return cmd.Root().GenFishCompletion(os.Stdout, true) + return cmd.Root().GenFishCompletion(out, true) case "powershell": - return cmd.Root().GenPowerShellCompletionWithDesc(os.Stdout) + return cmd.Root().GenPowerShellCompletionWithDesc(out) default: // This default case should never be reached due to Args validation // above, but is included for defensive programming. diff --git a/internal/cmd/flags.go b/internal/cmd/flags.go index 7939d6a12..9149ef680 100644 --- a/internal/cmd/flags.go +++ b/internal/cmd/flags.go @@ -76,15 +76,12 @@ func registerFlagCompletions(cmd *cobra.Command) { if err := cmd.MarkFlagFilename("config", "toml"); err != nil { debugLog.Printf("Failed to register --config filename completion: %v", err) } - cmd.RegisterFlagCompletionFunc("log-dir", func(_ *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) { - return nil, cobra.ShellCompDirectiveFilterDirs - }) - cmd.RegisterFlagCompletionFunc("payload-dir", func(_ *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) { - return nil, cobra.ShellCompDirectiveFilterDirs - }) - cmd.RegisterFlagCompletionFunc("wasm-cache-dir", func(_ *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) { - return nil, cobra.ShellCompDirectiveFilterDirs - }) + // Use MarkFlagDirname for directory flags (cobra best practice) + for _, dirFlag := range []string{"log-dir", "payload-dir", "wasm-cache-dir"} { + if err := cmd.MarkFlagDirname(dirFlag); err != nil { + debugLog.Printf("Failed to register --%s dirname completion: %v", dirFlag, err) + } + } // Show all files for --env — the canonical .env file has no extension. if err := cmd.MarkFlagFilename("env"); err != nil { debugLog.Printf("Failed to register --env filename completion: %v", err) diff --git a/internal/cmd/flags_test.go b/internal/cmd/flags_test.go index a49bfe60b..301a2d657 100644 --- a/internal/cmd/flags_test.go +++ b/internal/cmd/flags_test.go @@ -117,31 +117,31 @@ func TestRegisterFlagCompletions(t *testing.T) { t.Run("log-dir flag completion returns directory filter", func(t *testing.T) { cmd := setupCmd(t) - compFunc, ok := cmd.GetFlagCompletionFunc("log-dir") - require.True(t, ok, "log-dir flag should have a completion function") - _, directive := compFunc(cmd, nil, "") - assert.Equal(t, cobra.ShellCompDirectiveFilterDirs, directive, - "log-dir flag should have directory completion directive") + flag := cmd.Flags().Lookup("log-dir") + require.NotNil(t, flag, "log-dir flag should be registered") + require.NotNil(t, flag.Annotations, "log-dir flag should have dirname annotations") + _, ok := flag.Annotations[cobra.BashCompSubdirsInDir] + assert.True(t, ok, "log-dir flag should have directory completion annotation via MarkFlagDirname") }) t.Run("payload-dir flag completion returns directory filter", func(t *testing.T) { cmd := setupCmd(t) - compFunc, ok := cmd.GetFlagCompletionFunc("payload-dir") - require.True(t, ok, "payload-dir flag should have a completion function") - _, directive := compFunc(cmd, nil, "") - assert.Equal(t, cobra.ShellCompDirectiveFilterDirs, directive, - "payload-dir flag should have directory completion directive") + flag := cmd.Flags().Lookup("payload-dir") + require.NotNil(t, flag, "payload-dir flag should be registered") + require.NotNil(t, flag.Annotations, "payload-dir flag should have dirname annotations") + _, ok := flag.Annotations[cobra.BashCompSubdirsInDir] + assert.True(t, ok, "payload-dir flag should have directory completion annotation via MarkFlagDirname") }) t.Run("wasm-cache-dir flag completion returns directory filter", func(t *testing.T) { cmd := setupCmd(t) - compFunc, ok := cmd.GetFlagCompletionFunc("wasm-cache-dir") - require.True(t, ok, "wasm-cache-dir flag should have a completion function") - _, directive := compFunc(cmd, nil, "") - assert.Equal(t, cobra.ShellCompDirectiveFilterDirs, directive, - "wasm-cache-dir flag should have directory completion directive") + flag := cmd.Flags().Lookup("wasm-cache-dir") + require.NotNil(t, flag, "wasm-cache-dir flag should be registered") + require.NotNil(t, flag.Annotations, "wasm-cache-dir flag should have dirname annotations") + _, ok := flag.Annotations[cobra.BashCompSubdirsInDir] + assert.True(t, ok, "wasm-cache-dir flag should have directory completion annotation via MarkFlagDirname") }) t.Run("env flag completion shows all files (no extension filter)", func(t *testing.T) { diff --git a/internal/cmd/output.go b/internal/cmd/output.go index a8d9dcf6e..f9d227eb2 100644 --- a/internal/cmd/output.go +++ b/internal/cmd/output.go @@ -8,12 +8,13 @@ import ( "os" "github.com/github/gh-aw-mcpg/internal/config" + "github.com/spf13/cobra" ) // writeGatewayConfigToStdout writes the rewritten gateway configuration to stdout // per MCP Gateway Specification Section 5.4 -func writeGatewayConfigToStdout(cfg *config.Config, listenAddr, mode string, tlsEnabled bool) error { - return writeGatewayConfig(cfg, listenAddr, mode, tlsEnabled, os.Stdout) +func writeGatewayConfigToStdout(cmd *cobra.Command, cfg *config.Config, listenAddr, mode string, tlsEnabled bool) error { + return writeGatewayConfig(cfg, listenAddr, mode, tlsEnabled, cmd.OutOrStdout()) } func writeGatewayConfig(cfg *config.Config, listenAddr, mode string, tlsEnabled bool, w io.Writer) error { diff --git a/internal/cmd/proxy.go b/internal/cmd/proxy.go index 0fb8ccd8d..1ab62fbcf 100644 --- a/internal/cmd/proxy.go +++ b/internal/cmd/proxy.go @@ -137,6 +137,13 @@ Local usage: cmd.RegisterFlagCompletionFunc("guards-mode", cobra.FixedCompletions( difc.ValidModes, cobra.ShellCompDirectiveNoFileComp)) + // Use MarkFlagDirname for directory flags (cobra best practice) + for _, dirFlag := range []string{"log-dir", "wasm-cache-dir", "tls-dir"} { + if err := cmd.MarkFlagDirname(dirFlag); err != nil { + logProxyCmd.Printf("Failed to register --%s dirname completion: %v", dirFlag, err) + } + } + return cmd } @@ -290,22 +297,23 @@ func runProxy(cmd *cobra.Command, args []string) error { logger.LogInfo("startup", "Proxy listening on %s://%s", scheme, actualAddr) // Print connection info - fmt.Fprintf(os.Stderr, "\nMCPG GitHub API Proxy\n") - fmt.Fprintf(os.Stderr, " Listening: %s://%s\n", scheme, actualAddr) - fmt.Fprintf(os.Stderr, " Upstream: %s\n", apiURL) - fmt.Fprintf(os.Stderr, " Mode: %s\n", proxyDIFCMode) - fmt.Fprintf(os.Stderr, " Guard: %s\n", proxyGuardWasm) + stderr := cmd.ErrOrStderr() + fmt.Fprintf(stderr, "\nMCPG GitHub API Proxy\n") + fmt.Fprintf(stderr, " Listening: %s://%s\n", scheme, actualAddr) + fmt.Fprintf(stderr, " Upstream: %s\n", apiURL) + fmt.Fprintf(stderr, " Mode: %s\n", proxyDIFCMode) + fmt.Fprintf(stderr, " Guard: %s\n", proxyGuardWasm) if tlsCfg != nil { - fmt.Fprintf(os.Stderr, " CA cert: %s\n", tlsCfg.CACertPath) - fmt.Fprintf(os.Stderr, "\nConnect with:\n") - fmt.Fprintf(os.Stderr, " export GH_HOST=%s\n", clientAddr(actualAddr)) - fmt.Fprintf(os.Stderr, " export NODE_EXTRA_CA_CERTS=%s\n", tlsCfg.CACertPath) - fmt.Fprintf(os.Stderr, " export SSL_CERT_FILE=%s\n", tlsCfg.CACertPath) - fmt.Fprintf(os.Stderr, " export GIT_SSL_CAINFO=%s\n", tlsCfg.CACertPath) - fmt.Fprintf(os.Stderr, " gh issue list -R org/repo\n\n") + fmt.Fprintf(stderr, " CA cert: %s\n", tlsCfg.CACertPath) + fmt.Fprintf(stderr, "\nConnect with:\n") + fmt.Fprintf(stderr, " export GH_HOST=%s\n", clientAddr(actualAddr)) + fmt.Fprintf(stderr, " export NODE_EXTRA_CA_CERTS=%s\n", tlsCfg.CACertPath) + fmt.Fprintf(stderr, " export SSL_CERT_FILE=%s\n", tlsCfg.CACertPath) + fmt.Fprintf(stderr, " export GIT_SSL_CAINFO=%s\n", tlsCfg.CACertPath) + fmt.Fprintf(stderr, " gh issue list -R org/repo\n\n") } else { - fmt.Fprintf(os.Stderr, "\nConnect with:\n") - fmt.Fprintf(os.Stderr, " curl http://%s/repos/org/repo/issues\n\n", actualAddr) + fmt.Fprintf(stderr, "\nConnect with:\n") + fmt.Fprintf(stderr, " curl http://%s/repos/org/repo/issues\n\n", actualAddr) } return httpServer.Serve(listener) diff --git a/internal/cmd/root.go b/internal/cmd/root.go index 7d299534e..a67fece7f 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -77,6 +77,11 @@ func init() { // Set custom error prefix for better branding rootCmd.SetErrPrefix("MCPG Error:") + // Provide user-friendly flag parse error messages that include the usage hint + rootCmd.SetFlagErrorFunc(func(cmd *cobra.Command, err error) error { + return fmt.Errorf("%w\nSee '%s --help' for usage", err, cmd.CommandPath()) + }) + // Set custom version template with enhanced formatting rootCmd.SetVersionTemplate(`MCPG Gateway {{.Version}} `) @@ -88,6 +93,10 @@ func init() { // Register all flags from feature modules (flags_*.go files) registerAllFlags(rootCmd) + // Preserve flag registration order in help output (cobra sorts alphabetically by default) + rootCmd.Flags().SortFlags = false + rootCmd.PersistentFlags().SortFlags = false + // Register custom flag completions registerFlagCompletions(rootCmd) @@ -453,7 +462,7 @@ func run(cmd *cobra.Command, args []string) error { } // Write gateway configuration to stdout per spec section 5.4 - if err := writeGatewayConfigToStdout(cfg, listenAddr, mode, tlsEnabled); err != nil { + if err := writeGatewayConfigToStdout(cmd, cfg, listenAddr, mode, tlsEnabled); err != nil { log.Printf("Warning: failed to write gateway configuration to stdout: %v", err) } @@ -479,7 +488,7 @@ func run(cmd *cobra.Command, args []string) error { // Execute runs the root command func Execute() { if err := rootCmd.Execute(); err != nil { - fmt.Fprintln(os.Stderr, err) + fmt.Fprintln(rootCmd.ErrOrStderr(), err) os.Exit(1) } }