diff --git a/CLAUDE.md b/CLAUDE.md index 4c692c8e..7aceff6f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -61,6 +61,19 @@ mcpproxy doctor # Run health checks See [docs/cli-management-commands.md](docs/cli-management-commands.md) for complete reference. +### CLI Output Formatting +```bash +mcpproxy upstream list -o json # JSON output for scripting +mcpproxy upstream list -o yaml # YAML output +mcpproxy upstream list --json # Shorthand for -o json +mcpproxy --help-json # Machine-readable help for AI agents +``` + +**Formats**: `table` (default), `json`, `yaml` +**Environment**: `MCPPROXY_OUTPUT=json` sets default format + +See [docs/cli-output-formatting.md](docs/cli-output-formatting.md) for complete reference. + ## Architecture Overview ### Core Components @@ -69,6 +82,7 @@ See [docs/cli-management-commands.md](docs/cli-management-commands.md) for compl |-----------|---------| | `cmd/mcpproxy/` | CLI entry point, Cobra commands | | `cmd/mcpproxy-tray/` | System tray application with state machine | +| `internal/cli/output/` | CLI output formatters (table, JSON, YAML) | | `internal/runtime/` | Lifecycle, event bus, background services | | `internal/server/` | HTTP server, MCP proxy | | `internal/httpapi/` | REST API endpoints (`/api/v1`) | @@ -333,6 +347,8 @@ See `docs/prerelease-builds.md` for download instructions. ## Active Technologies - Go 1.24 (toolchain go1.24.10) (001-update-version-display) - In-memory only for version cache (no persistence per clarification) (001-update-version-display) +- Go 1.24 (toolchain go1.24.10) + Cobra CLI framework, encoding/json, gopkg.in/yaml.v3 (014-cli-output-formatting) +- N/A (CLI output only) (014-cli-output-formatting) ## Recent Changes - 001-update-version-display: Added Go 1.24 (toolchain go1.24.10) diff --git a/cmd/mcpproxy/call_cmd.go b/cmd/mcpproxy/call_cmd.go index 0577bed1..3c83aa13 100644 --- a/cmd/mcpproxy/call_cmd.go +++ b/cmd/mcpproxy/call_cmd.go @@ -10,6 +10,7 @@ import ( "time" "mcpproxy-go/internal/cache" + "mcpproxy-go/internal/cli/output" "mcpproxy-go/internal/cliclient" "mcpproxy-go/internal/config" "mcpproxy-go/internal/index" @@ -25,6 +26,8 @@ import ( "go.uber.org/zap" ) +// Call command output format constants (kept for backward compatibility) +// Note: "pretty" is the default for call command (different from global "table" default) const ( outputFormatJSON = "json" outputFormatPretty = "pretty" @@ -208,13 +211,14 @@ func loadCallConfig() (*config.Config, error) { return globalConfig, nil } -// outputCallResultAsJSON outputs the result in JSON format +// outputCallResultAsJSON outputs the result in JSON format using unified formatter func outputCallResultAsJSON(result interface{}) error { - output, err := json.MarshalIndent(result, "", " ") + formatter := &output.JSONFormatter{Indent: true} + formatted, err := formatter.Format(result) if err != nil { return fmt.Errorf("failed to format result as JSON: %w", err) } - fmt.Println(string(output)) + fmt.Println(formatted) return nil } diff --git a/cmd/mcpproxy/main.go b/cmd/mcpproxy/main.go index 40892a0e..c3903e04 100644 --- a/cmd/mcpproxy/main.go +++ b/cmd/mcpproxy/main.go @@ -38,6 +38,7 @@ import ( bbolterrors "go.etcd.io/bbolt/errors" "go.uber.org/zap" + "mcpproxy-go/internal/cli/output" "mcpproxy-go/internal/config" "mcpproxy-go/internal/experiments" "mcpproxy-go/internal/logs" @@ -66,6 +67,10 @@ var ( allowServerRemove bool enablePrompts bool + // Output formatting flags (global) + globalOutputFormat string + globalJSONOutput bool + version = "v0.1.0" // This will be injected by -ldflags during build ) @@ -98,6 +103,11 @@ func main() { rootCmd.PersistentFlags().BoolVar(&logToFile, "log-to-file", false, "Enable logging to file in standard OS location (default: console only)") rootCmd.PersistentFlags().StringVar(&logDir, "log-dir", "", "Custom log directory path (overrides standard OS location)") + // Output formatting flags (global) + rootCmd.PersistentFlags().StringVarP(&globalOutputFormat, "output", "o", "", "Output format: table, json, yaml") + rootCmd.PersistentFlags().BoolVar(&globalJSONOutput, "json", false, "Shorthand for -o json") + rootCmd.MarkFlagsMutuallyExclusive("output", "json") + // Add server command serverCmd := &cobra.Command{ Use: "serve", @@ -157,6 +167,10 @@ func main() { rootCmd.AddCommand(upstreamCmd) rootCmd.AddCommand(doctorCmd) + // Setup --help-json for machine-readable help discovery + // This must be called AFTER all commands are added + output.SetupHelpJSON(rootCmd) + // Default to server command for backward compatibility rootCmd.RunE = runServer @@ -633,3 +647,14 @@ func classifyError(err error) int { // Default to general error return ExitCodeGeneralError } + +// ResolveOutputFormat determines the output format from global flags and environment. +// Priority: --json alias > -o flag > MCPPROXY_OUTPUT env var > default (table) +func ResolveOutputFormat() string { + return output.ResolveFormat(globalOutputFormat, globalJSONOutput) +} + +// GetOutputFormatter creates a formatter for the resolved output format. +func GetOutputFormatter() (output.OutputFormatter, error) { + return output.NewFormatter(ResolveOutputFormat()) +} diff --git a/cmd/mcpproxy/secrets_cmd.go b/cmd/mcpproxy/secrets_cmd.go index 90a27ec7..64f031b2 100644 --- a/cmd/mcpproxy/secrets_cmd.go +++ b/cmd/mcpproxy/secrets_cmd.go @@ -2,7 +2,6 @@ package main import ( "context" - "encoding/json" "fmt" "os" "strings" @@ -10,6 +9,7 @@ import ( "github.com/spf13/cobra" + "mcpproxy-go/internal/cli/output" "mcpproxy-go/internal/config" "mcpproxy-go/internal/logs" "mcpproxy-go/internal/secret" @@ -185,10 +185,7 @@ func getSecretsDeleteCommand() *cobra.Command { // getSecretsListCommand returns the secrets list command func getSecretsListCommand() *cobra.Command { - var ( - jsonOutput bool - allTypes bool - ) + var allTypes bool cmd := &cobra.Command{ Use: "list", @@ -199,26 +196,24 @@ func getSecretsListCommand() *cobra.Command { ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() + // Get output format from global flags + outputFormat := ResolveOutputFormat() + formatter, err := GetOutputFormatter() + if err != nil { + return output.NewStructuredError(output.ErrCodeInvalidOutputFormat, err.Error()). + WithGuidance("Use -o table, -o json, or -o yaml") + } + + var refs []secret.Ref + var providerName string + if allTypes { // List from all available providers - refs, err := resolver.ListAll(ctx) + refs, err = resolver.ListAll(ctx) if err != nil { return fmt.Errorf("failed to list secrets: %w", err) } - - if jsonOutput { - return json.NewEncoder(os.Stdout).Encode(refs) - } - - if len(refs) == 0 { - fmt.Println("No secrets found") - return nil - } - - fmt.Printf("Found %d secrets:\n", len(refs)) - for _, ref := range refs { - fmt.Printf(" %s (%s)\n", ref.Name, ref.Type) - } + providerName = "all providers" } else { // List from keyring only keyringProvider := secret.NewKeyringProvider() @@ -226,31 +221,45 @@ func getSecretsListCommand() *cobra.Command { return fmt.Errorf("keyring is not available on this system") } - refs, err := keyringProvider.List(ctx) + refs, err = keyringProvider.List(ctx) if err != nil { return fmt.Errorf("failed to list keyring secrets: %w", err) } + providerName = "keyring" + } - if jsonOutput { - return json.NewEncoder(os.Stdout).Encode(refs) + // Handle JSON/YAML output + if outputFormat == "json" || outputFormat == "yaml" { + result, fmtErr := formatter.Format(refs) + if fmtErr != nil { + return fmt.Errorf("failed to format output: %w", fmtErr) } + fmt.Println(result) + return nil + } - if len(refs) == 0 { - fmt.Println("No secrets found in keyring") - return nil - } + // Table output + if len(refs) == 0 { + fmt.Printf("No secrets found in %s\n", providerName) + return nil + } - fmt.Printf("Found %d secrets in keyring:\n", len(refs)) - for _, ref := range refs { - fmt.Printf(" %s\n", ref.Name) - } + headers := []string{"NAME", "TYPE"} + var rows [][]string + for _, ref := range refs { + rows = append(rows, []string{ref.Name, ref.Type}) } + result, fmtErr := formatter.FormatTable(headers, rows) + if fmtErr != nil { + return fmt.Errorf("failed to format table: %w", fmtErr) + } + fmt.Print(result) return nil }, } - cmd.Flags().BoolVar(&jsonOutput, "json", false, "Output in JSON format") + // Note: --json flag removed, use global -o json instead cmd.Flags().BoolVar(&allTypes, "all", false, "List secrets from all available providers") return cmd diff --git a/cmd/mcpproxy/tools_cmd.go b/cmd/mcpproxy/tools_cmd.go index 0f618f5a..2ddc84ac 100644 --- a/cmd/mcpproxy/tools_cmd.go +++ b/cmd/mcpproxy/tools_cmd.go @@ -2,12 +2,12 @@ package main import ( "context" - "encoding/json" "fmt" "os" "path/filepath" "time" + "mcpproxy-go/internal/cli/output" "mcpproxy-go/internal/cliclient" "mcpproxy-go/internal/config" "mcpproxy-go/internal/logs" @@ -47,7 +47,6 @@ Examples: toolsLogLevel string configPath string timeout time.Duration - outputFormat string traceTransport bool // Enable HTTP/SSE frame-by-frame tracing ) @@ -65,7 +64,7 @@ func init() { toolsListCmd.Flags().StringVarP(&toolsLogLevel, "log-level", "l", "info", "Log level (trace, debug, info, warn, error)") toolsListCmd.Flags().StringVarP(&configPath, "config", "c", "", "Path to MCP configuration file (default: ~/.mcpproxy/mcp_config.json)") toolsListCmd.Flags().DurationVarP(&timeout, "timeout", "t", 30*time.Second, "Connection timeout") - toolsListCmd.Flags().StringVarP(&outputFormat, "output", "o", "table", "Output format (table, json, yaml)") + // Note: -o/--output flag is inherited from root command via globalOutputFormat toolsListCmd.Flags().BoolVar(&traceTransport, "trace-transport", false, "Enable detailed HTTP/SSE frame-by-frame tracing (useful for debugging SSE connection issues)") // Mark required flags @@ -164,45 +163,52 @@ func getAvailableServerNames(globalConfig *config.Config) []string { return names } -// displayToolsTable displays tools in a formatted table -func displayToolsTable(tools []*config.ToolMetadata, serverName string) { - fmt.Printf("πŸ“š Discovered Tools (%d):\n", len(tools)) - fmt.Printf("━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━\n") - +// outputToolsFromMetadata formats and displays tools from ToolMetadata (standalone mode) using unified formatters. +func outputToolsFromMetadata(tools []*config.ToolMetadata, serverName string) error { + // Convert to map format for unified output + toolMaps := make([]map[string]interface{}, len(tools)) for i, tool := range tools { - fmt.Printf("%d. %s\n", i+1, tool.Name) - if tool.Description != "" { - fmt.Printf(" πŸ“ %s\n", tool.Description) + toolMaps[i] = map[string]interface{}{ + "name": tool.Name, + "description": tool.Description, + "server": serverName, + "full_name": fmt.Sprintf("%s:%s", serverName, tool.Name), } - - // Show schema in debug/trace mode - if toolsLogLevel == "debug" || toolsLogLevel == "trace" { - if tool.ParamsJSON != "" { - fmt.Printf(" πŸ”§ Schema: %s\n", tool.ParamsJSON) - } + // Include schema in debug/trace mode + if (toolsLogLevel == "debug" || toolsLogLevel == "trace") && tool.ParamsJSON != "" { + toolMaps[i]["schema"] = tool.ParamsJSON } + } - fmt.Printf(" 🏷️ Format: %s:%s\n", serverName, tool.Name) + outputFormat := ResolveOutputFormat() + formatter, err := GetOutputFormatter() + if err != nil { + return output.NewStructuredError(output.ErrCodeInvalidOutputFormat, err.Error()). + WithGuidance("Use -o table, -o json, or -o yaml") + } - if i < len(tools)-1 { - fmt.Println() + // For JSON/YAML, format directly + if outputFormat == "json" || outputFormat == "yaml" { + result, fmtErr := formatter.Format(toolMaps) + if fmtErr != nil { + return fmt.Errorf("failed to format output: %w", fmtErr) } + fmt.Println(result) + return nil } - fmt.Printf("━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━\n") -} - -// outputToolsAsJSON outputs tools in JSON format -func outputToolsAsJSON(_ []*config.ToolMetadata) error { - // This would use encoding/json to output tools - fmt.Printf("πŸ“„ JSON output not yet implemented\n") - return nil -} + // Table format: show name and description + headers := []string{"NAME", "DESCRIPTION"} + var rows [][]string + for _, tool := range tools { + rows = append(rows, []string{tool.Name, tool.Description}) + } -// outputToolsAsYAML outputs tools in YAML format -func outputToolsAsYAML(_ []*config.ToolMetadata) error { - // This would use gopkg.in/yaml.v3 to output tools - fmt.Printf("πŸ“„ YAML output not yet implemented\n") + result, fmtErr := formatter.FormatTable(headers, rows) + if fmtErr != nil { + return fmt.Errorf("failed to format table: %w", fmtErr) + } + fmt.Print(result) return nil } @@ -241,40 +247,42 @@ func runToolsListClientMode(ctx context.Context, dataDir, serverName string, log } // Output results - return outputTools(tools, outputFormat, logger) + return outputTools(tools, logger) } -// outputTools formats and displays tools based on output format. -func outputTools(tools []map[string]interface{}, format string, logger *zap.Logger) error { - switch format { - case "json": - output, err := json.MarshalIndent(tools, "", " ") - if err != nil { - return fmt.Errorf("failed to format tools as JSON: %w", err) - } - fmt.Println(string(output)) - case "yaml": - // YAML output implementation (if needed) - return fmt.Errorf("YAML output not yet implemented, use json or table") - case "table": - fallthrough - default: - // Table output - fmt.Printf("━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━\n") - fmt.Printf("πŸ”§ Tools Available (%d total)\n", len(tools)) - fmt.Printf("━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━\n\n") - - for _, tool := range tools { - name, _ := tool["name"].(string) - desc, _ := tool["description"].(string) - - fmt.Printf("πŸ“Œ %s\n", name) - if desc != "" { - fmt.Printf(" %s\n", desc) - } - fmt.Println() +// outputTools formats and displays tools based on output format using unified formatters. +func outputTools(tools []map[string]interface{}, _ *zap.Logger) error { + outputFormat := ResolveOutputFormat() + formatter, err := GetOutputFormatter() + if err != nil { + return output.NewStructuredError(output.ErrCodeInvalidOutputFormat, err.Error()). + WithGuidance("Use -o table, -o json, or -o yaml") + } + + // For JSON/YAML, format directly + if outputFormat == "json" || outputFormat == "yaml" { + result, fmtErr := formatter.Format(tools) + if fmtErr != nil { + return fmt.Errorf("failed to format output: %w", fmtErr) } + fmt.Println(result) + return nil } + + // Table format: show name and description + headers := []string{"NAME", "DESCRIPTION"} + var rows [][]string + for _, tool := range tools { + name, _ := tool["name"].(string) + desc, _ := tool["description"].(string) + rows = append(rows, []string{name, desc}) + } + + result, fmtErr := formatter.FormatTable(headers, rows) + if fmtErr != nil { + return fmt.Errorf("failed to format table: %w", fmtErr) + } + fmt.Print(result) return nil } @@ -347,28 +355,19 @@ func runToolsListStandalone(ctx context.Context, serverName string, globalConfig return fmt.Errorf("failed to list tools: %w", err) } - // Output results based on format - switch outputFormat { - case "json": - return outputToolsAsJSON(tools) - case "yaml": - return outputToolsAsYAML(tools) - default: - // Table format (default) - fmt.Printf("βœ… Tool discovery completed successfully!\n\n") - - if len(tools) == 0 { + // Output results using unified formatter + if len(tools) == 0 { + outputFormat := ResolveOutputFormat() + if outputFormat == "table" { fmt.Printf("⚠️ No tools found on server '%s'\n", serverName) fmt.Printf("πŸ’‘ This could indicate:\n") fmt.Printf(" β€’ Server doesn't support tools\n") fmt.Printf(" β€’ Server is not properly configured\n") fmt.Printf(" β€’ Connection issues during tool discovery\n") - } else { - fmt.Printf("πŸŽ‰ Found %d tool(s) on server '%s'\n\n", len(tools), serverName) - displayToolsTable(tools, serverName) - fmt.Printf("\nπŸ’‘ Use these tools with: mcpproxy call_tool --tool=%s:\n", serverName) + return nil } + // For JSON/YAML, output empty array } - return nil + return outputToolsFromMetadata(tools, serverName) } diff --git a/cmd/mcpproxy/upstream_cmd.go b/cmd/mcpproxy/upstream_cmd.go index f2ff3e44..cd3480a9 100644 --- a/cmd/mcpproxy/upstream_cmd.go +++ b/cmd/mcpproxy/upstream_cmd.go @@ -2,7 +2,6 @@ package main import ( "context" - "encoding/json" "fmt" "os" "os/exec" @@ -16,6 +15,7 @@ import ( "github.com/spf13/cobra" "go.uber.org/zap" + "mcpproxy-go/internal/cli/output" "mcpproxy-go/internal/cliclient" "mcpproxy-go/internal/config" "mcpproxy-go/internal/health" @@ -78,8 +78,7 @@ Examples: } // Command flags - upstreamOutputFormat string - upstreamLogLevel string + upstreamLogLevel string upstreamConfigPath string upstreamLogsTail int upstreamLogsFollow bool @@ -103,8 +102,7 @@ func init() { upstreamCmd.AddCommand(upstreamDisableCmd) upstreamCmd.AddCommand(upstreamRestartCmd) - // Define flags - upstreamListCmd.Flags().StringVarP(&upstreamOutputFormat, "output", "o", "table", "Output format (table, json)") + // Define flags (note: output format handled by global --output/-o flag from root command) upstreamListCmd.Flags().StringVarP(&upstreamLogLevel, "log-level", "l", "warn", "Log level (trace, debug, info, warn, error)") upstreamListCmd.Flags().StringVarP(&upstreamConfigPath, "config", "c", "", "Path to MCP configuration file") @@ -134,15 +132,15 @@ func runUpstreamList(_ *cobra.Command, _ []string) error { // Load configuration globalConfig, err := loadUpstreamConfig() if err != nil { - fmt.Fprintf(os.Stderr, "Error loading configuration: %v\n", err) - return err + return outputError(output.NewStructuredError(output.ErrCodeConfigNotFound, err.Error()). + WithGuidance("Check that your config file exists and is valid"). + WithRecoveryCommand("mcpproxy doctor"), output.ErrCodeConfigNotFound) } // Create logger logger, err := createUpstreamLogger(upstreamLogLevel) if err != nil { - fmt.Fprintf(os.Stderr, "Error creating logger: %v\n", err) - return err + return outputError(err, output.ErrCodeOperationFailed) } // Check if daemon is running @@ -168,7 +166,9 @@ func runUpstreamListClientMode(ctx context.Context, dataDir string, logger *zap. // Call GET /api/v1/servers servers, err := client.GetServers(ctx) if err != nil { - return fmt.Errorf("failed to get servers from daemon: %w", err) + return outputError(output.NewStructuredError(output.ErrCodeConnectionFailed, err.Error()). + WithGuidance("Ensure the mcpproxy daemon is running"). + WithRecoveryCommand("mcpproxy serve"), output.ErrCodeConnectionFailed) } return outputServers(servers) @@ -223,81 +223,137 @@ func outputServers(servers []map[string]interface{}) error { return nameI < nameJ }) - switch upstreamOutputFormat { - case "json": - output, err := json.MarshalIndent(servers, "", " ") + // Get the output formatter based on global flags + formatter, err := GetOutputFormatter() + if err != nil { + return output.NewStructuredError(output.ErrCodeInvalidOutputFormat, err.Error()). + WithGuidance("Use -o table, -o json, or -o yaml") + } + + outputFormat := ResolveOutputFormat() + + // For structured formats (json, yaml), output raw data + if outputFormat == "json" || outputFormat == "yaml" { + result, err := formatter.Format(servers) if err != nil { return fmt.Errorf("failed to format output: %w", err) } - fmt.Println(string(output)) - case "table", "": - // Table format (default) with unified health status - fmt.Printf("%-4s %-25s %-10s %-10s %-30s %s\n", - "", "NAME", "PROTOCOL", "TOOLS", "STATUS", "ACTION") - fmt.Printf("━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━\n") - - for _, srv := range servers { - name := getStringField(srv, "name") - protocol := getStringField(srv, "protocol") - toolCount := getIntField(srv, "tool_count") - - // Extract unified health status - healthData, _ := srv["health"].(map[string]interface{}) - healthLevel := "unknown" - healthAdminState := "enabled" - healthSummary := getStringField(srv, "status") // fallback to old status - healthAction := "" - - if healthData != nil { - healthLevel = getStringField(healthData, "level") - healthAdminState = getStringField(healthData, "admin_state") - healthSummary = getStringField(healthData, "summary") - healthAction = getStringField(healthData, "action") - } + fmt.Println(result) + return nil + } - // Status emoji based on health level and admin state - statusEmoji := "βšͺ" // unknown - switch healthAdminState { - case "disabled": - statusEmoji = "⏸️ " // paused - case "quarantined": - statusEmoji = "πŸ”’" // locked - default: - switch healthLevel { - case "healthy": - statusEmoji = "βœ…" - case "degraded": - statusEmoji = "⚠️ " - case "unhealthy": - statusEmoji = "❌" - } - } + // For table format, build headers and rows with formatted data + headers := []string{"", "NAME", "PROTOCOL", "TOOLS", "STATUS", "ACTION"} + rows := make([][]string, 0, len(servers)) + + for _, srv := range servers { + name := getStringField(srv, "name") + protocol := getStringField(srv, "protocol") + toolCount := getIntField(srv, "tool_count") + + // Extract unified health status + healthData, _ := srv["health"].(map[string]interface{}) + healthLevel := "unknown" + healthAdminState := "enabled" + healthSummary := getStringField(srv, "status") // fallback to old status + healthAction := "" + + if healthData != nil { + healthLevel = getStringField(healthData, "level") + healthAdminState = getStringField(healthData, "admin_state") + healthSummary = getStringField(healthData, "summary") + healthAction = getStringField(healthData, "action") + } - // Format action as CLI command hint - actionHint := "-" - switch healthAction { - case "login": - actionHint = fmt.Sprintf("auth login --server=%s", name) - case "restart": - actionHint = fmt.Sprintf("upstream restart %s", name) - case "enable": - actionHint = fmt.Sprintf("upstream enable %s", name) - case "approve": - actionHint = "Approve in Web UI" - case "view_logs": - actionHint = fmt.Sprintf("upstream logs %s", name) + // Status emoji based on health level and admin state + statusEmoji := "βšͺ" // unknown + switch healthAdminState { + case "disabled": + statusEmoji = "⏸️ " // paused + case "quarantined": + statusEmoji = "πŸ”’" // locked + default: + switch healthLevel { + case "healthy": + statusEmoji = "βœ…" + case "degraded": + statusEmoji = "⚠️ " + case "unhealthy": + statusEmoji = "❌" } + } - fmt.Printf("%-4s %-25s %-10s %-10d %-30s %s\n", - statusEmoji, name, protocol, toolCount, healthSummary, actionHint) + // Format action as CLI command hint + actionHint := "-" + switch healthAction { + case "login": + actionHint = fmt.Sprintf("auth login --server=%s", name) + case "restart": + actionHint = fmt.Sprintf("upstream restart %s", name) + case "enable": + actionHint = fmt.Sprintf("upstream enable %s", name) + case "approve": + actionHint = "Approve in Web UI" + case "view_logs": + actionHint = fmt.Sprintf("upstream logs %s", name) } - default: - return fmt.Errorf("unknown output format: %s", upstreamOutputFormat) + + rows = append(rows, []string{ + statusEmoji, + name, + protocol, + fmt.Sprintf("%d", toolCount), + healthSummary, + actionHint, + }) } + result, err := formatter.FormatTable(headers, rows) + if err != nil { + return fmt.Errorf("failed to format table: %w", err) + } + fmt.Print(result) return nil } +// outputError formats and outputs an error based on the current output format. +// For structured formats (json, yaml), it outputs a StructuredError. +// For table format, it outputs a human-readable error message to stderr. +func outputError(err error, code string) error { + outputFormat := ResolveOutputFormat() + + // Convert to StructuredError if not already + var structErr output.StructuredError + if se, ok := err.(output.StructuredError); ok { + structErr = se + } else { + structErr = output.NewStructuredError(code, err.Error()) + } + + // For structured formats, output JSON/YAML error to stdout + if outputFormat == "json" || outputFormat == "yaml" { + formatter, fmtErr := GetOutputFormatter() + if fmtErr != nil { + // Fallback to plain error if formatter fails + fmt.Fprintf(os.Stderr, "Error: %v\n", err) + return err + } + + result, formatErr := formatter.FormatError(structErr) + if formatErr != nil { + fmt.Fprintf(os.Stderr, "Error: %v\n", err) + return err + } + + fmt.Println(result) + return structErr + } + + // For table format, output human-readable error to stderr + fmt.Fprintf(os.Stderr, "Error: %v\n", err) + return err +} + func loadUpstreamConfig() (*config.Config, error) { if upstreamConfigPath != "" { return config.LoadFromFile(upstreamConfigPath) diff --git a/cmd/mcpproxy/upstream_cmd_test.go b/cmd/mcpproxy/upstream_cmd_test.go index 0a4f7d8a..83009599 100644 --- a/cmd/mcpproxy/upstream_cmd_test.go +++ b/cmd/mcpproxy/upstream_cmd_test.go @@ -37,7 +37,9 @@ func TestOutputServers_TableFormat(t *testing.T) { os.Stdout = w defer func() { os.Stdout = oldStdout }() - upstreamOutputFormat = "table" + // Use global output format (default is "table") + globalOutputFormat = "table" + globalJSONOutput = false err := outputServers(servers) w.Close() @@ -93,7 +95,9 @@ func TestOutputServers_JSONFormat(t *testing.T) { os.Stdout = w defer func() { os.Stdout = oldStdout }() - upstreamOutputFormat = "json" + // Use global output format for JSON + globalOutputFormat = "json" + globalJSONOutput = false err := outputServers(servers) w.Close() @@ -125,7 +129,9 @@ func TestOutputServers_InvalidFormat(t *testing.T) { {"name": "test"}, } - upstreamOutputFormat = "invalid-format" + // Use global output format for invalid format test + globalOutputFormat = "invalid-format" + globalJSONOutput = false err := outputServers(servers) if err == nil { @@ -149,7 +155,9 @@ func TestOutputServers_Sorting(t *testing.T) { os.Stdout = w defer func() { os.Stdout = oldStdout }() - upstreamOutputFormat = "json" + // Use global output format for JSON + globalOutputFormat = "json" + globalJSONOutput = false err := outputServers(servers) w.Close() @@ -191,7 +199,9 @@ func TestOutputServers_EmptyList(t *testing.T) { os.Stdout = w defer func() { os.Stdout = oldStdout }() - upstreamOutputFormat = "table" + // Use global output format (default is "table") + globalOutputFormat = "table" + globalJSONOutput = false err := outputServers(servers) w.Close() @@ -203,9 +213,9 @@ func TestOutputServers_EmptyList(t *testing.T) { t.Errorf("outputServers() returned error: %v", err) } - // Should still show headers - if !strings.Contains(output, "NAME") { - t.Error("Empty table should still show headers") + // With new unified formatter, empty tables show "No results found" message + if !strings.Contains(output, "No results found") { + t.Error("Empty table should show 'No results found' message") } } @@ -392,7 +402,9 @@ func TestOutputServers_BooleanFields(t *testing.T) { os.Stdout = w defer func() { os.Stdout = oldStdout }() - upstreamOutputFormat = "table" + // Use global output format for table + globalOutputFormat = "table" + globalJSONOutput = false err := outputServers(servers) w.Close() @@ -438,7 +450,9 @@ func TestOutputServers_IntegerFields(t *testing.T) { os.Stdout = w defer func() { os.Stdout = oldStdout }() - upstreamOutputFormat = "table" + // Use global output format (default is "table") + globalOutputFormat = "table" + globalJSONOutput = false err := outputServers(servers) w.Close() @@ -485,7 +499,9 @@ func TestOutputServers_StatusMessages(t *testing.T) { os.Stdout = w defer func() { os.Stdout = oldStdout }() - upstreamOutputFormat = "table" + // Use global output format (default is "table") + globalOutputFormat = "table" + globalJSONOutput = false err := outputServers(servers) w.Close() diff --git a/docs/cli-output-formatting.md b/docs/cli-output-formatting.md new file mode 100644 index 00000000..90dd9313 --- /dev/null +++ b/docs/cli-output-formatting.md @@ -0,0 +1,208 @@ +# CLI Output Formatting + +MCPProxy CLI supports multiple output formats for machine-readable and human-readable output. + +## Output Format Options + +### Global Flags + +All commands support the following output format options: + +| Flag | Description | +|------|-------------| +| `-o, --output ` | Output format: `table` (default), `json`, `yaml` | +| `--json` | Shorthand for `-o json` | + +### Environment Variable + +You can set the default output format using the `MCPPROXY_OUTPUT` environment variable: + +```bash +export MCPPROXY_OUTPUT=json +mcpproxy upstream list # Outputs JSON +``` + +### Format Precedence + +1. Command-line flag (`-o`/`--json`) - highest priority +2. Environment variable (`MCPPROXY_OUTPUT`) +3. Default (`table`) + +## Supported Formats + +### Table Format (Default) + +Human-readable tabular output with aligned columns. + +```bash +mcpproxy upstream list +``` + +``` + NAME PROTOCOL TOOLS STATUS ACTION +βœ… github-server http 15 Connected +⏸️ ast-grep stdio 0 Disabled Enable +``` + +Features: +- Aligned columns using tabwriter +- Unicode status indicators +- Respects `NO_COLOR=1` environment variable +- Simplified output for non-TTY contexts + +### JSON Format + +Machine-readable JSON output for scripting and AI agents. + +```bash +mcpproxy upstream list -o json +``` + +```json +[ + { + "name": "github-server", + "enabled": true, + "protocol": "http", + "connected": true, + "tool_count": 15, + "health": { + "level": "healthy", + "admin_state": "enabled", + "summary": "Connected" + } + } +] +``` + +Features: +- Pretty-printed with 2-space indentation +- Empty arrays output as `[]` (not `null`) +- Snake_case field names +- Structured error output with recovery hints + +### YAML Format + +Human-readable structured output for configuration scenarios. + +```bash +mcpproxy upstream list -o yaml +``` + +```yaml +- name: github-server + enabled: true + protocol: http + connected: true + tool_count: 15 + health: + level: healthy + admin_state: enabled + summary: Connected +``` + +## Machine-Readable Help + +The `--help-json` flag outputs structured help information for command discovery: + +```bash +mcpproxy --help-json +``` + +```json +{ + "name": "mcpproxy", + "description": "Smart MCP Proxy - Intelligent tool discovery and proxying", + "usage": "mcpproxy [flags]", + "flags": [ + { + "name": "output", + "shorthand": "o", + "description": "Output format: table, json, yaml", + "type": "string", + "default": "" + } + ], + "commands": [ + { + "name": "upstream", + "description": "Manage upstream MCP servers", + "usage": "mcpproxy upstream [flags]", + "has_subcommands": true + } + ] +} +``` + +This enables AI agents to discover available commands and their options programmatically. + +## Structured Error Output + +When using JSON output format, errors include structured information: + +```json +{ + "code": "ERR_SERVER_NOT_FOUND", + "message": "Server 'foo' not found", + "guidance": "Check the server name and try again", + "recovery_command": "mcpproxy upstream list", + "context": { + "server_name": "foo" + } +} +``` + +Error fields: +- `code`: Machine-readable error code +- `message`: Human-readable error message +- `guidance`: Suggested next steps +- `recovery_command`: Command to help resolve the issue +- `context`: Additional context about the error + +## Command Examples + +### List servers in different formats + +```bash +# Table (default) +mcpproxy upstream list + +# JSON for scripting +mcpproxy upstream list -o json + +# YAML for readability +mcpproxy upstream list -o yaml + +# Using --json alias +mcpproxy upstream list --json +``` + +### Parse JSON output with jq + +```bash +# Get connected server names +mcpproxy upstream list -o json | jq -r '.[] | select(.connected) | .name' + +# Count total tools +mcpproxy upstream list -o json | jq '[.[].tool_count] | add' +``` + +### Discover commands for AI agents + +```bash +# Get all available commands +mcpproxy --help-json | jq '.commands[].name' + +# Get flags for a specific command +mcpproxy upstream list --help-json | jq '.flags' +``` + +## NO_COLOR Support + +The CLI respects the `NO_COLOR` environment variable to disable colored output: + +```bash +NO_COLOR=1 mcpproxy upstream list +``` + +This affects table format output only (JSON and YAML are always plain text). diff --git a/docs/proposals/001-cli-architecture.md b/docs/proposals/001-cli-architecture.md index c44f4ad1..2742ccb6 100644 --- a/docs/proposals/001-cli-architecture.md +++ b/docs/proposals/001-cli-architecture.md @@ -67,7 +67,7 @@ mcpproxy upstream add --help-json | Aspect | Human Mode (default) | Agent Mode (flags/env) | |--------|---------------------|------------------------| -| Output | Formatted tables | `--json` | +| Output | Formatted tables | `-o json` or `--json` | | Help | Prose descriptions | `--help-json` | | Prompts | Interactive | `--yes` / `MCPPROXY_NO_PROMPT=1` | | Colors | Yes | `--no-color` / detect TTY | @@ -78,7 +78,7 @@ mcpproxy upstream add --help-json ## Complete Command Tree (v1.0) -> **Note on common flags**: Most commands support `--json` for machine-readable output. These are omitted from the tree below for brevity. See [Global Flags](#global-flags-all-commands) section. +> **Note on common flags**: Most commands support `-o/--output` for format selection (table, json, yaml, csv). The `--json` flag is a convenient alias for `-o json`. These are omitted from the tree below for brevity. See [Global Flags](#global-flags-all-commands) section. ``` mcpproxy @@ -102,16 +102,21 @@ mcpproxy β”‚ β”‚ β”œβ”€β”€ --jq # JQ filter expression β”‚ β”‚ └── --names-only # Just server names (newline-delimited) β”‚ β”‚ -β”‚ β”œβ”€β”€ add # Add server -β”‚ β”‚ β”œβ”€β”€ --name # Server name (required) -β”‚ β”‚ β”œβ”€β”€ --url # Server URL (for http protocol) -β”‚ β”‚ β”œβ”€β”€ --command # Command (for stdio protocol) -β”‚ β”‚ β”œβ”€β”€ --args # Command arguments (comma-separated) -β”‚ β”‚ β”œβ”€β”€ --protocol # Protocol: http, stdio (default: http) -β”‚ β”‚ β”œβ”€β”€ --working-dir # Working directory for stdio servers +β”‚ β”œβ”€β”€ add # Add server (Claude-style notation) +β”‚ β”‚ β”‚ # HTTP: mcpproxy upstream add +β”‚ β”‚ β”‚ # Stdio: mcpproxy upstream add -- [args...] +β”‚ β”‚ β”œβ”€β”€ --transport # Transport: http, stdio (default: infer from args) +β”‚ β”‚ β”œβ”€β”€ --env KEY=value # Environment variable (repeatable) +β”‚ β”‚ β”œβ”€β”€ --header "Name: value" # HTTP header (repeatable, http only) +β”‚ β”‚ β”œβ”€β”€ --working-dir # Working directory (stdio only) +β”‚ β”‚ β”œβ”€β”€ --scope # [FUTURE] Scope: global, project (default: global) β”‚ β”‚ └── --if-not-exists # No error if server exists β”‚ β”‚ +β”‚ β”œβ”€β”€ add-json '' # Add server from JSON config +β”‚ β”‚ └── --scope # [FUTURE] Scope: global, project +β”‚ β”‚ β”‚ β”œβ”€β”€ remove # Remove server +β”‚ β”‚ β”œβ”€β”€ --scope # [FUTURE] Scope: global, project β”‚ β”‚ β”œβ”€β”€ --if-exists # No error if server missing β”‚ β”‚ └── --yes # Skip confirmation β”‚ β”‚ @@ -266,7 +271,8 @@ mcpproxy | Flag | Short | Description | Environment Variable | |------|-------|-------------|---------------------| -| `--json` | | JSON output | `MCPPROXY_JSON=1` | +| `--output ` | `-o` | Output format: table, json, yaml, csv | `MCPPROXY_OUTPUT` | +| `--json` | | Shorthand for `-o json` | `MCPPROXY_JSON=1` | | `--help-json` | | Machine-readable help | | | `--jq ` | | Filter JSON output | | | `--quiet` | `-q` | Minimal output | `MCPPROXY_QUIET=1` | @@ -276,6 +282,35 @@ mcpproxy | `--project ` | `-p` | Project context *(WIP)* | `MCPPROXY_PROJECT` | | `--profile ` | | Use specific profile *(WIP)* | `MCPPROXY_PROFILE` | +### Output Format Design Rationale + +The `-o/--output` flag follows the **kubectl/AWS CLI pattern** for extensibility, while `--json` provides a convenient alias following the **gh CLI pattern**: + +| Pattern | Tools Using It | Pros | Cons | +|---------|---------------|------|------| +| `-o ` | kubectl, AWS, Azure, gcloud, Helm | Extensible, one flag for all formats | Slightly more typing | +| `--json` | gh, Terraform | Ergonomic for common case | Poor extensibility (need `--yaml`, `--csv`, etc.) | + +**Hybrid approach**: Use `-o` as primary for extensibility, `--json` as alias for ergonomics. + +```go +// Implementation pattern +cmd.Flags().StringVarP(&outputFormat, "output", "o", "table", + "Output format: table, json, yaml, csv") +cmd.Flags().BoolVar(&jsonOutput, "json", false, + "Shorthand for -o json") +cmd.MarkFlagsMutuallyExclusive("output", "json") +``` + +This allows future format additions without new flags: +```bash +mcpproxy activity list -o json # Primary pattern +mcpproxy activity list --json # Convenience alias +mcpproxy activity list -o csv # Export format +mcpproxy activity list -o jsonl # Streaming format +mcpproxy activity list -o yaml # Config-friendly format +``` + --- ## Environment Variables @@ -325,7 +360,7 @@ MCPPROXY_LOG_LEVEL=debug # Log level ## Structured Error Output -When `--json` is used, errors include recovery hints: +When `-o json` or `--json` is used, errors include recovery hints: ```json { @@ -346,13 +381,133 @@ When `--json` is used, errors include recovery hints: --- +## Server Management (Claude-Style Notation) + +The `upstream add` command follows the [Claude Code MCP CLI](https://docs.anthropic.com/en/docs/claude-code/mcp) notation for consistency across the MCP ecosystem. + +### Adding HTTP/SSE Servers + +```bash +# Basic HTTP server +mcpproxy upstream add notion https://mcp.notion.com/sse + +# With custom headers (repeatable) +mcpproxy upstream add github https://api.github.com/mcp \ + --header "Authorization: Bearer $GITHUB_TOKEN" \ + --header "X-Custom-Header: value" + +# Explicit transport type +mcpproxy upstream add --transport http myapi https://api.example.com/mcp +``` + +### Adding Stdio Servers + +The `--` separator divides mcpproxy flags from the server command: + +```bash +# Basic stdio server +mcpproxy upstream add filesystem -- npx -y @anthropic/mcp-server-filesystem + +# With environment variables (repeatable) +mcpproxy upstream add github-mcp \ + --env GITHUB_TOKEN=ghp_xxx \ + --env GITHUB_ORG=myorg \ + -- npx -y @anthropic/mcp-server-github + +# With working directory +mcpproxy upstream add project-tools \ + --working-dir /path/to/project \ + -- node ./tools/mcp-server.js + +# Complex command with arguments +mcpproxy upstream add postgres \ + --env DATABASE_URL=postgres://user:pass@localhost/db \ + -- npx -y @anthropic/mcp-server-postgres --readonly +``` + +### Adding from JSON + +For complex configurations, use `add-json`: + +```bash +# HTTP server with full config +mcpproxy upstream add-json weather-api '{ + "url": "https://api.weather.com/mcp", + "protocol": "http", + "headers": { + "Authorization": "Bearer token", + "X-API-Version": "2" + } +}' + +# Stdio server with full config +mcpproxy upstream add-json db-tools '{ + "command": "npx", + "args": ["-y", "@anthropic/mcp-server-postgres"], + "protocol": "stdio", + "env": { + "DATABASE_URL": "postgres://localhost/mydb" + }, + "working_dir": "/app" +}' +``` + +### Project-Scoped Servers (Future) + +> **Note**: The `--scope` flag is planned for future implementation. Currently all servers are added to the global config. + +Use `--scope project` to add servers to `.mcpproxy/config.json` in the current project: + +```bash +# Add to project config (committed to git, shared with team) +mcpproxy upstream add project-db \ + --scope project \ + --env DATABASE_URL=\${DATABASE_URL} \ + -- npx -y @anthropic/mcp-server-postgres + +# Add to global config (default, user-specific) +mcpproxy upstream add personal-api \ + --scope global \ + -- node ~/tools/my-mcp-server.js +``` + +### Removing Servers + +```bash +# Remove from global config (default) +mcpproxy upstream remove github + +# Remove from project config +mcpproxy upstream remove project-db --scope project + +# Idempotent remove (no error if missing) +mcpproxy upstream remove old-server --if-exists + +# Skip confirmation prompt +mcpproxy upstream remove github --yes +``` + +### Command Notation Summary + +| Pattern | Example | +|---------|---------| +| HTTP server | `mcpproxy upstream add ` | +| Stdio server | `mcpproxy upstream add -- [args...]` | +| With env vars | `--env KEY=value` (repeatable) | +| With headers | `--header "Name: value"` (repeatable) | +| Working dir | `--working-dir /path` | +| Project scope | `--scope project` *(future)* | +| From JSON | `mcpproxy upstream add-json ''` | + +--- + ## Idempotent Operations Commands should be safe to run multiple times: ```bash -mcpproxy upstream add github --if-not-exists # No error if exists -mcpproxy upstream remove github --if-exists # No error if missing +mcpproxy upstream add github https://api.github.com/mcp --if-not-exists +mcpproxy upstream remove github --if-exists mcpproxy upstream enable github # No-op if already enabled ``` @@ -361,14 +516,18 @@ mcpproxy upstream enable github # No-op if already enabled ## Output Formatting Options ```bash -# JSON output (machine-readable) -mcpproxy upstream list --json +# JSON output (machine-readable) - two equivalent ways +mcpproxy upstream list -o json # Primary (kubectl/AWS style) +mcpproxy upstream list --json # Alias (gh style) -# JSON with field selection (reduces tokens for agents) -mcpproxy upstream list --json name,health.level,health.action +# Other formats (extensible) +mcpproxy upstream list -o table # Default human-readable +mcpproxy upstream list -o yaml # Config-friendly +mcpproxy activity export -o csv # Spreadsheet export +mcpproxy activity list -o jsonl # Streaming/line-delimited JSON # JQ filtering (built-in, no external jq needed) -mcpproxy upstream list --jq '.[] | select(.health.level == "unhealthy")' +mcpproxy upstream list -o json --jq '.[] | select(.health.level == "unhealthy")' # Names only (minimal output) mcpproxy upstream list --names-only @@ -438,8 +597,8 @@ rootCmd.AddCommand(&cobra.Command{ | Priority | Features | |----------|----------| -| **P0** | Universal `--json`, `--help-json`, non-interactive mode, completion, upstream/tools/call/code/auth, doctor | -| **P1** | `--jq` filtering, structured errors | +| **P0** | Universal `-o/--output` with `--json` alias, `--help-json`, non-interactive mode, completion, upstream/tools/call/code/auth, doctor | +| **P1** | `--jq` filtering, structured errors, yaml/csv output formats | | **P2** | config commands, run scripts, activity commands (RFC-003), integrate commands | | **P3** | watch commands, registry operations | | **WIP** | project management, profile management *(design TBD)* | @@ -460,8 +619,12 @@ rootCmd.AddCommand(&cobra.Command{ ## References +- [Claude Code MCP CLI](https://docs.anthropic.com/en/docs/claude-code/mcp) - Server management notation - [GitHub CLI Patterns](https://cli.github.com/manual/) - [Cobra CLI Framework](https://cobra.dev/) - [12 Factor CLI Apps](https://medium.com/@jdxcode/12-factor-cli-apps-dd3c227a0e46) +- [kubectl Output Formatting](https://kubernetes.io/docs/reference/kubectl/#output-options) +- [AWS CLI Output Format](https://docs.aws.amazon.com/cli/latest/userguide/cli-usage-output-format.html) +- [Command Line Interface Guidelines](https://clig.dev/) - Issue #55: Projects/Profiles Support - Issue #136: Code Execution with MCP diff --git a/internal/cli/output/config.go b/internal/cli/output/config.go new file mode 100644 index 00000000..2d4fc545 --- /dev/null +++ b/internal/cli/output/config.go @@ -0,0 +1,55 @@ +package output + +import "os" + +// FormatConfig holds configuration for output formatting behavior. +type FormatConfig struct { + // Format is the output format (table, json, yaml) + Format string + + // NoColor disables ANSI color codes + NoColor bool + + // Quiet suppresses non-essential output + Quiet bool + + // Pretty enables human-readable formatting (indentation, etc.) + Pretty bool +} + +// DefaultConfig returns the default format configuration. +func DefaultConfig() FormatConfig { + return FormatConfig{ + Format: "table", + NoColor: os.Getenv("NO_COLOR") == "1", + Quiet: false, + Pretty: true, + } +} + +// FromEnv creates config from environment variables. +func FromEnv() FormatConfig { + cfg := DefaultConfig() + if format := os.Getenv("MCPPROXY_OUTPUT"); format != "" { + cfg.Format = format + } + return cfg +} + +// WithFormat returns a copy with the specified format. +func (c FormatConfig) WithFormat(format string) FormatConfig { + c.Format = format + return c +} + +// WithNoColor returns a copy with NoColor set. +func (c FormatConfig) WithNoColor(noColor bool) FormatConfig { + c.NoColor = noColor + return c +} + +// WithQuiet returns a copy with Quiet set. +func (c FormatConfig) WithQuiet(quiet bool) FormatConfig { + c.Quiet = quiet + return c +} diff --git a/internal/cli/output/error.go b/internal/cli/output/error.go new file mode 100644 index 00000000..dd9acdee --- /dev/null +++ b/internal/cli/output/error.go @@ -0,0 +1,78 @@ +package output + +// StructuredError represents errors with machine-parseable metadata for AI agent recovery. +type StructuredError struct { + // Code is a machine-readable error identifier (e.g., "CONFIG_NOT_FOUND") + Code string `json:"code" yaml:"code"` + + // Message is a human-readable error description + Message string `json:"message" yaml:"message"` + + // Guidance provides context on why this error occurred + Guidance string `json:"guidance,omitempty" yaml:"guidance,omitempty"` + + // RecoveryCommand suggests a command to fix the issue + RecoveryCommand string `json:"recovery_command,omitempty" yaml:"recovery_command,omitempty"` + + // Context contains additional structured data about the error + Context map[string]interface{} `json:"context,omitempty" yaml:"context,omitempty"` +} + +// Error implements the error interface for StructuredError. +func (e StructuredError) Error() string { + return e.Message +} + +// Common error codes for CLI operations +const ( + ErrCodeConfigNotFound = "CONFIG_NOT_FOUND" + ErrCodeServerNotFound = "SERVER_NOT_FOUND" + ErrCodeDaemonNotRunning = "DAEMON_NOT_RUNNING" + ErrCodeInvalidOutputFormat = "INVALID_OUTPUT_FORMAT" + ErrCodeAuthRequired = "AUTH_REQUIRED" + ErrCodeConnectionFailed = "CONNECTION_FAILED" + ErrCodeTimeout = "TIMEOUT" + ErrCodePermissionDenied = "PERMISSION_DENIED" + ErrCodeInvalidInput = "INVALID_INPUT" + ErrCodeOperationFailed = "OPERATION_FAILED" +) + +// NewStructuredError creates a new StructuredError with the given code and message. +func NewStructuredError(code, message string) StructuredError { + return StructuredError{ + Code: code, + Message: message, + } +} + +// WithGuidance adds guidance to the error. +func (e StructuredError) WithGuidance(guidance string) StructuredError { + e.Guidance = guidance + return e +} + +// WithRecoveryCommand adds a recovery command suggestion. +func (e StructuredError) WithRecoveryCommand(cmd string) StructuredError { + e.RecoveryCommand = cmd + return e +} + +// WithContext adds context data to the error. +func (e StructuredError) WithContext(key string, value interface{}) StructuredError { + if e.Context == nil { + e.Context = make(map[string]interface{}) + } + e.Context[key] = value + return e +} + +// FromError converts a standard error to a StructuredError. +func FromError(err error, code string) StructuredError { + if se, ok := err.(StructuredError); ok { + return se + } + return StructuredError{ + Code: code, + Message: err.Error(), + } +} diff --git a/internal/cli/output/formatter.go b/internal/cli/output/formatter.go new file mode 100644 index 00000000..8ee46cc4 --- /dev/null +++ b/internal/cli/output/formatter.go @@ -0,0 +1,57 @@ +// Package output provides unified output formatting for CLI commands. +// It supports multiple output formats (table, JSON, YAML) and structured errors. +package output + +import ( + "fmt" + "os" + "strings" +) + +// OutputFormatter formats structured data for CLI output. +// Implementations are stateless and thread-safe. +type OutputFormatter interface { + // Format converts data to formatted string output. + // data should be a struct, slice, or map that can be marshaled. + Format(data interface{}) (string, error) + + // FormatError converts a structured error to formatted output. + FormatError(err StructuredError) (string, error) + + // FormatTable formats tabular data with headers. + // headers defines column names, rows contains data. + FormatTable(headers []string, rows [][]string) (string, error) +} + +// NewFormatter creates a formatter for the specified format. +// Supported formats: table, json, yaml (case-insensitive). +func NewFormatter(format string) (OutputFormatter, error) { + switch strings.ToLower(format) { + case "json": + return &JSONFormatter{Indent: true}, nil + case "yaml": + return &YAMLFormatter{}, nil + case "table", "": + return &TableFormatter{ + NoColor: os.Getenv("NO_COLOR") == "1", + Unicode: true, + }, nil + default: + return nil, fmt.Errorf("unknown output format: %s (valid: table, json, yaml)", format) + } +} + +// ResolveFormat determines the output format from flags and environment. +// Priority: explicit flag > --json alias > MCPPROXY_OUTPUT env var > default (table) +func ResolveFormat(outputFlag string, jsonFlag bool) string { + if jsonFlag { + return "json" + } + if outputFlag != "" { + return outputFlag + } + if envFormat := os.Getenv("MCPPROXY_OUTPUT"); envFormat != "" { + return envFormat + } + return "table" +} diff --git a/internal/cli/output/formatter_test.go b/internal/cli/output/formatter_test.go new file mode 100644 index 00000000..be49857b --- /dev/null +++ b/internal/cli/output/formatter_test.go @@ -0,0 +1,189 @@ +package output + +import ( + "os" + "testing" +) + +// T021: Verify --json alias works same as -o json +func TestResolveFormat_JSONFlag(t *testing.T) { + tests := []struct { + name string + outputFlag string + jsonFlag bool + want string + }{ + { + name: "json flag takes precedence", + outputFlag: "", + jsonFlag: true, + want: "json", + }, + { + name: "output flag works alone", + outputFlag: "yaml", + jsonFlag: false, + want: "yaml", + }, + { + name: "default is table", + outputFlag: "", + jsonFlag: false, + want: "table", + }, + { + name: "json flag overrides even when output is set", // mutual exclusivity is handled by Cobra + outputFlag: "table", + jsonFlag: true, + want: "json", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Clear env var to ensure test isolation + os.Unsetenv("MCPPROXY_OUTPUT") + + got := ResolveFormat(tt.outputFlag, tt.jsonFlag) + if got != tt.want { + t.Errorf("ResolveFormat(%q, %v) = %q, want %q", tt.outputFlag, tt.jsonFlag, got, tt.want) + } + }) + } +} + +// T022: Verify MCPPROXY_OUTPUT env var works +func TestResolveFormat_EnvVar(t *testing.T) { + tests := []struct { + name string + envValue string + outputFlag string + jsonFlag bool + want string + }{ + { + name: "env var used when no flags", + envValue: "json", + outputFlag: "", + jsonFlag: false, + want: "json", + }, + { + name: "env var yaml", + envValue: "yaml", + outputFlag: "", + jsonFlag: false, + want: "yaml", + }, + { + name: "output flag overrides env var", + envValue: "json", + outputFlag: "table", + jsonFlag: false, + want: "table", + }, + { + name: "json flag overrides env var", + envValue: "yaml", + outputFlag: "", + jsonFlag: true, + want: "json", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.envValue != "" { + os.Setenv("MCPPROXY_OUTPUT", tt.envValue) + defer os.Unsetenv("MCPPROXY_OUTPUT") + } else { + os.Unsetenv("MCPPROXY_OUTPUT") + } + + got := ResolveFormat(tt.outputFlag, tt.jsonFlag) + if got != tt.want { + t.Errorf("ResolveFormat(%q, %v) with MCPPROXY_OUTPUT=%q = %q, want %q", + tt.outputFlag, tt.jsonFlag, tt.envValue, got, tt.want) + } + }) + } +} + +func TestNewFormatter(t *testing.T) { + tests := []struct { + name string + format string + wantErr bool + }{ + {"json format", "json", false}, + {"JSON uppercase", "JSON", false}, + {"yaml format", "yaml", false}, + {"table format", "table", false}, + {"empty default", "", false}, + {"invalid format", "invalid", true}, + {"csv not supported", "csv", true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f, err := NewFormatter(tt.format) + if (err != nil) != tt.wantErr { + t.Errorf("NewFormatter(%q) error = %v, wantErr %v", tt.format, err, tt.wantErr) + return + } + if !tt.wantErr && f == nil { + t.Errorf("NewFormatter(%q) returned nil formatter", tt.format) + } + }) + } +} + +func TestNewFormatter_JSONType(t *testing.T) { + f, err := NewFormatter("json") + if err != nil { + t.Fatalf("NewFormatter(json) error = %v", err) + } + if _, ok := f.(*JSONFormatter); !ok { + t.Errorf("NewFormatter(json) returned %T, want *JSONFormatter", f) + } +} + +func TestNewFormatter_YAMLType(t *testing.T) { + f, err := NewFormatter("yaml") + if err != nil { + t.Fatalf("NewFormatter(yaml) error = %v", err) + } + if _, ok := f.(*YAMLFormatter); !ok { + t.Errorf("NewFormatter(yaml) returned %T, want *YAMLFormatter", f) + } +} + +func TestNewFormatter_TableType(t *testing.T) { + f, err := NewFormatter("table") + if err != nil { + t.Fatalf("NewFormatter(table) error = %v", err) + } + if _, ok := f.(*TableFormatter); !ok { + t.Errorf("NewFormatter(table) returned %T, want *TableFormatter", f) + } +} + +func TestNewFormatter_NoColorSupport(t *testing.T) { + // Test NO_COLOR env var is respected + os.Setenv("NO_COLOR", "1") + defer os.Unsetenv("NO_COLOR") + + f, err := NewFormatter("table") + if err != nil { + t.Fatalf("NewFormatter(table) error = %v", err) + } + + tableFormatter, ok := f.(*TableFormatter) + if !ok { + t.Fatalf("NewFormatter(table) returned %T, want *TableFormatter", f) + } + + if !tableFormatter.NoColor { + t.Error("Expected NoColor to be true when NO_COLOR=1") + } +} diff --git a/internal/cli/output/help.go b/internal/cli/output/help.go new file mode 100644 index 00000000..69ad8cf4 --- /dev/null +++ b/internal/cli/output/help.go @@ -0,0 +1,214 @@ +package output + +import ( + "encoding/json" + "fmt" + "os" + + "github.com/spf13/cobra" + "github.com/spf13/pflag" +) + +// HelpInfo contains structured help information for a command hierarchy. +type HelpInfo struct { + // Name is the command name + Name string `json:"name"` + + // Description is a short description of the command + Description string `json:"description"` + + // Usage shows how to use the command + Usage string `json:"usage"` + + // Flags lists all available flags for this command + Flags []FlagInfo `json:"flags,omitempty"` + + // Commands lists subcommands (for parent commands) + Commands []CommandInfo `json:"commands,omitempty"` +} + +// CommandInfo contains information about a subcommand. +type CommandInfo struct { + // Name is the subcommand name + Name string `json:"name"` + + // Description is a short description of the subcommand + Description string `json:"description"` + + // Usage shows how to use the subcommand + Usage string `json:"usage"` + + // HasSubcommands indicates if this command has further subcommands + HasSubcommands bool `json:"has_subcommands,omitempty"` +} + +// FlagInfo contains information about a command flag. +type FlagInfo struct { + // Name is the long flag name (e.g., "output") + Name string `json:"name"` + + // Shorthand is the short flag (e.g., "o") + Shorthand string `json:"shorthand,omitempty"` + + // Description describes what the flag does + Description string `json:"description"` + + // Type is the flag type (string, bool, int, etc.) + Type string `json:"type"` + + // Default is the default value + Default string `json:"default,omitempty"` + + // Required indicates if the flag is required + Required bool `json:"required,omitempty"` +} + +// ExtractHelpInfo builds a HelpInfo struct from a cobra.Command. +func ExtractHelpInfo(cmd *cobra.Command) HelpInfo { + info := HelpInfo{ + Name: cmd.Name(), + Description: cmd.Short, + Usage: cmd.UseLine(), + } + + // Extract flags + info.Flags = extractFlags(cmd) + + // Extract subcommands + for _, sub := range cmd.Commands() { + if sub.Hidden || !sub.IsAvailableCommand() { + continue + } + cmdInfo := CommandInfo{ + Name: sub.Name(), + Description: sub.Short, + Usage: sub.UseLine(), + HasSubcommands: len(sub.Commands()) > 0, + } + info.Commands = append(info.Commands, cmdInfo) + } + + return info +} + +// extractFlags extracts flag information from a command. +func extractFlags(cmd *cobra.Command) []FlagInfo { + var flags []FlagInfo + + // Add local flags + cmd.LocalFlags().VisitAll(func(f *pflag.Flag) { + if f.Hidden { + return + } + flags = append(flags, FlagInfo{ + Name: f.Name, + Shorthand: f.Shorthand, + Description: f.Usage, + Type: f.Value.Type(), + Default: f.DefValue, + }) + }) + + // Add inherited persistent flags + cmd.InheritedFlags().VisitAll(func(f *pflag.Flag) { + if f.Hidden { + return + } + flags = append(flags, FlagInfo{ + Name: f.Name, + Shorthand: f.Shorthand, + Description: f.Usage, + Type: f.Value.Type(), + Default: f.DefValue, + }) + }) + + return flags +} + +// AddHelpJSONFlag adds a --help-json flag to a command. +// When invoked, it outputs structured help information as JSON. +func AddHelpJSONFlag(cmd *cobra.Command) { + var helpJSON bool + cmd.PersistentFlags().BoolVar(&helpJSON, "help-json", false, "Output help information as JSON") + + // Store original PreRunE if any + originalPreRunE := cmd.PreRunE + + // Wrap PreRunE to check for --help-json + cmd.PreRunE = func(cmd *cobra.Command, args []string) error { + if helpJSON { + // Output help as JSON and exit + info := ExtractHelpInfo(cmd) + output, err := json.MarshalIndent(info, "", " ") + if err != nil { + return fmt.Errorf("failed to marshal help info: %w", err) + } + fmt.Println(string(output)) + os.Exit(0) + } + + if originalPreRunE != nil { + return originalPreRunE(cmd, args) + } + return nil + } +} + +// SetupHelpJSON sets up --help-json on a command tree. +// It adds the flag and hooks to check for it on all commands. +func SetupHelpJSON(rootCmd *cobra.Command) { + rootCmd.PersistentFlags().Bool("help-json", false, "Output help information as JSON") + + // Store original PersistentPreRunE if any + originalPersistentPreRunE := rootCmd.PersistentPreRunE + + // Wrap PersistentPreRunE to check for --help-json + rootCmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { + helpJSONFlag, _ := cmd.Flags().GetBool("help-json") + if helpJSONFlag { + info := ExtractHelpInfo(cmd) + output, err := json.MarshalIndent(info, "", " ") + if err != nil { + return fmt.Errorf("failed to marshal help info: %w", err) + } + fmt.Println(string(output)) + os.Exit(0) + } + + if originalPersistentPreRunE != nil { + return originalPersistentPreRunE(cmd, args) + } + return nil + } + + // Also add a hook for parent commands that don't have Run + // by wrapping the help template + addHelpJSONToTree(rootCmd) +} + +// addHelpJSONToTree recursively adds --help-json support to commands that don't have Run. +func addHelpJSONToTree(cmd *cobra.Command) { + // For commands without Run (like "upstream"), add a Run that checks for --help-json + if cmd.Run == nil && cmd.RunE == nil { + cmd.RunE = func(c *cobra.Command, args []string) error { + helpJSONFlag, _ := c.Flags().GetBool("help-json") + if helpJSONFlag { + info := ExtractHelpInfo(c) + output, err := json.MarshalIndent(info, "", " ") + if err != nil { + return fmt.Errorf("failed to marshal help info: %w", err) + } + fmt.Println(string(output)) + return nil + } + // Show normal help if not --help-json + return c.Help() + } + } + + // Recurse into subcommands + for _, sub := range cmd.Commands() { + addHelpJSONToTree(sub) + } +} diff --git a/internal/cli/output/help_test.go b/internal/cli/output/help_test.go new file mode 100644 index 00000000..a3cc5479 --- /dev/null +++ b/internal/cli/output/help_test.go @@ -0,0 +1,290 @@ +package output + +import ( + "testing" + + "github.com/spf13/cobra" +) + +// T037: Unit test for HelpInfo structure +func TestHelpInfo_Structure(t *testing.T) { + info := HelpInfo{ + Name: "test", + Description: "A test command", + Usage: "test [flags]", + Flags: []FlagInfo{ + { + Name: "output", + Shorthand: "o", + Description: "Output format", + Type: "string", + Default: "table", + }, + }, + Commands: []CommandInfo{ + { + Name: "sub", + Description: "A subcommand", + Usage: "test sub", + HasSubcommands: false, + }, + }, + } + + if info.Name != "test" { + t.Errorf("Expected name 'test', got: %s", info.Name) + } + if len(info.Flags) != 1 { + t.Errorf("Expected 1 flag, got: %d", len(info.Flags)) + } + if len(info.Commands) != 1 { + t.Errorf("Expected 1 command, got: %d", len(info.Commands)) + } +} + +// T038: Unit test for ExtractHelpInfo from Cobra command +func TestExtractHelpInfo(t *testing.T) { + // Create a test command hierarchy + rootCmd := &cobra.Command{ + Use: "myapp", + Short: "My test application", + } + + subCmd := &cobra.Command{ + Use: "list", + Short: "List items", + Run: func(cmd *cobra.Command, args []string) {}, // Need a Run function for command to be available + } + rootCmd.AddCommand(subCmd) + + // Add a flag to root command + rootCmd.Flags().StringP("output", "o", "table", "Output format") + + info := ExtractHelpInfo(rootCmd) + + if info.Name != "myapp" { + t.Errorf("Expected name 'myapp', got: %s", info.Name) + } + if info.Description != "My test application" { + t.Errorf("Expected description 'My test application', got: %s", info.Description) + } + + // Should have the subcommand + if len(info.Commands) != 1 { + t.Fatalf("Expected 1 subcommand, got: %d", len(info.Commands)) + } + if info.Commands[0].Name != "list" { + t.Errorf("Expected subcommand 'list', got: %s", info.Commands[0].Name) + } +} + +func TestExtractHelpInfo_WithSubcommands(t *testing.T) { + rootCmd := &cobra.Command{ + Use: "app", + Short: "Application", + } + + parentCmd := &cobra.Command{ + Use: "parent", + Short: "Parent command", + Run: func(cmd *cobra.Command, args []string) {}, + } + + childCmd := &cobra.Command{ + Use: "child", + Short: "Child command", + Run: func(cmd *cobra.Command, args []string) {}, + } + + parentCmd.AddCommand(childCmd) + rootCmd.AddCommand(parentCmd) + + info := ExtractHelpInfo(rootCmd) + + // Root should show parent command + if len(info.Commands) != 1 { + t.Fatalf("Expected 1 command, got: %d", len(info.Commands)) + } + + // Parent command should indicate it has subcommands + if !info.Commands[0].HasSubcommands { + t.Error("Expected HasSubcommands to be true for parent command") + } +} + +func TestExtractHelpInfo_HiddenCommands(t *testing.T) { + rootCmd := &cobra.Command{ + Use: "app", + Short: "Application", + } + + visibleCmd := &cobra.Command{ + Use: "visible", + Short: "Visible command", + Run: func(cmd *cobra.Command, args []string) {}, + } + + hiddenCmd := &cobra.Command{ + Use: "hidden", + Short: "Hidden command", + Hidden: true, + Run: func(cmd *cobra.Command, args []string) {}, + } + + rootCmd.AddCommand(visibleCmd) + rootCmd.AddCommand(hiddenCmd) + + info := ExtractHelpInfo(rootCmd) + + // Should only have the visible command + if len(info.Commands) != 1 { + t.Errorf("Expected 1 visible command, got: %d", len(info.Commands)) + } + if len(info.Commands) > 0 && info.Commands[0].Name != "visible" { + t.Errorf("Expected 'visible' command, got: %s", info.Commands[0].Name) + } +} + +// T039: Unit test for FlagInfo extraction +func TestExtractHelpInfo_Flags(t *testing.T) { + cmd := &cobra.Command{ + Use: "test", + Short: "Test command", + Run: func(cmd *cobra.Command, args []string) {}, + } + + cmd.Flags().StringP("output", "o", "table", "Output format: table, json, yaml") + cmd.Flags().BoolP("verbose", "v", false, "Enable verbose output") + cmd.Flags().Int("limit", 10, "Maximum number of results") + + info := ExtractHelpInfo(cmd) + + if len(info.Flags) != 3 { + t.Fatalf("Expected 3 flags, got: %d", len(info.Flags)) + } + + // Find the output flag + var outputFlag *FlagInfo + for i := range info.Flags { + if info.Flags[i].Name == "output" { + outputFlag = &info.Flags[i] + break + } + } + + if outputFlag == nil { + t.Fatal("Expected to find 'output' flag") + } + + if outputFlag.Shorthand != "o" { + t.Errorf("Expected shorthand 'o', got: %s", outputFlag.Shorthand) + } + if outputFlag.Type != "string" { + t.Errorf("Expected type 'string', got: %s", outputFlag.Type) + } + if outputFlag.Default != "table" { + t.Errorf("Expected default 'table', got: %s", outputFlag.Default) + } +} + +func TestExtractHelpInfo_HiddenFlags(t *testing.T) { + cmd := &cobra.Command{ + Use: "test", + Short: "Test command", + Run: func(cmd *cobra.Command, args []string) {}, + } + + cmd.Flags().String("visible", "", "Visible flag") + cmd.Flags().String("hidden", "", "Hidden flag") + cmd.Flags().MarkHidden("hidden") + + info := ExtractHelpInfo(cmd) + + // Should only have the visible flag + if len(info.Flags) != 1 { + t.Errorf("Expected 1 visible flag, got: %d", len(info.Flags)) + } + if len(info.Flags) > 0 && info.Flags[0].Name != "visible" { + t.Errorf("Expected 'visible' flag, got: %s", info.Flags[0].Name) + } +} + +func TestExtractHelpInfo_InheritedFlags(t *testing.T) { + rootCmd := &cobra.Command{ + Use: "app", + Short: "Application", + } + rootCmd.PersistentFlags().StringP("output", "o", "table", "Output format") + + subCmd := &cobra.Command{ + Use: "sub", + Short: "Subcommand", + Run: func(cmd *cobra.Command, args []string) {}, + } + subCmd.Flags().Bool("local", false, "Local flag") + + rootCmd.AddCommand(subCmd) + + info := ExtractHelpInfo(subCmd) + + // Should have both local and inherited flags + if len(info.Flags) < 2 { + t.Errorf("Expected at least 2 flags (local + inherited), got: %d", len(info.Flags)) + } + + // Find the inherited output flag + var hasOutputFlag bool + var hasLocalFlag bool + for _, f := range info.Flags { + if f.Name == "output" { + hasOutputFlag = true + } + if f.Name == "local" { + hasLocalFlag = true + } + } + + if !hasOutputFlag { + t.Error("Expected inherited 'output' flag to be present") + } + if !hasLocalFlag { + t.Error("Expected local 'local' flag to be present") + } +} + +func TestFlagInfo_Structure(t *testing.T) { + flag := FlagInfo{ + Name: "timeout", + Shorthand: "t", + Description: "Request timeout in seconds", + Type: "int", + Default: "30", + Required: false, + } + + if flag.Name != "timeout" { + t.Errorf("Expected name 'timeout', got: %s", flag.Name) + } + if flag.Shorthand != "t" { + t.Errorf("Expected shorthand 't', got: %s", flag.Shorthand) + } + if flag.Type != "int" { + t.Errorf("Expected type 'int', got: %s", flag.Type) + } +} + +func TestCommandInfo_Structure(t *testing.T) { + cmd := CommandInfo{ + Name: "serve", + Description: "Start the server", + Usage: "app serve [flags]", + HasSubcommands: false, + } + + if cmd.Name != "serve" { + t.Errorf("Expected name 'serve', got: %s", cmd.Name) + } + if cmd.HasSubcommands { + t.Error("Expected HasSubcommands to be false") + } +} diff --git a/internal/cli/output/json.go b/internal/cli/output/json.go new file mode 100644 index 00000000..0f4c3117 --- /dev/null +++ b/internal/cli/output/json.go @@ -0,0 +1,48 @@ +package output + +import ( + "encoding/json" +) + +// JSONFormatter formats output as JSON. +type JSONFormatter struct { + Indent bool // Whether to pretty-print with indentation +} + +// Format marshals data to JSON. +func (f *JSONFormatter) Format(data interface{}) (string, error) { + var output []byte + var err error + if f.Indent { + output, err = json.MarshalIndent(data, "", " ") + } else { + output, err = json.Marshal(data) + } + if err != nil { + return "", err + } + return string(output), nil +} + +// FormatError marshals a structured error to JSON. +func (f *JSONFormatter) FormatError(err StructuredError) (string, error) { + return f.Format(err) +} + +// FormatTable converts tabular data to a JSON array of objects. +func (f *JSONFormatter) FormatTable(headers []string, rows [][]string) (string, error) { + // Convert table to slice of maps + result := make([]map[string]string, 0, len(rows)) + for _, row := range rows { + obj := make(map[string]string) + for i, header := range headers { + if i < len(row) { + obj[header] = row[i] + } else { + obj[header] = "" + } + } + result = append(result, obj) + } + return f.Format(result) +} diff --git a/internal/cli/output/json_test.go b/internal/cli/output/json_test.go new file mode 100644 index 00000000..1494802e --- /dev/null +++ b/internal/cli/output/json_test.go @@ -0,0 +1,218 @@ +package output + +import ( + "encoding/json" + "testing" +) + +// T011: Unit test for JSONFormatter.Format() +func TestJSONFormatter_Format(t *testing.T) { + f := &JSONFormatter{Indent: true} + + tests := []struct { + name string + data interface{} + wantErr bool + }{ + { + name: "format slice of maps", + data: []map[string]interface{}{ + {"name": "server1", "status": "healthy"}, + {"name": "server2", "status": "unhealthy"}, + }, + wantErr: false, + }, + { + name: "format struct", + data: struct { + Name string `json:"name"` + Status string `json:"status"` + }{Name: "test", Status: "ok"}, + wantErr: false, + }, + { + name: "format nil", + data: nil, + wantErr: false, + }, + { + name: "format empty slice", + data: []string{}, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := f.Format(tt.data) + if (err != nil) != tt.wantErr { + t.Errorf("Format() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !tt.wantErr { + // Verify result is valid JSON + var parsed interface{} + if err := json.Unmarshal([]byte(result), &parsed); err != nil { + t.Errorf("Format() result is not valid JSON: %v", err) + } + } + }) + } +} + +// T011: Test indentation +func TestJSONFormatter_Format_Indentation(t *testing.T) { + data := map[string]string{"key": "value"} + + // With indentation + fIndent := &JSONFormatter{Indent: true} + resultIndent, err := fIndent.Format(data) + if err != nil { + t.Fatalf("Format() error = %v", err) + } + if resultIndent == `{"key":"value"}` { + t.Error("Expected indented output but got compact") + } + + // Without indentation + fCompact := &JSONFormatter{Indent: false} + resultCompact, err := fCompact.Format(data) + if err != nil { + t.Fatalf("Format() error = %v", err) + } + if resultCompact != `{"key":"value"}` { + t.Errorf("Expected compact output, got: %s", resultCompact) + } +} + +// T012: Unit test for JSONFormatter.FormatError() +func TestJSONFormatter_FormatError(t *testing.T) { + f := &JSONFormatter{Indent: true} + + err := StructuredError{ + Code: "TEST_ERROR", + Message: "Test error message", + Guidance: "Try doing X instead", + RecoveryCommand: "mcpproxy test", + Context: map[string]interface{}{"key": "value"}, + } + + result, formatErr := f.FormatError(err) + if formatErr != nil { + t.Fatalf("FormatError() error = %v", formatErr) + } + + // Verify result is valid JSON + var parsed map[string]interface{} + if jsonErr := json.Unmarshal([]byte(result), &parsed); jsonErr != nil { + t.Fatalf("FormatError() result is not valid JSON: %v", jsonErr) + } + + // Verify required fields + if parsed["code"] != "TEST_ERROR" { + t.Errorf("Expected code 'TEST_ERROR', got: %v", parsed["code"]) + } + if parsed["message"] != "Test error message" { + t.Errorf("Expected message 'Test error message', got: %v", parsed["message"]) + } + if parsed["guidance"] != "Try doing X instead" { + t.Errorf("Expected guidance 'Try doing X instead', got: %v", parsed["guidance"]) + } + if parsed["recovery_command"] != "mcpproxy test" { + t.Errorf("Expected recovery_command 'mcpproxy test', got: %v", parsed["recovery_command"]) + } +} + +// T013: Unit test for JSONFormatter.FormatTable() +func TestJSONFormatter_FormatTable(t *testing.T) { + f := &JSONFormatter{Indent: true} + + headers := []string{"name", "status", "health"} + rows := [][]string{ + {"server1", "enabled", "healthy"}, + {"server2", "disabled", "unhealthy"}, + } + + result, err := f.FormatTable(headers, rows) + if err != nil { + t.Fatalf("FormatTable() error = %v", err) + } + + // Verify result is valid JSON array + var parsed []map[string]string + if jsonErr := json.Unmarshal([]byte(result), &parsed); jsonErr != nil { + t.Fatalf("FormatTable() result is not valid JSON array: %v", jsonErr) + } + + // Verify correct number of rows + if len(parsed) != 2 { + t.Errorf("Expected 2 rows, got: %d", len(parsed)) + } + + // Verify first row + if parsed[0]["name"] != "server1" { + t.Errorf("Expected name 'server1', got: %v", parsed[0]["name"]) + } + if parsed[0]["status"] != "enabled" { + t.Errorf("Expected status 'enabled', got: %v", parsed[0]["status"]) + } +} + +// T014: Unit test for empty array output (not null) +func TestJSONFormatter_EmptyArray(t *testing.T) { + f := &JSONFormatter{Indent: true} + + // Empty slice should return [] not null + result, err := f.Format([]string{}) + if err != nil { + t.Fatalf("Format() error = %v", err) + } + + // Should be "[]" not "null" + if result != "[]" { + t.Errorf("Expected '[]' for empty slice, got: %s", result) + } + + // Empty table should return [] + resultTable, err := f.FormatTable([]string{"a", "b"}, [][]string{}) + if err != nil { + t.Fatalf("FormatTable() error = %v", err) + } + if resultTable != "[]" { + t.Errorf("Expected '[]' for empty table, got: %s", resultTable) + } +} + +// Test snake_case field names +func TestJSONFormatter_SnakeCase(t *testing.T) { + f := &JSONFormatter{Indent: false} + + err := StructuredError{ + Code: "TEST", + Message: "test", + RecoveryCommand: "cmd", + } + + result, formatErr := f.FormatError(err) + if formatErr != nil { + t.Fatalf("FormatError() error = %v", formatErr) + } + + // Verify snake_case is used (recovery_command not recoveryCommand) + if !contains(result, `"recovery_command"`) { + t.Errorf("Expected snake_case 'recovery_command' in output: %s", result) + } +} + +func contains(s, substr string) bool { + return len(s) >= len(substr) && (s == substr || len(s) > 0 && containsHelper(s, substr)) +} + +func containsHelper(s, substr string) bool { + for i := 0; i <= len(s)-len(substr); i++ { + if s[i:i+len(substr)] == substr { + return true + } + } + return false +} diff --git a/internal/cli/output/table.go b/internal/cli/output/table.go new file mode 100644 index 00000000..6518774b --- /dev/null +++ b/internal/cli/output/table.go @@ -0,0 +1,108 @@ +package output + +import ( + "bytes" + "fmt" + "os" + "strings" + "text/tabwriter" + + "golang.org/x/term" +) + +// TableFormatter formats output as a human-readable table. +type TableFormatter struct { + NoColor bool // Disable ANSI colors + Unicode bool // Use Unicode box-drawing characters + Condensed bool // Simplified output for non-TTY +} + +// Format renders data as a formatted table. +// data must be a slice of structs or maps. +func (f *TableFormatter) Format(data interface{}) (string, error) { + // For generic data, delegate to JSON and indicate table not available + // This is a placeholder - full implementation will use reflection + return fmt.Sprintf("%v", data), nil +} + +// FormatError renders an error in human-readable format. +func (f *TableFormatter) FormatError(err StructuredError) (string, error) { + var buf bytes.Buffer + + // Use simple format for non-TTY or condensed mode + if f.Condensed || !f.isTTY() { + buf.WriteString(fmt.Sprintf("Error: %s\n", err.Message)) + if err.Guidance != "" { + buf.WriteString(fmt.Sprintf(" Guidance: %s\n", err.Guidance)) + } + if err.RecoveryCommand != "" { + buf.WriteString(fmt.Sprintf(" Try: %s\n", err.RecoveryCommand)) + } + return buf.String(), nil + } + + // Rich format with unicode + buf.WriteString("━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━\n") + buf.WriteString(fmt.Sprintf("❌ Error [%s]\n", err.Code)) + buf.WriteString("━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━\n") + buf.WriteString(fmt.Sprintf("\n%s\n", err.Message)) + + if err.Guidance != "" { + buf.WriteString(fmt.Sprintf("\nπŸ’‘ %s\n", err.Guidance)) + } + + if err.RecoveryCommand != "" { + buf.WriteString(fmt.Sprintf("\nπŸ”§ Try: %s\n", err.RecoveryCommand)) + } + + buf.WriteString("\n━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━\n") + + return buf.String(), nil +} + +// FormatTable renders tabular data with headers and alignment. +func (f *TableFormatter) FormatTable(headers []string, rows [][]string) (string, error) { + if len(rows) == 0 { + return "No results found\n", nil + } + + var buf bytes.Buffer + w := tabwriter.NewWriter(&buf, 0, 0, 2, ' ', 0) + + // Write separator line if unicode enabled and TTY + if f.Unicode && f.isTTY() { + separator := strings.Repeat("━", 80) + fmt.Fprintln(w, separator) + } + + // Write headers + headerLine := strings.Join(headers, "\t") + fmt.Fprintln(w, headerLine) + + // Write header separator + if f.Unicode && f.isTTY() { + separators := make([]string, len(headers)) + for i := range separators { + separators[i] = strings.Repeat("─", len(headers[i])+2) + } + fmt.Fprintln(w, strings.Join(separators, "\t")) + } + + // Write rows + for _, row := range rows { + rowLine := strings.Join(row, "\t") + fmt.Fprintln(w, rowLine) + } + + // Flush tabwriter + if err := w.Flush(); err != nil { + return "", err + } + + return buf.String(), nil +} + +// isTTY checks if stdout is a terminal. +func (f *TableFormatter) isTTY() bool { + return term.IsTerminal(int(os.Stdout.Fd())) +} diff --git a/internal/cli/output/table_test.go b/internal/cli/output/table_test.go new file mode 100644 index 00000000..93b22443 --- /dev/null +++ b/internal/cli/output/table_test.go @@ -0,0 +1,283 @@ +package output + +import ( + "os" + "strings" + "testing" +) + +// T024: Unit test for TableFormatter.Format() +func TestTableFormatter_Format(t *testing.T) { + f := &TableFormatter{} + + tests := []struct { + name string + data interface{} + wantErr bool + }{ + { + name: "format string", + data: "hello", + wantErr: false, + }, + { + name: "format slice", + data: []string{"a", "b", "c"}, + wantErr: false, + }, + { + name: "format map", + data: map[string]string{"key": "value"}, + wantErr: false, + }, + { + name: "format nil", + data: nil, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := f.Format(tt.data) + if (err != nil) != tt.wantErr { + t.Errorf("Format() error = %v, wantErr %v", err, tt.wantErr) + } + if !tt.wantErr && result == "" && tt.data != nil { + t.Error("Format() returned empty string for non-nil data") + } + }) + } +} + +// T025: Unit test for TableFormatter.FormatTable() with column alignment +func TestTableFormatter_FormatTable(t *testing.T) { + // Disable TTY features for consistent testing + f := &TableFormatter{Unicode: false, Condensed: true} + + headers := []string{"NAME", "STATUS", "TOOLS"} + rows := [][]string{ + {"server1", "healthy", "5"}, + {"server2", "unhealthy", "10"}, + {"very-long-server-name", "degraded", "3"}, + } + + result, err := f.FormatTable(headers, rows) + if err != nil { + t.Fatalf("FormatTable() error = %v", err) + } + + // Verify headers are present + if !strings.Contains(result, "NAME") { + t.Error("Expected header 'NAME' in output") + } + if !strings.Contains(result, "STATUS") { + t.Error("Expected header 'STATUS' in output") + } + if !strings.Contains(result, "TOOLS") { + t.Error("Expected header 'TOOLS' in output") + } + + // Verify all rows are present + if !strings.Contains(result, "server1") { + t.Error("Expected 'server1' in output") + } + if !strings.Contains(result, "server2") { + t.Error("Expected 'server2' in output") + } + if !strings.Contains(result, "very-long-server-name") { + t.Error("Expected 'very-long-server-name' in output") + } + + // Verify values are present + if !strings.Contains(result, "healthy") { + t.Error("Expected 'healthy' in output") + } +} + +func TestTableFormatter_FormatTable_EmptyRows(t *testing.T) { + f := &TableFormatter{} + + headers := []string{"NAME", "STATUS"} + rows := [][]string{} + + result, err := f.FormatTable(headers, rows) + if err != nil { + t.Fatalf("FormatTable() error = %v", err) + } + + if !strings.Contains(result, "No results found") { + t.Errorf("Expected 'No results found' message, got: %s", result) + } +} + +func TestTableFormatter_FormatTable_UnevenRows(t *testing.T) { + f := &TableFormatter{Condensed: true} + + headers := []string{"A", "B", "C"} + rows := [][]string{ + {"1", "2", "3"}, + {"4", "5"}, // Missing last column + } + + result, err := f.FormatTable(headers, rows) + if err != nil { + t.Fatalf("FormatTable() error = %v", err) + } + + // Should handle gracefully + if !strings.Contains(result, "1") || !strings.Contains(result, "4") { + t.Error("Expected row data in output") + } +} + +// T026: Unit test for NO_COLOR=1 environment variable +func TestTableFormatter_NoColor(t *testing.T) { + tests := []struct { + name string + noColor bool + wantEmoji bool // When NoColor is false and TTY, emojis should appear + }{ + { + name: "NoColor false allows formatting", + noColor: false, + wantEmoji: true, + }, + { + name: "NoColor true suppresses formatting", + noColor: true, + wantEmoji: true, // Emojis are not colors, still appear + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := &TableFormatter{NoColor: tt.noColor} + + err := StructuredError{ + Code: "TEST", + Message: "Test error", + } + + result, fmtErr := f.FormatError(err) + if fmtErr != nil { + t.Fatalf("FormatError() error = %v", fmtErr) + } + + // Both should contain the error message + if !strings.Contains(result, "Test error") { + t.Error("Expected error message in output") + } + }) + } +} + +// T027: Unit test for non-TTY simplified output +func TestTableFormatter_Condensed(t *testing.T) { + // Condensed mode simulates non-TTY behavior + f := &TableFormatter{Condensed: true} + + err := StructuredError{ + Code: "TEST_ERROR", + Message: "Something went wrong", + Guidance: "Try again later", + RecoveryCommand: "mcpproxy fix", + } + + result, fmtErr := f.FormatError(err) + if fmtErr != nil { + t.Fatalf("FormatError() error = %v", fmtErr) + } + + // Condensed format should be simpler + if !strings.Contains(result, "Error: Something went wrong") { + t.Error("Expected simple error format in condensed mode") + } + if !strings.Contains(result, "Guidance: Try again later") { + t.Error("Expected guidance in condensed mode") + } + if !strings.Contains(result, "Try: mcpproxy fix") { + t.Error("Expected recovery command in condensed mode") + } + + // Should NOT have unicode separators in condensed mode + if strings.Contains(result, "━━━") { + t.Error("Condensed mode should not have unicode separators") + } +} + +func TestTableFormatter_FormatError_RichFormat(t *testing.T) { + // When not condensed and TTY (we can't test true TTY in unit tests, + // but we can test the non-condensed path when isTTY returns true) + f := &TableFormatter{Unicode: true, Condensed: false} + + err := StructuredError{ + Code: "AUTH_REQUIRED", + Message: "Authentication required", + Guidance: "Please log in first", + RecoveryCommand: "mcpproxy auth login", + } + + result, fmtErr := f.FormatError(err) + if fmtErr != nil { + t.Fatalf("FormatError() error = %v", fmtErr) + } + + // In non-TTY (unit test), it will use condensed format + // This test verifies the condensed fallback works + if !strings.Contains(result, "Authentication required") { + t.Error("Expected error message in output") + } +} + +// Test that NewFormatter respects NO_COLOR env var +func TestNewFormatter_RespectsNO_COLOR(t *testing.T) { + // Set NO_COLOR + os.Setenv("NO_COLOR", "1") + defer os.Unsetenv("NO_COLOR") + + f, err := NewFormatter("table") + if err != nil { + t.Fatalf("NewFormatter() error = %v", err) + } + + tf, ok := f.(*TableFormatter) + if !ok { + t.Fatalf("Expected *TableFormatter, got %T", f) + } + + if !tf.NoColor { + t.Error("Expected NoColor to be true when NO_COLOR=1") + } +} + +// Test table alignment with varying column widths +func TestTableFormatter_ColumnAlignment(t *testing.T) { + f := &TableFormatter{Condensed: true} + + headers := []string{"ID", "NAME", "DESCRIPTION"} + rows := [][]string{ + {"1", "short", "A brief description"}, + {"123", "a-very-long-name", "Another description"}, + {"4", "x", "Y"}, + } + + result, err := f.FormatTable(headers, rows) + if err != nil { + t.Fatalf("FormatTable() error = %v", err) + } + + lines := strings.Split(strings.TrimSpace(result), "\n") + if len(lines) < 4 { // header + 3 rows + t.Errorf("Expected at least 4 lines, got %d", len(lines)) + } + + // All data should be present + for _, row := range rows { + for _, cell := range row { + if !strings.Contains(result, cell) { + t.Errorf("Expected '%s' in output", cell) + } + } + } +} diff --git a/internal/cli/output/yaml.go b/internal/cli/output/yaml.go new file mode 100644 index 00000000..1348ee9a --- /dev/null +++ b/internal/cli/output/yaml.go @@ -0,0 +1,40 @@ +package output + +import ( + "gopkg.in/yaml.v3" +) + +// YAMLFormatter formats output as YAML. +type YAMLFormatter struct{} + +// Format marshals data to YAML. +func (f *YAMLFormatter) Format(data interface{}) (string, error) { + output, err := yaml.Marshal(data) + if err != nil { + return "", err + } + return string(output), nil +} + +// FormatError marshals a structured error to YAML. +func (f *YAMLFormatter) FormatError(err StructuredError) (string, error) { + return f.Format(err) +} + +// FormatTable converts tabular data to YAML. +func (f *YAMLFormatter) FormatTable(headers []string, rows [][]string) (string, error) { + // Convert table to slice of maps + result := make([]map[string]string, 0, len(rows)) + for _, row := range rows { + obj := make(map[string]string) + for i, header := range headers { + if i < len(row) { + obj[header] = row[i] + } else { + obj[header] = "" + } + } + result = append(result, obj) + } + return f.Format(result) +} diff --git a/internal/cli/output/yaml_test.go b/internal/cli/output/yaml_test.go new file mode 100644 index 00000000..dadca7fb --- /dev/null +++ b/internal/cli/output/yaml_test.go @@ -0,0 +1,228 @@ +package output + +import ( + "strings" + "testing" + + "gopkg.in/yaml.v3" +) + +// T048: Unit test for YAMLFormatter.Format() +func TestYAMLFormatter_Format(t *testing.T) { + f := &YAMLFormatter{} + + tests := []struct { + name string + data interface{} + wantErr bool + }{ + { + name: "format slice of maps", + data: []map[string]interface{}{ + {"name": "server1", "status": "healthy"}, + {"name": "server2", "status": "unhealthy"}, + }, + wantErr: false, + }, + { + name: "format struct", + data: struct { + Name string `yaml:"name"` + Status string `yaml:"status"` + }{Name: "test", Status: "ok"}, + wantErr: false, + }, + { + name: "format nil", + data: nil, + wantErr: false, + }, + { + name: "format empty slice", + data: []string{}, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := f.Format(tt.data) + if (err != nil) != tt.wantErr { + t.Errorf("Format() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !tt.wantErr { + // Verify result is valid YAML + var parsed interface{} + if err := yaml.Unmarshal([]byte(result), &parsed); err != nil { + t.Errorf("Format() result is not valid YAML: %v", err) + } + } + }) + } +} + +func TestYAMLFormatter_Format_FieldNames(t *testing.T) { + f := &YAMLFormatter{} + + data := struct { + ServerName string `yaml:"server_name"` + ToolCount int `yaml:"tool_count"` + IsConnected bool `yaml:"is_connected"` + ErrorMessage string `yaml:"error_message,omitempty"` + }{ + ServerName: "test-server", + ToolCount: 5, + IsConnected: true, + } + + result, err := f.Format(data) + if err != nil { + t.Fatalf("Format() error = %v", err) + } + + // Verify YAML field names are snake_case + if !strings.Contains(result, "server_name:") { + t.Error("Expected 'server_name:' in YAML output") + } + if !strings.Contains(result, "tool_count:") { + t.Error("Expected 'tool_count:' in YAML output") + } + if !strings.Contains(result, "is_connected:") { + t.Error("Expected 'is_connected:' in YAML output") + } +} + +// T049: Unit test for YAMLFormatter.FormatError() +func TestYAMLFormatter_FormatError(t *testing.T) { + f := &YAMLFormatter{} + + err := StructuredError{ + Code: "TEST_ERROR", + Message: "Test error message", + Guidance: "Try doing X instead", + RecoveryCommand: "mcpproxy test", + Context: map[string]interface{}{"key": "value"}, + } + + result, formatErr := f.FormatError(err) + if formatErr != nil { + t.Fatalf("FormatError() error = %v", formatErr) + } + + // Verify result is valid YAML + var parsed map[string]interface{} + if yamlErr := yaml.Unmarshal([]byte(result), &parsed); yamlErr != nil { + t.Fatalf("FormatError() result is not valid YAML: %v", yamlErr) + } + + // Verify required fields are present + if parsed["code"] != "TEST_ERROR" { + t.Errorf("Expected code 'TEST_ERROR', got: %v", parsed["code"]) + } + if parsed["message"] != "Test error message" { + t.Errorf("Expected message 'Test error message', got: %v", parsed["message"]) + } +} + +func TestYAMLFormatter_FormatError_OmitEmpty(t *testing.T) { + f := &YAMLFormatter{} + + // Error without optional fields + err := StructuredError{ + Code: "SIMPLE_ERROR", + Message: "Simple error", + } + + result, formatErr := f.FormatError(err) + if formatErr != nil { + t.Fatalf("FormatError() error = %v", formatErr) + } + + // Optional fields should not appear when empty (due to omitempty) + if strings.Contains(result, "guidance:") { + t.Error("Empty guidance should be omitted") + } + if strings.Contains(result, "recovery_command:") { + t.Error("Empty recovery_command should be omitted") + } + if strings.Contains(result, "context:") { + t.Error("Empty context should be omitted") + } +} + +func TestYAMLFormatter_FormatTable(t *testing.T) { + f := &YAMLFormatter{} + + headers := []string{"name", "status", "health"} + rows := [][]string{ + {"server1", "enabled", "healthy"}, + {"server2", "disabled", "unhealthy"}, + } + + result, err := f.FormatTable(headers, rows) + if err != nil { + t.Fatalf("FormatTable() error = %v", err) + } + + // Verify result is valid YAML + var parsed []map[string]string + if yamlErr := yaml.Unmarshal([]byte(result), &parsed); yamlErr != nil { + t.Fatalf("FormatTable() result is not valid YAML: %v", yamlErr) + } + + // Verify correct number of rows + if len(parsed) != 2 { + t.Errorf("Expected 2 rows, got: %d", len(parsed)) + } + + // Verify first row + if parsed[0]["name"] != "server1" { + t.Errorf("Expected name 'server1', got: %v", parsed[0]["name"]) + } + if parsed[0]["status"] != "enabled" { + t.Errorf("Expected status 'enabled', got: %v", parsed[0]["status"]) + } +} + +func TestYAMLFormatter_FormatTable_Empty(t *testing.T) { + f := &YAMLFormatter{} + + headers := []string{"a", "b"} + rows := [][]string{} + + result, err := f.FormatTable(headers, rows) + if err != nil { + t.Fatalf("FormatTable() error = %v", err) + } + + // Empty table should produce empty YAML array + if result != "[]\n" { + t.Errorf("Expected empty YAML array '[]\\n', got: %q", result) + } +} + +func TestYAMLFormatter_FormatTable_MismatchedColumns(t *testing.T) { + f := &YAMLFormatter{} + + headers := []string{"a", "b", "c"} + rows := [][]string{ + {"1", "2"}, // Missing column c + } + + result, err := f.FormatTable(headers, rows) + if err != nil { + t.Fatalf("FormatTable() error = %v", err) + } + + // Should handle gracefully, filling missing columns with empty strings + var parsed []map[string]string + if yamlErr := yaml.Unmarshal([]byte(result), &parsed); yamlErr != nil { + t.Fatalf("FormatTable() result is not valid YAML: %v", yamlErr) + } + + // Column c should be empty + if parsed[0]["c"] != "" { + t.Errorf("Expected empty string for missing column 'c', got: %q", parsed[0]["c"]) + } +} diff --git a/specs/014-cli-output-formatting/checklists/requirements.md b/specs/014-cli-output-formatting/checklists/requirements.md new file mode 100644 index 00000000..a39fc88f --- /dev/null +++ b/specs/014-cli-output-formatting/checklists/requirements.md @@ -0,0 +1,36 @@ +# Specification Quality Checklist: CLI Output Formatting System + +**Purpose**: Validate specification completeness and quality before proceeding to planning +**Created**: 2025-12-26 +**Feature**: [spec.md](../spec.md) + +## Content Quality + +- [x] No implementation details (languages, frameworks, APIs) +- [x] Focused on user value and business needs +- [x] Written for non-technical stakeholders +- [x] All mandatory sections completed + +## Requirement Completeness + +- [x] No [NEEDS CLARIFICATION] markers remain +- [x] Requirements are testable and unambiguous +- [x] Success criteria are measurable +- [x] Success criteria are technology-agnostic (no implementation details) +- [x] All acceptance scenarios are defined +- [x] Edge cases are identified +- [x] Scope is clearly bounded +- [x] Dependencies and assumptions identified + +## Feature Readiness + +- [x] All functional requirements have clear acceptance criteria +- [x] User scenarios cover primary flows +- [x] Feature meets measurable outcomes defined in Success Criteria +- [x] No implementation details leak into specification + +## Notes + +- Spec is ready for `/speckit.plan` phase +- All checklist items pass validation +- No clarifications needed - requirements are clear from RFC-001 reference diff --git a/specs/014-cli-output-formatting/contracts/cli-output-schema.json b/specs/014-cli-output-formatting/contracts/cli-output-schema.json new file mode 100644 index 00000000..4d38808d --- /dev/null +++ b/specs/014-cli-output-formatting/contracts/cli-output-schema.json @@ -0,0 +1,192 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "MCPProxy CLI Output Schemas", + "description": "JSON schemas for mcpproxy CLI command outputs", + "definitions": { + "StructuredError": { + "type": "object", + "description": "Machine-readable error output when -o json is used", + "required": ["code", "message"], + "properties": { + "code": { + "type": "string", + "pattern": "^[A-Z][A-Z0-9_]*$", + "description": "Machine-readable error code", + "examples": ["CONFIG_NOT_FOUND", "SERVER_NOT_FOUND", "AUTH_REQUIRED"] + }, + "message": { + "type": "string", + "minLength": 1, + "description": "Human-readable error message" + }, + "guidance": { + "type": "string", + "description": "Additional context explaining why this error occurred" + }, + "recovery_command": { + "type": "string", + "description": "Suggested mcpproxy command to fix the issue" + }, + "context": { + "type": "object", + "description": "Additional structured data about the error", + "additionalProperties": true + } + }, + "additionalProperties": false + }, + "HelpInfo": { + "type": "object", + "description": "Machine-readable command help for --help-json", + "required": ["name", "description", "usage"], + "properties": { + "name": { + "type": "string", + "description": "Command name" + }, + "description": { + "type": "string", + "description": "Short description of the command" + }, + "usage": { + "type": "string", + "description": "Command syntax/usage pattern" + }, + "examples": { + "type": "array", + "items": {"type": "string"}, + "description": "Usage examples" + }, + "subcommands": { + "type": "array", + "items": {"$ref": "#/definitions/CommandInfo"}, + "description": "Child commands (for parent commands)" + }, + "flags": { + "type": "array", + "items": {"$ref": "#/definitions/FlagInfo"}, + "description": "Available flags" + } + }, + "additionalProperties": false + }, + "CommandInfo": { + "type": "object", + "required": ["name", "description"], + "properties": { + "name": {"type": "string"}, + "description": {"type": "string"} + }, + "additionalProperties": false + }, + "FlagInfo": { + "type": "object", + "required": ["name", "type", "description"], + "properties": { + "name": { + "type": "string", + "description": "Long flag name (e.g., 'output')" + }, + "shorthand": { + "type": "string", + "maxLength": 1, + "description": "Single-letter alias (e.g., 'o')" + }, + "type": { + "type": "string", + "enum": ["string", "bool", "int", "duration", "stringArray"], + "description": "Flag value type" + }, + "description": { + "type": "string", + "description": "What the flag does" + }, + "default": { + "type": "string", + "description": "Default value as string" + }, + "required": { + "type": "boolean", + "default": false, + "description": "Whether the flag is mandatory" + }, + "choices": { + "type": "array", + "items": {"type": "string"}, + "description": "Valid values for enum-like flags" + } + }, + "additionalProperties": false + }, + "ServerListOutput": { + "type": "array", + "description": "Output of 'mcpproxy upstream list -o json'", + "items": { + "type": "object", + "required": ["name", "protocol", "enabled"], + "properties": { + "name": {"type": "string"}, + "protocol": {"type": "string", "enum": ["http", "stdio"]}, + "enabled": {"type": "boolean"}, + "quarantined": {"type": "boolean"}, + "tool_count": {"type": "integer", "minimum": 0}, + "health": { + "type": "object", + "properties": { + "level": {"type": "string", "enum": ["healthy", "degraded", "unhealthy"]}, + "admin_state": {"type": "string", "enum": ["enabled", "disabled", "quarantined"]}, + "summary": {"type": "string"}, + "detail": {"type": "string"}, + "action": {"type": "string"} + } + } + } + } + }, + "ToolListOutput": { + "type": "array", + "description": "Output of 'mcpproxy tools list -o json'", + "items": { + "type": "object", + "required": ["name"], + "properties": { + "name": {"type": "string"}, + "description": {"type": "string"}, + "server": {"type": "string"}, + "input_schema": {"type": "object"} + } + } + }, + "DiagnosticsOutput": { + "type": "object", + "description": "Output of 'mcpproxy doctor -o json'", + "properties": { + "diagnostics": { + "type": "object", + "properties": { + "total_issues": {"type": "integer"}, + "upstream_errors": {"type": "array"}, + "oauth_required": {"type": "array", "items": {"type": "string"}}, + "oauth_issues": {"type": "array"}, + "missing_secrets": {"type": "array"}, + "runtime_warnings": {"type": "array"} + } + }, + "info": { + "type": "object", + "properties": { + "version": {"type": "string"}, + "update": { + "type": "object", + "properties": { + "available": {"type": "boolean"}, + "latest_version": {"type": "string"}, + "release_url": {"type": "string"} + } + } + } + } + } + } + } +} diff --git a/specs/014-cli-output-formatting/data-model.md b/specs/014-cli-output-formatting/data-model.md new file mode 100644 index 00000000..2b1b8bc2 --- /dev/null +++ b/specs/014-cli-output-formatting/data-model.md @@ -0,0 +1,247 @@ +# Data Model: CLI Output Formatting System + +**Feature**: 014-cli-output-formatting +**Date**: 2025-12-26 + +## Core Types + +### OutputFormatter Interface + +```go +// OutputFormatter formats structured data for CLI output. +// Implementations are stateless and thread-safe. +type OutputFormatter interface { + // Format converts data to formatted string output. + // data should be a struct, slice, or map that can be marshaled. + Format(data interface{}) (string, error) + + // FormatError converts a structured error to formatted output. + FormatError(err StructuredError) (string, error) + + // FormatTable formats tabular data with headers. + // headers defines column names, rows contains data. + FormatTable(headers []string, rows [][]string) (string, error) +} +``` + +### StructuredError + +Represents errors with machine-parseable metadata for AI agent recovery. + +```go +type StructuredError struct { + // Code is a machine-readable error identifier (e.g., "CONFIG_NOT_FOUND") + Code string `json:"code"` + + // Message is a human-readable error description + Message string `json:"message"` + + // Guidance provides context on why this error occurred + Guidance string `json:"guidance,omitempty"` + + // RecoveryCommand suggests a command to fix the issue + RecoveryCommand string `json:"recovery_command,omitempty"` + + // Context contains additional structured data about the error + Context map[string]interface{} `json:"context,omitempty"` +} +``` + +**Field Validation**: +- `Code`: Required, uppercase snake_case (e.g., `SERVER_NOT_FOUND`) +- `Message`: Required, non-empty string +- `Guidance`: Optional, explains the error context +- `RecoveryCommand`: Optional, valid mcpproxy command +- `Context`: Optional, arbitrary key-value pairs + +**Error Code Catalog**: + +| Code | Message | Guidance | Recovery Command | +|------|---------|----------|------------------| +| `CONFIG_NOT_FOUND` | Configuration file not found | Run daemon to create config | `mcpproxy serve` | +| `SERVER_NOT_FOUND` | Server '{name}' not found | Check available servers | `mcpproxy upstream list` | +| `DAEMON_NOT_RUNNING` | Daemon is not running | Start the daemon first | `mcpproxy serve` | +| `INVALID_OUTPUT_FORMAT` | Unknown format '{format}' | Use table, json, or yaml | - | +| `AUTH_REQUIRED` | OAuth authentication required | Authenticate with server | `mcpproxy auth login --server={name}` | +| `CONNECTION_FAILED` | Failed to connect to server | Check server config | `mcpproxy doctor` | + +### HelpInfo + +Machine-readable command help for `--help-json` output. + +```go +type HelpInfo struct { + // Name is the command name (e.g., "upstream") + Name string `json:"name"` + + // Description is a short description of the command + Description string `json:"description"` + + // Usage shows the command syntax + Usage string `json:"usage"` + + // Examples provides usage examples + Examples []string `json:"examples,omitempty"` + + // Subcommands lists child commands (for parent commands) + Subcommands []CommandInfo `json:"subcommands,omitempty"` + + // Flags lists available flags for this command + Flags []FlagInfo `json:"flags,omitempty"` +} + +type CommandInfo struct { + Name string `json:"name"` + Description string `json:"description"` +} + +type FlagInfo struct { + // Name is the long flag name (e.g., "output") + Name string `json:"name"` + + // Shorthand is the single-letter alias (e.g., "o") + Shorthand string `json:"shorthand,omitempty"` + + // Type is the flag value type (e.g., "string", "bool", "int") + Type string `json:"type"` + + // Description explains the flag's purpose + Description string `json:"description"` + + // Default is the default value as string + Default string `json:"default,omitempty"` + + // Required indicates if the flag is mandatory + Required bool `json:"required,omitempty"` + + // Choices lists valid values for enum-like flags + Choices []string `json:"choices,omitempty"` +} +``` + +### FormatConfig + +Configuration for output formatting behavior. + +```go +type FormatConfig struct { + // Format is the output format (table, json, yaml) + Format string + + // NoColor disables ANSI color codes + NoColor bool + + // Quiet suppresses non-essential output + Quiet bool + + // Pretty enables human-readable formatting (indentation, etc.) + Pretty bool +} + +// DefaultConfig returns the default format configuration +func DefaultConfig() FormatConfig { + return FormatConfig{ + Format: "table", + NoColor: os.Getenv("NO_COLOR") == "1", + Quiet: false, + Pretty: true, + } +} + +// FromEnv creates config from environment variables +func FromEnv() FormatConfig { + cfg := DefaultConfig() + if format := os.Getenv("MCPPROXY_OUTPUT"); format != "" { + cfg.Format = format + } + return cfg +} +``` + +## Formatter Implementations + +### JSONFormatter + +```go +type JSONFormatter struct { + Indent bool // Whether to pretty-print with indentation +} + +// Format marshals data to JSON +func (f *JSONFormatter) Format(data interface{}) (string, error) + +// FormatError marshals error to JSON +func (f *JSONFormatter) FormatError(err StructuredError) (string, error) + +// FormatTable converts table to JSON array of objects +func (f *JSONFormatter) FormatTable(headers []string, rows [][]string) (string, error) +``` + +### YAMLFormatter + +```go +type YAMLFormatter struct{} + +// Format marshals data to YAML +func (f *YAMLFormatter) Format(data interface{}) (string, error) + +// FormatError marshals error to YAML +func (f *YAMLFormatter) FormatError(err StructuredError) (string, error) + +// FormatTable converts table to YAML list +func (f *YAMLFormatter) FormatTable(headers []string, rows [][]string) (string, error) +``` + +### TableFormatter + +```go +type TableFormatter struct { + NoColor bool // Disable ANSI colors + Unicode bool // Use Unicode box-drawing characters + Condensed bool // Simplified output for non-TTY +} + +// Format renders data as formatted table +// data must be a slice of structs or maps +func (f *TableFormatter) Format(data interface{}) (string, error) + +// FormatError renders error in human-readable format +func (f *TableFormatter) FormatError(err StructuredError) (string, error) + +// FormatTable renders explicit headers and rows +func (f *TableFormatter) FormatTable(headers []string, rows [][]string) (string, error) +``` + +## State Transitions + +N/A - Formatters are stateless. Each call produces independent output. + +## Relationships + +``` +β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β” +β”‚ CLI Command β”‚ +β”‚ (upstream list)β”‚ +β””β”€β”€β”€β”€β”€β”€β”€β”€β”¬β”€β”€β”€β”€β”€β”€β”€β”€β”˜ + β”‚ uses + β–Ό +β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β” creates β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β” +β”‚ resolveFormat() β”‚ ───────────────► β”‚ OutputFormatter β”‚ +β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜ β”‚ (interface) β”‚ + β””β”€β”€β”€β”€β”€β”€β”€β”€β”¬β”€β”€β”€β”€β”€β”€β”€β”€β”˜ + β”‚ implemented by + β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”Όβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β” + β–Ό β–Ό β–Ό + β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β” β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β” β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β” + β”‚ JSONFormatter β”‚ β”‚ YAMLFormatter β”‚ β”‚ TableFormatter β”‚ + β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜ β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜ β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜ +``` + +## Validation Rules + +1. **Format string**: Must be one of `table`, `json`, `yaml` (case-insensitive) +2. **Error code**: Must be uppercase snake_case, max 50 characters +3. **JSON output**: Must be valid JSON, parseable by any JSON parser +4. **YAML output**: Must be valid YAML 1.2 +5. **Table output**: Headers must match row column count +6. **Empty data**: JSON returns `[]`, table shows "No results found" diff --git a/specs/014-cli-output-formatting/plan.md b/specs/014-cli-output-formatting/plan.md new file mode 100644 index 00000000..9b816424 --- /dev/null +++ b/specs/014-cli-output-formatting/plan.md @@ -0,0 +1,107 @@ +# Implementation Plan: CLI Output Formatting System + +**Branch**: `014-cli-output-formatting` | **Date**: 2025-12-26 | **Spec**: [spec.md](spec.md) +**Input**: Feature specification from `/specs/014-cli-output-formatting/spec.md` + +**Note**: This template is filled in by the `/speckit.plan` command. See `.specify/templates/commands/plan.md` for the execution workflow. + +## Summary + +Implement a unified output formatting system for mcpproxy CLI commands. The system provides: +1. Global `-o/--output` flag with table, json, yaml formats +2. `--json` convenience alias for `-o json` +3. `--help-json` for machine-readable command discovery +4. Structured error output with recovery hints +5. `MCPPROXY_OUTPUT` environment variable for default format + +Current state: Commands like `upstream list`, `tools list`, `doctor` already have ad-hoc `-o json` implementations with duplicated formatting logic. This spec consolidates them into a reusable `internal/cli/output/` package. + +## Technical Context + +**Language/Version**: Go 1.24 (toolchain go1.24.10) +**Primary Dependencies**: Cobra CLI framework, encoding/json, gopkg.in/yaml.v3 +**Storage**: N/A (CLI output only) +**Testing**: go test, ./scripts/test-api-e2e.sh, mcp-eval scenarios +**Target Platform**: macOS (darwin), Linux, Windows +**Project Type**: CLI application (single binary) +**Performance Goals**: --help-json returns in <100ms, formatting adds <10ms overhead +**Constraints**: No external dependencies beyond existing yaml.v3, maintain backward compatibility +**Scale/Scope**: ~15 CLI commands to update, 4 formatter implementations + +## Constitution Check + +*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.* + +| Principle | Status | Notes | +|-----------|--------|-------| +| I. Performance at Scale | βœ… PASS | Formatting is O(n) where n = output rows, negligible overhead | +| II. Actor-Based Concurrency | βœ… PASS | Formatters are stateless, no locks required | +| III. Configuration-Driven | βœ… PASS | MCPPROXY_OUTPUT env var for default format | +| IV. Security by Default | βœ… PASS | No security implications for output formatting | +| V. TDD | βœ… PASS | Unit tests for formatters, E2E tests for commands | +| VI. Documentation Hygiene | βœ… PASS | Update docs/ with CLI output guide | +| Separation of Concerns | βœ… PASS | `internal/cli/output/` is presentation layer | +| Event-Driven Updates | N/A | Output formatting is synchronous, not event-driven | +| DDD Layering | βœ… PASS | Output formatters are presentation layer | +| Upstream Client Modularity | N/A | Not applicable to CLI output | + +**Gate Result**: βœ… PASS - No violations, proceed to Phase 0 + +## Project Structure + +### Documentation (this feature) + +```text +specs/[###-feature]/ +β”œβ”€β”€ plan.md # This file (/speckit.plan command output) +β”œβ”€β”€ research.md # Phase 0 output (/speckit.plan command) +β”œβ”€β”€ data-model.md # Phase 1 output (/speckit.plan command) +β”œβ”€β”€ quickstart.md # Phase 1 output (/speckit.plan command) +β”œβ”€β”€ contracts/ # Phase 1 output (/speckit.plan command) +└── tasks.md # Phase 2 output (/speckit.tasks command - NOT created by /speckit.plan) +``` + +### Source Code (repository root) + +```text +internal/cli/output/ # NEW: Output formatting package +β”œβ”€β”€ formatter.go # OutputFormatter interface + factory +β”œβ”€β”€ table.go # TableFormatter implementation +β”œβ”€β”€ json.go # JSONFormatter implementation +β”œβ”€β”€ yaml.go # YAMLFormatter implementation +β”œβ”€β”€ error.go # StructuredError type + ErrorFormatter +β”œβ”€β”€ help.go # HelpInfo type + HelpFormatter for --help-json +└── formatter_test.go # Unit tests for all formatters + +cmd/mcpproxy/ # MODIFY: Update existing commands +β”œβ”€β”€ main.go # Add global --output flag, --json alias +β”œβ”€β”€ upstream_cmd.go # Migrate to use OutputFormatter +β”œβ”€β”€ tools_cmd.go # Migrate to use OutputFormatter +β”œβ”€β”€ doctor_cmd.go # Migrate to use OutputFormatter +β”œβ”€β”€ call_cmd.go # Migrate to use OutputFormatter +β”œβ”€β”€ auth_cmd.go # Migrate to use OutputFormatter +└── secrets_cmd.go # Migrate to use OutputFormatter + +docs/ # UPDATE: Documentation +└── cli-output-formatting.md # NEW: CLI output guide for agents +``` + +**Structure Decision**: Single project structure. New `internal/cli/output/` package follows existing pattern of `internal/` subdirectories. All formatting logic centralized there, commands delegate to formatters. + +## Complexity Tracking + +> No violations - all gates passed. No complexity justification needed. + +## Phase 0 Artifacts + +- [research.md](research.md) - Technical decisions and alternatives + +## Phase 1 Artifacts + +- [data-model.md](data-model.md) - Core types and interfaces +- [contracts/cli-output-schema.json](contracts/cli-output-schema.json) - JSON schemas for output +- [quickstart.md](quickstart.md) - Implementation guide + +## Next Steps + +Run `/speckit.tasks` to generate the implementation task list. diff --git a/specs/014-cli-output-formatting/quickstart.md b/specs/014-cli-output-formatting/quickstart.md new file mode 100644 index 00000000..ad39c819 --- /dev/null +++ b/specs/014-cli-output-formatting/quickstart.md @@ -0,0 +1,253 @@ +# Quickstart: CLI Output Formatting System + +**Feature**: 014-cli-output-formatting +**Date**: 2025-12-26 + +## Overview + +This guide covers implementing the CLI output formatting system for mcpproxy. The system provides unified output formatting across all CLI commands with support for table, JSON, and YAML formats. + +## Prerequisites + +- Go 1.24+ +- mcpproxy repository cloned +- `golangci-lint` installed + +## Quick Implementation Steps + +### Step 1: Create the Output Package + +```bash +mkdir -p internal/cli/output +``` + +Create the core formatter interface: + +```go +// internal/cli/output/formatter.go +package output + +import ( + "fmt" + "os" +) + +// OutputFormatter formats structured data for CLI output +type OutputFormatter interface { + Format(data interface{}) (string, error) + FormatError(err StructuredError) (string, error) + FormatTable(headers []string, rows [][]string) (string, error) +} + +// NewFormatter creates a formatter for the specified format +func NewFormatter(format string) (OutputFormatter, error) { + switch format { + case "json": + return &JSONFormatter{Indent: true}, nil + case "yaml": + return &YAMLFormatter{}, nil + case "table", "": + return &TableFormatter{ + NoColor: os.Getenv("NO_COLOR") == "1", + Unicode: true, + }, nil + default: + return nil, fmt.Errorf("unknown output format: %s (valid: table, json, yaml)", format) + } +} +``` + +### Step 2: Implement Formatters + +**JSON Formatter** (`internal/cli/output/json.go`): +```go +package output + +import "encoding/json" + +type JSONFormatter struct { + Indent bool +} + +func (f *JSONFormatter) Format(data interface{}) (string, error) { + var output []byte + var err error + if f.Indent { + output, err = json.MarshalIndent(data, "", " ") + } else { + output, err = json.Marshal(data) + } + if err != nil { + return "", err + } + return string(output), nil +} +``` + +### Step 3: Add Global Flags to Root Command + +In `cmd/mcpproxy/main.go`: + +```go +var ( + globalOutputFormat string + globalJSONOutput bool +) + +func init() { + rootCmd.PersistentFlags().StringVarP(&globalOutputFormat, "output", "o", "", + "Output format: table, json, yaml") + rootCmd.PersistentFlags().BoolVar(&globalJSONOutput, "json", false, + "Shorthand for -o json") + rootCmd.MarkFlagsMutuallyExclusive("output", "json") +} + +// ResolveOutputFormat determines the output format from flags and env +func ResolveOutputFormat() string { + if globalJSONOutput { + return "json" + } + if globalOutputFormat != "" { + return globalOutputFormat + } + if envFormat := os.Getenv("MCPPROXY_OUTPUT"); envFormat != "" { + return envFormat + } + return "table" +} +``` + +### Step 4: Migrate Existing Commands + +Replace ad-hoc formatting with formatter calls: + +```go +// Before +func outputServers(servers []map[string]interface{}) error { + switch upstreamOutputFormat { + case "json": + output, err := json.MarshalIndent(servers, "", " ") + // ... + case "table": + fmt.Printf("%-4s %-25s...", ...) + } +} + +// After +func outputServers(servers []map[string]interface{}) error { + formatter, err := output.NewFormatter(ResolveOutputFormat()) + if err != nil { + return err + } + result, err := formatter.Format(servers) + if err != nil { + return err + } + fmt.Print(result) + return nil +} +``` + +### Step 5: Add --help-json Support + +```go +// internal/cli/output/help.go +func AddHelpJSONFlag(cmd *cobra.Command) { + cmd.Flags().Bool("help-json", false, "Output help as JSON") + cmd.SetHelpFunc(createHelpFunc(cmd)) +} + +func createHelpFunc(original *cobra.Command) func(*cobra.Command, []string) { + return func(cmd *cobra.Command, args []string) { + helpJSON, _ := cmd.Flags().GetBool("help-json") + if helpJSON { + info := ExtractHelpInfo(cmd) + output, _ := json.MarshalIndent(info, "", " ") + fmt.Println(string(output)) + return + } + // Default help + cmd.Help() + } +} +``` + +## Testing + +### Unit Tests + +```bash +go test ./internal/cli/output/... -v +``` + +### E2E Tests + +```bash +# Test JSON output +./mcpproxy upstream list -o json | jq . + +# Test YAML output +./mcpproxy upstream list -o yaml + +# Test help-json +./mcpproxy --help-json | jq . +./mcpproxy upstream --help-json | jq . + +# Test env var +MCPPROXY_OUTPUT=json ./mcpproxy upstream list | jq . + +# Test error output +./mcpproxy upstream list --server nonexistent -o json +``` + +### mcp-eval Scenarios + +Update mcp-eval scenarios to use JSON output: +```yaml +- name: list-servers + command: mcpproxy upstream list -o json + validate: + type: json + schema: ServerListOutput +``` + +## Common Patterns + +### Handling Empty Results + +```go +if len(servers) == 0 { + if format == "json" { + fmt.Println("[]") + } else { + fmt.Println("No servers found") + } + return nil +} +``` + +### Error Formatting + +```go +func handleError(err error, format string) { + structErr := output.StructuredError{ + Code: "OPERATION_FAILED", + Message: err.Error(), + } + + formatter, _ := output.NewFormatter(format) + errOutput, _ := formatter.FormatError(structErr) + fmt.Fprintln(os.Stderr, errOutput) +} +``` + +## Verification Checklist + +- [ ] `mcpproxy upstream list -o json` returns valid JSON +- [ ] `mcpproxy upstream list --json` works (alias) +- [ ] `mcpproxy upstream list -o yaml` returns valid YAML +- [ ] `mcpproxy --help-json` returns command structure +- [ ] `MCPPROXY_OUTPUT=json mcpproxy upstream list` uses JSON +- [ ] Errors with `-o json` return structured error JSON +- [ ] `NO_COLOR=1` disables colors in table output +- [ ] All existing commands still work with default table format diff --git a/specs/014-cli-output-formatting/research.md b/specs/014-cli-output-formatting/research.md new file mode 100644 index 00000000..7fc76e48 --- /dev/null +++ b/specs/014-cli-output-formatting/research.md @@ -0,0 +1,217 @@ +# Research: CLI Output Formatting System + +**Feature**: 014-cli-output-formatting +**Date**: 2025-12-26 + +## Research Questions + +### 1. How should global flags be implemented with Cobra? + +**Decision**: Use `PersistentFlags()` on root command for global flags + +**Rationale**: Cobra's `PersistentFlags()` propagates flags to all subcommands automatically. This is the standard pattern used by kubectl, gh, and other Go CLIs. + +**Implementation**: +```go +// In main.go +rootCmd.PersistentFlags().StringVarP(&outputFormat, "output", "o", "", + "Output format: table, json, yaml") +rootCmd.PersistentFlags().BoolVar(&jsonOutput, "json", false, + "Shorthand for -o json") + +// Make mutually exclusive +rootCmd.MarkFlagsMutuallyExclusive("output", "json") +``` + +**Alternatives Considered**: +- Per-command flags: Rejected because it requires duplicating flag definitions in every command +- Viper config: Rejected as overkill for simple output format; env var support is sufficient + +### 2. How to handle `--json` as alias for `-o json`? + +**Decision**: Implement as separate boolean flag with mutual exclusivity + +**Rationale**: Cobra doesn't support true flag aliases. Using a boolean flag with mutual exclusivity achieves the same UX while being explicit about the behavior. + +**Implementation**: +```go +func resolveOutputFormat() string { + if jsonOutput { + return "json" + } + if outputFormat != "" { + return outputFormat + } + if envFormat := os.Getenv("MCPPROXY_OUTPUT"); envFormat != "" { + return envFormat + } + return "table" +} +``` + +**Alternatives Considered**: +- True alias: Not supported by Cobra +- Custom flag type: Overengineered for this use case + +### 3. What's the best pattern for OutputFormatter interface? + +**Decision**: Functional approach with stateless formatter instances + +**Rationale**: Formatters don't need state. A simple interface with `Format(data interface{}) (string, error)` allows for easy testing and composition. + +**Implementation**: +```go +// OutputFormatter formats structured data for CLI output +type OutputFormatter interface { + Format(data interface{}) (string, error) + FormatError(err StructuredError) (string, error) +} + +// Factory function +func NewFormatter(format string) (OutputFormatter, error) { + switch format { + case "json": + return &JSONFormatter{}, nil + case "yaml": + return &YAMLFormatter{}, nil + case "table": + return &TableFormatter{}, nil + default: + return nil, fmt.Errorf("unknown format: %s", format) + } +} +``` + +**Alternatives Considered**: +- Single function with switch: Less extensible, harder to test individual formatters +- Plugin architecture: Overkill, no need for dynamic formatter loading + +### 4. How to implement `--help-json`? + +**Decision**: Hook into Cobra's help system via `SetHelpFunc` + custom flag + +**Rationale**: Cobra allows overriding the help function. We can check for `--help-json` and return structured JSON instead of the default help text. + +**Implementation**: +```go +// Add flag to every command +cmd.Flags().Bool("help-json", false, "Output help as JSON for machine parsing") + +// Override help function +cmd.SetHelpFunc(func(cmd *cobra.Command, args []string) { + if helpJSON, _ := cmd.Flags().GetBool("help-json"); helpJSON { + outputHelpJSON(cmd) + return + } + // Default help behavior + cmd.Help() +}) +``` + +**Alternatives Considered**: +- Separate `mcpproxy help --json `: Less discoverable, requires remembering different syntax +- Global flag only: Would conflict with command-specific help + +### 5. What structure for StructuredError? + +**Decision**: Error struct with code, message, guidance, recovery_command, and context + +**Rationale**: Matches RFC-001 design. Provides enough information for AI agents to recover from errors automatically. + +**Implementation**: +```go +type StructuredError struct { + Code string `json:"code"` + Message string `json:"message"` + Guidance string `json:"guidance,omitempty"` + RecoveryCommand string `json:"recovery_command,omitempty"` + Context map[string]interface{} `json:"context,omitempty"` +} +``` + +**Alternatives Considered**: +- Simple error message: Insufficient for AI agent recovery +- Full stack trace: Too verbose, security concern with internal details + +### 6. How to handle table formatting with variable column widths? + +**Decision**: Use tabwriter from standard library with dynamic column detection + +**Rationale**: Go's `text/tabwriter` handles column alignment automatically. Current implementation uses fixed-width printf which doesn't adapt well. + +**Implementation**: +```go +type TableFormatter struct { + writer *tabwriter.Writer + buf *bytes.Buffer +} + +func (f *TableFormatter) Format(data interface{}) (string, error) { + f.buf = &bytes.Buffer{} + f.writer = tabwriter.NewWriter(f.buf, 0, 0, 2, ' ', 0) + + // Write headers and rows + // ... + + f.writer.Flush() + return f.buf.String(), nil +} +``` + +**Alternatives Considered**: +- Third-party table libraries (tablewriter): Extra dependency for simple use case +- Fixed-width columns: Current approach, doesn't adapt to content + +### 7. How to detect TTY for table simplification? + +**Decision**: Use golang.org/x/term package (already a dependency via Cobra) + +**Rationale**: `term.IsTerminal(int(os.Stdout.Fd()))` is the standard Go approach. Already used in `confirmation.go`. + +**Implementation**: +```go +func (f *TableFormatter) Format(data interface{}) (string, error) { + if !term.IsTerminal(int(os.Stdout.Fd())) { + // Simplified output without borders + return f.formatSimple(data) + } + return f.formatRich(data) +} +``` + +### 8. How to integrate formatters with existing commands? + +**Decision**: Gradual migration with backward compatibility + +**Rationale**: Existing commands work. Migrate one at a time, ensuring output format stays identical. + +**Migration Pattern**: +```go +// Before (in upstream_cmd.go) +func outputServers(servers []map[string]interface{}) error { + switch upstreamOutputFormat { + case "json": + output, err := json.MarshalIndent(servers, "", " ") + // ... + } +} + +// After +func outputServers(servers []map[string]interface{}) error { + formatter := output.NewFormatter(resolveOutputFormat()) + result, err := formatter.Format(servers) + if err != nil { + return err + } + fmt.Print(result) + return nil +} +``` + +## Summary + +All technical questions resolved. The implementation follows Go idioms: +- Stateless formatters implementing simple interface +- Standard library for JSON/YAML/table formatting +- Cobra's built-in mechanisms for flag handling +- Gradual migration preserving backward compatibility diff --git a/specs/014-cli-output-formatting/spec.md b/specs/014-cli-output-formatting/spec.md new file mode 100644 index 00000000..c0ece270 --- /dev/null +++ b/specs/014-cli-output-formatting/spec.md @@ -0,0 +1,202 @@ +# Feature Specification: CLI Output Formatting System + +**Feature Branch**: `014-cli-output-formatting` +**Created**: 2025-12-26 +**Status**: Draft +**Input**: User description: "Implement CLI Output Formatting system (RFC-001 foundation)" + +## User Scenarios & Testing *(mandatory)* + +### User Story 1 - Machine-Readable Output for AI Agents (Priority: P1) + +AI agents (Claude Code, Cursor, Goose) need to parse CLI output programmatically. When an agent runs `mcpproxy upstream list`, it needs structured JSON output instead of human-formatted tables to reliably extract server information for decision-making. + +**Why this priority**: This is the core value proposition of the feature. AI agents are primary users of mcpproxy CLI, and they cannot reliably parse table output. Without JSON output, agents resort to brittle regex parsing. + +**Independent Test**: Can be fully tested by running `mcpproxy upstream list -o json` and verifying the output parses as valid JSON with expected schema. + +**Acceptance Scenarios**: + +1. **Given** the daemon is running with 3 upstream servers, **When** an agent runs `mcpproxy upstream list -o json`, **Then** the output is valid JSON array with server objects containing name, status, and health fields +2. **Given** the daemon is running, **When** an agent runs `mcpproxy upstream list --json`, **Then** the output is identical to `-o json` (alias works) +3. **Given** MCPPROXY_OUTPUT=json is set, **When** an agent runs `mcpproxy upstream list`, **Then** JSON output is used by default without explicit flag +4. **Given** an agent runs any CLI command with `-o json`, **When** an error occurs, **Then** the error is returned as structured JSON with error code, message, and recovery_command fields + +--- + +### User Story 2 - Human-Readable Table Output (Priority: P2) + +Developers debugging mcpproxy need clean, formatted table output that's easy to read in a terminal. The default output should be a well-formatted table with aligned columns. + +**Why this priority**: Human developers need good UX when debugging, but they can fall back to web UI. AI agents have no alternative. + +**Independent Test**: Can be tested by running `mcpproxy upstream list` and verifying output displays as aligned table with headers. + +**Acceptance Scenarios**: + +1. **Given** the daemon is running, **When** a user runs `mcpproxy upstream list` without flags, **Then** output displays as a formatted table with headers (NAME, STATUS, HEALTH, etc.) +2. **Given** a table has columns of varying widths, **When** the table is rendered, **Then** columns are aligned and readable +3. **Given** NO_COLOR=1 is set, **When** table output is rendered, **Then** no ANSI color codes are included + +--- + +### User Story 3 - Hierarchical Command Discovery (Priority: P3) + +AI agents need to discover available commands and their options without loading full documentation. The `--help-json` flag provides machine-readable command metadata for progressive discovery. + +**Why this priority**: Reduces token usage for agents by allowing them to discover commands incrementally instead of loading full documentation. + +**Independent Test**: Can be tested by running `mcpproxy --help-json` and verifying structured command tree is returned. + +**Acceptance Scenarios**: + +1. **Given** an agent runs `mcpproxy --help-json`, **Then** output contains JSON with commands array listing all top-level commands with name and description +2. **Given** an agent runs `mcpproxy upstream --help-json`, **Then** output contains subcommands array for upstream command with their options +3. **Given** an agent runs `mcpproxy upstream add --help-json`, **Then** output contains flags array with flag name, type, description, and required status + +--- + +### User Story 4 - YAML Output for Configuration (Priority: P4) + +Some users prefer YAML format for configuration export/import scenarios. YAML is more readable than JSON for complex nested structures. + +**Why this priority**: Nice-to-have format option. JSON covers most use cases. + +**Independent Test**: Can be tested by running `mcpproxy upstream list -o yaml` and verifying valid YAML output. + +**Acceptance Scenarios**: + +1. **Given** the daemon is running, **When** a user runs `mcpproxy upstream list -o yaml`, **Then** output is valid YAML matching JSON structure + +--- + +### Edge Cases + +- What happens when `-o json` and `--json` are both provided? (Should be handled gracefully - use JSON, don't error) +- What happens with `-o invalid`? (Return error with list of valid formats) +- How does output work when stdout is not a TTY? (Auto-detect and simplify table formatting) +- What happens when output data is empty? (Return empty array `[]` for JSON, "No results" for table) + +## Requirements *(mandatory)* + +### Functional Requirements + +**Output Format Selection**: +- **FR-001**: System MUST support `-o/--output` flag with values: table, json, yaml +- **FR-002**: System MUST support `--json` as alias for `-o json` +- **FR-003**: System MUST respect `MCPPROXY_OUTPUT` environment variable as default format +- **FR-004**: Default output format MUST be "table" when no flag or environment variable is set +- **FR-005**: `-o/--output` and `--json` flags MUST be mutually exclusive (error if both specified with different values) + +**JSON Output**: +- **FR-006**: JSON output MUST be valid, parseable JSON +- **FR-007**: JSON output MUST use snake_case for all field names +- **FR-008**: JSON output MUST NOT include ANSI color codes or formatting characters +- **FR-009**: Empty results MUST return empty array `[]` not null + +**Table Output**: +- **FR-010**: Table output MUST include column headers +- **FR-011**: Table columns MUST be aligned for readability +- **FR-012**: Table output MUST respect `NO_COLOR=1` environment variable +- **FR-013**: Table output SHOULD auto-detect TTY and simplify formatting for non-TTY + +**Help JSON**: +- **FR-014**: System MUST support `--help-json` flag on all commands +- **FR-015**: Help JSON MUST include command name, description, subcommands, and flags +- **FR-016**: Flags in help JSON MUST include name, shorthand, type, description, default, and required status + +**Error Output**: +- **FR-017**: When `-o json` is used, errors MUST be returned as structured JSON +- **FR-018**: Error JSON MUST include fields: code, message, guidance, recovery_command +- **FR-019**: Error JSON SHOULD include context object with relevant state information + +**Output Formatter Abstraction**: +- **FR-020**: All CLI commands MUST use shared output formatter instead of direct printing +- **FR-021**: Output formatter MUST be injectable for testing +- **FR-022**: Output formatter MUST handle both success and error cases + +### Key Entities + +- **OutputFormatter**: Abstraction that formats data for output. Implementations include TableFormatter, JSONFormatter, YAMLFormatter. + +- **StructuredError**: Error representation with code, message, guidance, recovery_command, and context fields for machine-readable error handling. + +- **HelpInfo**: Machine-readable command help with name, description, subcommands, and flags arrays. + +## Success Criteria *(mandatory)* + +### Measurable Outcomes + +- **SC-001**: All CLI commands that produce output support `-o json` flag and produce valid JSON (verifiable by running each command and parsing output) +- **SC-002**: AI agents using mcp-eval scenarios achieve 95%+ trajectory similarity with JSON output vs baseline (verifiable by running mcp-eval test suite) +- **SC-003**: Table output renders correctly in terminals of 80+ character width (verifiable by manual testing) +- **SC-004**: `--help-json` returns valid JSON for all commands in under 100ms (verifiable by timing command execution) +- **SC-005**: Error output with `-o json` always returns parseable JSON with required fields (verifiable by triggering various error conditions) +- **SC-006**: Zero breaking changes to existing CLI output structure for JSON format (verifiable by comparing before/after output) + +## Assumptions + +- The current `-o json` implementation in some commands (like `upstream list`) can be extended rather than replaced +- Cobra framework's built-in help system can be extended to support `--help-json` +- Performance impact of formatter abstraction is negligible (simple delegation) +- YAML format uses standard Go yaml.v3 library encoding + +## Dependencies + +- **Existing Components**: + - `cmd/mcpproxy/*.go`: CLI commands to be updated + - Cobra CLI framework (already used) + +- **New Components**: + - `internal/cli/output/` package (to be created) + +- **External**: + - gopkg.in/yaml.v3 for YAML output + +## Out of Scope + +- CSV output format (planned for activity export in separate spec) +- JSONL streaming format (planned for activity watch in separate spec) +- Colorized JSON output (jq-style) +- Custom output templates +- Pager integration (less, more) + +## Commit Message Conventions *(mandatory)* + +When committing changes for this feature, follow these guidelines: + +### Issue References +- Use: `Related #[issue-number]` - Links the commit to the issue without auto-closing +- Do NOT use: `Fixes #[issue-number]`, `Closes #[issue-number]`, `Resolves #[issue-number]` - These auto-close issues on merge + +**Rationale**: Issues should only be closed manually after verification and testing in production, not automatically on merge. + +### Co-Authorship +- Do NOT include: `Co-Authored-By: Claude ` +- Do NOT include: "Generated with [Claude Code](https://claude.com/claude-code)" + +**Rationale**: Commit authorship should reflect the human contributors, not the AI tools used. + +### Example Commit Message +``` +feat(cli): add output formatting system with -o/--output flag + +Related #[issue-number] + +Implement unified output formatting for CLI commands following RFC-001. +Adds TableFormatter, JSONFormatter, and YAMLFormatter implementations. + +## Changes +- Add internal/cli/output package with OutputFormatter interface +- Implement TableFormatter with column alignment +- Implement JSONFormatter with snake_case fields +- Implement YAMLFormatter using yaml.v3 +- Add --help-json flag support to all commands +- Add structured error output for -o json +- Update upstream list command to use new formatter + +## Testing +- Unit tests for all formatters +- E2E tests for upstream list -o json +- mcp-eval scenarios updated for JSON output +``` diff --git a/specs/014-cli-output-formatting/tasks.md b/specs/014-cli-output-formatting/tasks.md new file mode 100644 index 00000000..5f79c012 --- /dev/null +++ b/specs/014-cli-output-formatting/tasks.md @@ -0,0 +1,276 @@ +# Tasks: CLI Output Formatting System + +**Input**: Design documents from `/specs/014-cli-output-formatting/` +**Prerequisites**: plan.md βœ…, spec.md βœ…, research.md βœ…, data-model.md βœ…, contracts/ βœ… + +**Tests**: Included as per constitution (TDD principle) and spec requirements. + +**Organization**: Tasks are grouped by user story to enable independent implementation and testing of each story. + +## Format: `[ID] [P?] [Story] Description` + +- **[P]**: Can run in parallel (different files, no dependencies) +- **[Story]**: Which user story this task belongs to (e.g., US1, US2, US3, US4) +- Include exact file paths in descriptions + +## Path Conventions + +Based on plan.md structure: +- **New package**: `internal/cli/output/` +- **CLI commands**: `cmd/mcpproxy/` +- **Documentation**: `docs/` + +--- + +## Phase 1: Setup (Shared Infrastructure) + +**Purpose**: Create output formatting package structure + +- [x] T001 Create output package directory structure at internal/cli/output/ +- [x] T002 Create OutputFormatter interface and factory in internal/cli/output/formatter.go +- [x] T003 Create StructuredError type in internal/cli/output/error.go +- [x] T004 Create FormatConfig with env var support in internal/cli/output/config.go +- [x] T005 Add global --output and --json flags to cmd/mcpproxy/main.go +- [x] T006 Add resolveOutputFormat() function to cmd/mcpproxy/main.go + +--- + +## Phase 2: Foundational (Blocking Prerequisites) + +**Purpose**: Core infrastructure that MUST be complete before user stories + +**⚠️ CRITICAL**: No user story work can begin until this phase is complete + +- [x] T007 Implement base Format() method signature in internal/cli/output/formatter.go +- [x] T008 Implement base FormatError() method signature in internal/cli/output/formatter.go +- [x] T009 Implement base FormatTable() method signature in internal/cli/output/formatter.go +- [x] T010 Create NewFormatter() factory function with format validation in internal/cli/output/formatter.go + +**Checkpoint**: Foundation ready - user story implementation can now begin + +--- + +## Phase 3: User Story 1 - Machine-Readable JSON Output (Priority: P1) 🎯 MVP + +**Goal**: AI agents can parse CLI output programmatically via `-o json` flag + +**Independent Test**: Run `mcpproxy upstream list -o json | jq .` and verify valid JSON output + +### Tests for User Story 1 + +- [x] T011 [P] [US1] Unit test for JSONFormatter.Format() in internal/cli/output/json_test.go +- [x] T012 [P] [US1] Unit test for JSONFormatter.FormatError() in internal/cli/output/json_test.go +- [x] T013 [P] [US1] Unit test for JSONFormatter.FormatTable() in internal/cli/output/json_test.go +- [x] T014 [P] [US1] Unit test for empty array output (not null) in internal/cli/output/json_test.go + +### Implementation for User Story 1 + +- [x] T015 [US1] Implement JSONFormatter struct in internal/cli/output/json.go +- [x] T016 [US1] Implement JSONFormatter.Format() with snake_case fields in internal/cli/output/json.go +- [x] T017 [US1] Implement JSONFormatter.FormatError() for structured errors in internal/cli/output/json.go +- [x] T018 [US1] Implement JSONFormatter.FormatTable() converting to JSON array in internal/cli/output/json.go +- [x] T019 [US1] Migrate upstream list command to use OutputFormatter in cmd/mcpproxy/upstream_cmd.go +- [x] T020 [US1] Add structured error output when -o json and error occurs in cmd/mcpproxy/upstream_cmd.go +- [x] T021 [US1] Verify --json alias works same as -o json in cmd/mcpproxy/main.go +- [x] T022 [US1] Verify MCPPROXY_OUTPUT=json env var works in cmd/mcpproxy/main.go +- [x] T023 [US1] E2E test: mcpproxy upstream list -o json produces valid JSON + +**Checkpoint**: JSON output works for upstream list command, testable independently βœ… + +--- + +## Phase 4: User Story 2 - Human-Readable Table Output (Priority: P2) + +**Goal**: Developers see clean, aligned table output by default + +**Independent Test**: Run `mcpproxy upstream list` and verify formatted table with headers + +### Tests for User Story 2 + +- [x] T024 [P] [US2] Unit test for TableFormatter.Format() in internal/cli/output/table_test.go +- [x] T025 [P] [US2] Unit test for TableFormatter.FormatTable() with column alignment in internal/cli/output/table_test.go +- [x] T026 [P] [US2] Unit test for NO_COLOR=1 environment variable in internal/cli/output/table_test.go +- [x] T027 [P] [US2] Unit test for non-TTY simplified output in internal/cli/output/table_test.go + +### Implementation for User Story 2 + +- [x] T028 [US2] Implement TableFormatter struct in internal/cli/output/table.go +- [x] T029 [US2] Implement TableFormatter.Format() using text/tabwriter in internal/cli/output/table.go +- [x] T030 [US2] Implement TableFormatter.FormatTable() with headers and alignment in internal/cli/output/table.go +- [x] T031 [US2] Implement TableFormatter.FormatError() for human-readable errors in internal/cli/output/table.go +- [x] T032 [US2] Add TTY detection for simplified non-TTY output in internal/cli/output/table.go +- [x] T033 [US2] Add NO_COLOR support to TableFormatter in internal/cli/output/table.go +- [x] T034 [US2] Migrate tools list command to use OutputFormatter in cmd/mcpproxy/tools_cmd.go +- [ ] T035 [US2] Migrate doctor command to use OutputFormatter in cmd/mcpproxy/doctor_cmd.go +- [x] T036 [US2] E2E test: mcpproxy upstream list displays aligned table + +**Checkpoint**: Table output works, both JSON and table formats functional βœ… + +--- + +## Phase 5: User Story 3 - Hierarchical Command Discovery (Priority: P3) + +**Goal**: AI agents discover commands via `--help-json` without loading full docs + +**Independent Test**: Run `mcpproxy --help-json | jq .` and verify command structure + +### Tests for User Story 3 + +- [x] T037 [P] [US3] Unit test for HelpInfo structure in internal/cli/output/help_test.go +- [x] T038 [P] [US3] Unit test for ExtractHelpInfo from Cobra command in internal/cli/output/help_test.go +- [x] T039 [P] [US3] Unit test for FlagInfo extraction in internal/cli/output/help_test.go + +### Implementation for User Story 3 + +- [x] T040 [US3] Create HelpInfo, CommandInfo, FlagInfo types in internal/cli/output/help.go +- [x] T041 [US3] Implement ExtractHelpInfo() to build HelpInfo from cobra.Command in internal/cli/output/help.go +- [x] T042 [US3] Implement AddHelpJSONFlag() to add --help-json to commands in internal/cli/output/help.go +- [x] T043 [US3] Implement custom help function with --help-json check in internal/cli/output/help.go +- [x] T044 [US3] Add --help-json flag to root command in cmd/mcpproxy/main.go +- [x] T045 [US3] Propagate --help-json to all subcommands in cmd/mcpproxy/main.go +- [x] T046 [US3] E2E test: mcpproxy --help-json returns valid JSON with commands array +- [x] T047 [US3] E2E test: mcpproxy upstream --help-json returns subcommands array + +**Checkpoint**: --help-json works on all commands, agents can discover CLI structure βœ… + +--- + +## Phase 6: User Story 4 - YAML Output (Priority: P4) + +**Goal**: Users can export data in YAML format for configuration scenarios + +**Independent Test**: Run `mcpproxy upstream list -o yaml` and verify valid YAML output + +### Tests for User Story 4 + +- [x] T048 [P] [US4] Unit test for YAMLFormatter.Format() in internal/cli/output/yaml_test.go +- [x] T049 [P] [US4] Unit test for YAMLFormatter.FormatError() in internal/cli/output/yaml_test.go + +### Implementation for User Story 4 + +- [x] T050 [US4] Implement YAMLFormatter struct in internal/cli/output/yaml.go +- [x] T051 [US4] Implement YAMLFormatter.Format() using yaml.v3 in internal/cli/output/yaml.go +- [x] T052 [US4] Implement YAMLFormatter.FormatError() in internal/cli/output/yaml.go +- [x] T053 [US4] Implement YAMLFormatter.FormatTable() in internal/cli/output/yaml.go +- [x] T054 [US4] E2E test: mcpproxy upstream list -o yaml produces valid YAML + +**Checkpoint**: All output formats (table, json, yaml) working βœ… + +--- + +## Phase 7: Command Migration + +**Purpose**: Migrate remaining commands to use unified formatters + +- [x] T055 [P] Migrate call command to use OutputFormatter in cmd/mcpproxy/call_cmd.go +- [x] T056 [P] Migrate auth command to use OutputFormatter in cmd/mcpproxy/auth_cmd.go (N/A - auth status uses custom format) +- [x] T057 [P] Migrate secrets command to use OutputFormatter in cmd/mcpproxy/secrets_cmd.go +- [x] T058 Remove legacy output formatting code from migrated commands +- [x] T059 Verify backward compatibility: existing -o json behavior unchanged + +--- + +## Phase 8: Polish & Cross-Cutting Concerns + +**Purpose**: Documentation, cleanup, and validation + +- [x] T060 [P] Create CLI output formatting documentation at docs/cli-output-formatting.md +- [x] T061 [P] Update CLAUDE.md with output formatting patterns +- [x] T062 [P] Add mcp-eval scenarios for JSON output validation (N/A - mcp-eval not in project) +- [x] T063 Run golangci-lint and fix any issues +- [x] T064 Run full test suite: ./scripts/run-all-tests.sh +- [x] T065 Run E2E API tests: ./scripts/test-api-e2e.sh +- [x] T066 Validate quickstart.md examples work correctly (N/A - no quickstart changes needed) + +--- + +## Dependencies & Execution Order + +### Phase Dependencies + +- **Setup (Phase 1)**: No dependencies - can start immediately +- **Foundational (Phase 2)**: Depends on Setup completion - BLOCKS all user stories +- **User Stories (Phase 3-6)**: All depend on Foundational phase completion + - US1 β†’ US2 β†’ US3 β†’ US4 (priority order) OR can run in parallel +- **Command Migration (Phase 7)**: Depends on US1 and US2 (need JSON + Table working) +- **Polish (Phase 8)**: Depends on all user stories being complete + +### User Story Dependencies + +- **User Story 1 (P1)**: Can start after Phase 2 - No dependencies on other stories +- **User Story 2 (P2)**: Can start after Phase 2 - Independent of US1 +- **User Story 3 (P3)**: Can start after Phase 2 - Independent of US1/US2 +- **User Story 4 (P4)**: Can start after Phase 2 - Independent of US1/US2/US3 + +### Within Each User Story + +- Tests written FIRST, verify they FAIL +- Core type/struct implementation +- Method implementations +- Command integration +- E2E verification + +### Parallel Opportunities + +- All test tasks within a story marked [P] can run in parallel +- Phase 7 command migrations marked [P] can run in parallel +- Phase 8 documentation tasks marked [P] can run in parallel + +--- + +## Parallel Example: User Story 1 Tests + +```bash +# Launch all tests for User Story 1 together: +go test -run TestJSONFormatter_Format ./internal/cli/output/ +go test -run TestJSONFormatter_FormatError ./internal/cli/output/ +go test -run TestJSONFormatter_FormatTable ./internal/cli/output/ +go test -run TestJSONFormatter_EmptyArray ./internal/cli/output/ +``` + +--- + +## Implementation Strategy + +### MVP First (User Story 1 Only) + +1. Complete Phase 1: Setup (T001-T006) +2. Complete Phase 2: Foundational (T007-T010) +3. Complete Phase 3: User Story 1 - JSON Output (T011-T023) +4. **STOP and VALIDATE**: `mcpproxy upstream list -o json | jq .` +5. Deploy/demo if ready - agents can now use JSON output + +### Incremental Delivery + +1. Setup + Foundational β†’ Foundation ready +2. Add User Story 1 (JSON) β†’ Test independently β†’ **MVP!** +3. Add User Story 2 (Table) β†’ Improves human UX +4. Add User Story 3 (--help-json) β†’ Agent discovery +5. Add User Story 4 (YAML) β†’ Nice-to-have format +6. Command Migration β†’ Full CLI coverage +7. Polish β†’ Documentation complete + +### Task Count Summary + +| Phase | Task Count | Parallel Tasks | +|-------|------------|----------------| +| Phase 1: Setup | 6 | 0 | +| Phase 2: Foundational | 4 | 0 | +| Phase 3: US1 JSON (P1) | 13 | 4 tests | +| Phase 4: US2 Table (P2) | 13 | 4 tests | +| Phase 5: US3 Help-JSON (P3) | 11 | 3 tests | +| Phase 6: US4 YAML (P4) | 7 | 2 tests | +| Phase 7: Migration | 5 | 3 commands | +| Phase 8: Polish | 7 | 3 docs | +| **Total** | **66** | **19** | + +--- + +## Notes + +- [P] tasks = different files, no dependencies +- [Story] label maps task to specific user story for traceability +- Each user story is independently completable and testable +- Constitution requires TDD: write tests first, verify they fail +- Commit after each task or logical group +- Stop at any checkpoint to validate story independently