diff --git a/internal/cmd/completion.go b/internal/cmd/completion.go index cfe3e04a..e1506f60 100644 --- a/internal/cmd/completion.go +++ b/internal/cmd/completion.go @@ -9,7 +9,7 @@ import ( // newCompletionCmd creates a completion command for generating shell completion scripts func newCompletionCmd() *cobra.Command { - return &cobra.Command{ + cmd := &cobra.Command{ Use: "completion [bash|zsh|fish|powershell]", Short: "Generate completion script", Long: `To load completions: @@ -66,4 +66,11 @@ PowerShell: } }, } + + // Override the parent's PersistentPreRunE to skip validation for completion command + cmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { + return nil + } + + return cmd } diff --git a/internal/cmd/root.go b/internal/cmd/root.go index f03a8f70..d69de584 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -46,6 +46,7 @@ var ( enableDIFC bool logDir string validateEnv bool + verbosity int // Verbosity level: 0 (default), 1 (-v info), 2 (-vv debug), 3 (-vvv trace) debugLog = logger.New("cmd:root") version = "dev" // Default version, overridden by SetVersion ) @@ -56,11 +57,15 @@ var rootCmd = &cobra.Command{ Version: version, Long: `MCPG is a proxy server for Model Context Protocol (MCP) servers. It provides routing, aggregation, and management of multiple MCP backend servers.`, - SilenceUsage: true, // Don't show help on runtime errors - RunE: run, + SilenceUsage: true, // Don't show help on runtime errors + PersistentPreRunE: preRun, + RunE: run, } func init() { + // Set custom error prefix for better branding + rootCmd.SetErrPrefix("MCPG Error:") + rootCmd.Flags().StringVarP(&configFile, "config", "c", defaultConfigFile, "Path to config file") rootCmd.Flags().BoolVar(&configStdin, "config-stdin", defaultConfigStdin, "Read MCP server configuration from stdin (JSON format). When enabled, overrides --config") rootCmd.Flags().StringVarP(&listenAddr, "listen", "l", defaultListenAddr, "HTTP server listen address") @@ -70,10 +75,14 @@ func init() { rootCmd.Flags().BoolVar(&enableDIFC, "enable-difc", defaultEnableDIFC, "Enable DIFC enforcement and session requirement (requires sys___init call before tool access)") rootCmd.Flags().StringVar(&logDir, "log-dir", getDefaultLogDir(), "Directory for log files (falls back to stdout if directory cannot be created)") rootCmd.Flags().BoolVar(&validateEnv, "validate-env", false, "Validate execution environment (Docker, env vars) before starting") + rootCmd.Flags().CountVarP(&verbosity, "verbose", "v", "Increase verbosity level (use -v for info, -vv for debug, -vvv for trace)") // Mark mutually exclusive flags rootCmd.MarkFlagsMutuallyExclusive("routed", "unified") + // Register custom flag completions + registerFlagCompletions(rootCmd) + // Add completion command rootCmd.AddCommand(newCompletionCmd()) } @@ -87,15 +96,66 @@ func getDefaultLogDir() string { return defaultLogDir } -func run(cmd *cobra.Command, args []string) error { - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() +const ( + // Debug log patterns for different verbosity levels + debugMainPackages = "cmd:*,server:*,launcher:*" + debugAllPackages = "*" +) +// registerFlagCompletions registers custom completion functions for flags +func registerFlagCompletions(cmd *cobra.Command) { + // Custom completion for --config flag (complete with .toml files) + cmd.RegisterFlagCompletionFunc("config", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { + return []string{"toml"}, cobra.ShellCompDirectiveFilterFileExt + }) + + // Custom completion for --log-dir flag (complete with directories) + cmd.RegisterFlagCompletionFunc("log-dir", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { + return nil, cobra.ShellCompDirectiveFilterDirs + }) + + // Custom completion for --env flag (complete with .env files) + cmd.RegisterFlagCompletionFunc("env", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { + return []string{"env"}, cobra.ShellCompDirectiveFilterFileExt + }) +} + +// preRun performs validation before command execution +func preRun(cmd *cobra.Command, args []string) error { // Validate that either --config or --config-stdin is provided if !configStdin && configFile == "" { return fmt.Errorf("configuration source required: specify either --config or --config-stdin") } + // Apply verbosity level to logging (if DEBUG is not already set) + // -v (1): info level, -vv (2): debug level, -vvv (3): trace level + if verbosity > 0 && os.Getenv("DEBUG") == "" { + // Set DEBUG env var based on verbosity level + // Level 1: basic info (no special DEBUG setting needed, handled by logger) + // Level 2: enable debug logs for cmd and server packages + // Level 3: enable all debug logs + switch verbosity { + case 1: + // Info level - no special DEBUG setting (standard log output) + debugLog.Printf("Verbosity level: info") + case 2: + // Debug level - enable debug logs for main packages + os.Setenv("DEBUG", debugMainPackages) + debugLog.Printf("Verbosity level: debug (DEBUG=%s)", debugMainPackages) + default: + // Trace level (3+) - enable all debug logs + os.Setenv("DEBUG", debugAllPackages) + debugLog.Printf("Verbosity level: trace (DEBUG=%s)", debugAllPackages) + } + } + + return nil +} + +func run(cmd *cobra.Command, args []string) error { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + // Initialize file logger early if err := logger.InitFileLogger(logDir, "mcp-gateway.log"); err != nil { log.Printf("Warning: Failed to initialize file logger: %v", err) diff --git a/internal/cmd/root_test.go b/internal/cmd/root_test.go index bf481937..cfd2eee4 100644 --- a/internal/cmd/root_test.go +++ b/internal/cmd/root_test.go @@ -79,7 +79,7 @@ func TestRunRequiresConfigSource(t *testing.T) { t.Run("no config source provided", func(t *testing.T) { configFile = "" configStdin = false - err := run(nil, nil) + err := preRun(nil, nil) require.Error(t, err, "Expected error when neither --config nor --config-stdin is provided") assert.Contains(t, err.Error(), "configuration source required", "Error should mention configuration source required") }) @@ -87,37 +87,145 @@ func TestRunRequiresConfigSource(t *testing.T) { t.Run("config file provided", func(t *testing.T) { configFile = "test.toml" configStdin = false - err := run(nil, nil) - // Should not be the "configuration source required" error - // (will fail later due to missing file, but should pass validation) - if err != nil { - assert.NotContains(t, err.Error(), "configuration source required", - "Should not require config source when --config is provided") - } + err := preRun(nil, nil) + // Should pass validation when --config is provided + assert.NoError(t, err, "Should not error when --config is provided") }) t.Run("config stdin provided", func(t *testing.T) { configFile = "" configStdin = true - err := run(nil, nil) - // Should not be the "configuration source required" error - // (will fail later due to stdin not being readable, but should pass validation) - if err != nil { - assert.NotContains(t, err.Error(), "configuration source required", - "Should not require config source when --config-stdin is provided") - } + err := preRun(nil, nil) + // Should pass validation when --config-stdin is provided + assert.NoError(t, err, "Should not error when --config-stdin is provided") }) t.Run("both config file and stdin provided", func(t *testing.T) { configFile = "test.toml" configStdin = true - err := run(nil, nil) - // When both are provided, stdin takes precedence per flag description - // Should not be the "configuration source required" error - if err != nil { - assert.NotContains(t, err.Error(), "configuration source required", - "Should not require config source when both are provided") - } + err := preRun(nil, nil) + // When both are provided, should pass validation + assert.NoError(t, err, "Should not error when both are provided") + }) +} + +// TestPreRunValidation tests the preRun validation function +func TestPreRunValidation(t *testing.T) { + // Save original values + origConfigFile := configFile + origConfigStdin := configStdin + origVerbosity := verbosity + t.Cleanup(func() { + configFile = origConfigFile + configStdin = origConfigStdin + verbosity = origVerbosity + }) + + t.Run("validation passes with config file", func(t *testing.T) { + configFile = "test.toml" + configStdin = false + verbosity = 0 + err := preRun(nil, nil) + assert.NoError(t, err) + }) + + t.Run("validation passes with config stdin", func(t *testing.T) { + configFile = "" + configStdin = true + verbosity = 0 + err := preRun(nil, nil) + assert.NoError(t, err) + }) + + t.Run("validation fails without config source", func(t *testing.T) { + configFile = "" + configStdin = false + verbosity = 0 + err := preRun(nil, nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "configuration source required") + }) + + t.Run("verbosity level 1 does not set DEBUG", func(t *testing.T) { + // Save and clear DEBUG env var + origDebug, wasSet := os.LookupEnv("DEBUG") + t.Cleanup(func() { + if wasSet { + os.Setenv("DEBUG", origDebug) + } else { + os.Unsetenv("DEBUG") + } + }) + os.Unsetenv("DEBUG") + + configFile = "test.toml" + configStdin = false + verbosity = 1 + err := preRun(nil, nil) + assert.NoError(t, err) + // Level 1 doesn't set DEBUG env var + assert.Empty(t, os.Getenv("DEBUG")) + }) + + t.Run("verbosity level 2 sets DEBUG for main packages", func(t *testing.T) { + // Save and clear DEBUG env var + origDebug, wasSet := os.LookupEnv("DEBUG") + t.Cleanup(func() { + if wasSet { + os.Setenv("DEBUG", origDebug) + } else { + os.Unsetenv("DEBUG") + } + }) + os.Unsetenv("DEBUG") + + configFile = "test.toml" + configStdin = false + verbosity = 2 + err := preRun(nil, nil) + assert.NoError(t, err) + assert.Equal(t, "cmd:*,server:*,launcher:*", os.Getenv("DEBUG")) + }) + + t.Run("verbosity level 3 sets DEBUG to all", func(t *testing.T) { + // Save and clear DEBUG env var + origDebug, wasSet := os.LookupEnv("DEBUG") + t.Cleanup(func() { + if wasSet { + os.Setenv("DEBUG", origDebug) + } else { + os.Unsetenv("DEBUG") + } + }) + os.Unsetenv("DEBUG") + + configFile = "test.toml" + configStdin = false + verbosity = 3 + err := preRun(nil, nil) + assert.NoError(t, err) + assert.Equal(t, "*", os.Getenv("DEBUG")) + }) + + t.Run("does not override existing DEBUG env var", func(t *testing.T) { + // Save DEBUG env var + origDebug, wasSet := os.LookupEnv("DEBUG") + t.Cleanup(func() { + if wasSet { + os.Setenv("DEBUG", origDebug) + } else { + os.Unsetenv("DEBUG") + } + }) + os.Setenv("DEBUG", "custom:*") + + configFile = "test.toml" + configStdin = false + verbosity = 2 + err := preRun(nil, nil) + assert.NoError(t, err) + // Should not override existing DEBUG + assert.Equal(t, "custom:*", os.Getenv("DEBUG")) }) } diff --git a/main.go b/main.go index 8a6c9101..c6574f2a 100644 --- a/main.go +++ b/main.go @@ -1,8 +1,68 @@ package main -import "github.com/githubnext/gh-aw-mcpg/internal/cmd" +import ( + "fmt" + "runtime/debug" + "strings" + + "github.com/githubnext/gh-aw-mcpg/internal/cmd" +) func main() { - cmd.SetVersion(Version) + // Build version string with metadata + versionStr := buildVersionString() + + // Set the version for the CLI + cmd.SetVersion(versionStr) + + // Execute the root command cmd.Execute() } + +const ( + shortHashLength = 7 // Length for short git commit hash +) + +// buildVersionString constructs a detailed version string with build metadata +func buildVersionString() string { + var parts []string + + // Add main version + if Version != "" { + parts = append(parts, Version) + } else { + parts = append(parts, "dev") + } + + // Add git commit if available + if GitCommit != "" { + parts = append(parts, fmt.Sprintf("commit: %s", GitCommit)) + } else if buildInfo, ok := debug.ReadBuildInfo(); ok { + // Try to extract commit from build info if not set via ldflags + for _, setting := range buildInfo.Settings { + if setting.Key == "vcs.revision" { + commitHash := setting.Value + if len(commitHash) > shortHashLength { + commitHash = commitHash[:shortHashLength] // Short hash + } + parts = append(parts, fmt.Sprintf("commit: %s", commitHash)) + break + } + } + } + + // Add build date if available + if BuildDate != "" { + parts = append(parts, fmt.Sprintf("built: %s", BuildDate)) + } else if buildInfo, ok := debug.ReadBuildInfo(); ok { + // Try to extract build time from build info if not set via ldflags + for _, setting := range buildInfo.Settings { + if setting.Key == "vcs.time" { + parts = append(parts, fmt.Sprintf("built: %s", setting.Value)) + break + } + } + } + + return strings.Join(parts, ", ") +} diff --git a/main_test.go b/main_test.go new file mode 100644 index 00000000..782ce925 --- /dev/null +++ b/main_test.go @@ -0,0 +1,119 @@ +package main + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestBuildVersionString(t *testing.T) { + tests := []struct { + name string + version string + gitCommit string + buildDate string + expectedParts []string + unexpectedPart string + }{ + { + name: "all metadata present", + version: "v1.0.0", + gitCommit: "abc123", + buildDate: "2026-01-21T10:00:00Z", + expectedParts: []string{"v1.0.0", "commit: abc123", "built: 2026-01-21T10:00:00Z"}, + }, + { + name: "only version", + version: "v1.0.0", + gitCommit: "", + buildDate: "", + expectedParts: []string{"v1.0.0"}, + }, + { + name: "version with commit", + version: "v1.0.0", + gitCommit: "abc123", + buildDate: "", + expectedParts: []string{"v1.0.0", "commit: abc123"}, + }, + { + name: "version with build date", + version: "v1.0.0", + gitCommit: "", + buildDate: "2026-01-21T10:00:00Z", + expectedParts: []string{"v1.0.0", "built: 2026-01-21T10:00:00Z"}, + }, + { + name: "no version defaults to dev", + version: "", + gitCommit: "", + buildDate: "", + expectedParts: []string{"dev"}, + unexpectedPart: "commit:", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Set test values + origVersion := Version + origGitCommit := GitCommit + origBuildDate := BuildDate + t.Cleanup(func() { + Version = origVersion + GitCommit = origGitCommit + BuildDate = origBuildDate + }) + + Version = tt.version + GitCommit = tt.gitCommit + BuildDate = tt.buildDate + + result := buildVersionString() + + // Check expected parts + for _, part := range tt.expectedParts { + assert.Contains(t, result, part, "Version string should contain: %s", part) + } + + // Check unexpected parts + if tt.unexpectedPart != "" { + assert.NotContains(t, result, tt.unexpectedPart, "Version string should not contain: %s", tt.unexpectedPart) + } + + // Ensure parts are comma-separated + if len(tt.expectedParts) > 1 { + assert.True(t, strings.Contains(result, ", "), "Multi-part version should be comma-separated") + } + }) + } +} + +func TestBuildVersionString_UsesVCSInfo(t *testing.T) { + // When ldflags are not set, buildVersionString should fall back to VCS info from runtime/debug + // This test verifies the code structure, actual VCS extraction depends on build settings + + // Set empty values to trigger VCS fallback + origVersion := Version + origGitCommit := GitCommit + origBuildDate := BuildDate + t.Cleanup(func() { + Version = origVersion + GitCommit = origGitCommit + BuildDate = origBuildDate + }) + + Version = "" + GitCommit = "" + BuildDate = "" + + result := buildVersionString() + + // Should at least have "dev" + assert.Contains(t, result, "dev", "Should contain 'dev' when Version is empty") + + // May or may not have commit/built info depending on VCS availability + // Just verify it doesn't panic or return empty string + assert.NotEmpty(t, result, "Should return non-empty string") +} diff --git a/version.go b/version.go index 91da8bc0..14b0754f 100644 --- a/version.go +++ b/version.go @@ -5,4 +5,12 @@ var ( // Version is the semantic version of the binary (e.g., "v1.0.0") // Set via -ldflags "-X main.Version=" during build Version = "dev" + + // GitCommit is the git commit hash at build time + // Set via -ldflags "-X main.GitCommit=" during build + GitCommit = "" + + // BuildDate is the date/time of the build (RFC3339 format) + // Set via -ldflags "-X main.BuildDate=" during build + BuildDate = "" )