Prevent MCP stdio startup diagnostics from polluting JSON-RPC stdout

pkg/cli/mcp_server_command.go

@@ -2,11 +2,9 @@ package cli
 
 import (
 	"context"
-	"fmt"
 	"os"
 	"strings"
 
-	"github.com/github/gh-aw/pkg/console"
 	"github.com/github/gh-aw/pkg/logger"
 	"github.com/github/gh-aw/pkg/workflow"
 	"github.com/modelcontextprotocol/go-sdk/mcp"
@@ -73,24 +71,20 @@ Examples:
 	return cmd
 }
 
-// checkAndLogGHVersion checks if gh CLI is available and logs its version
+// checkAndLogGHVersion checks if gh CLI is available and logs its version.
+// Diagnostics are emitted through the debug logger only.
 func checkAndLogGHVersion() {
 	cmd := workflow.ExecGH("version")
 	output, err := cmd.CombinedOutput()
 
 	if err != nil {
 		mcpLog.Print("WARNING: gh CLI not found in PATH")
-		fmt.Fprintln(os.Stderr, console.FormatWarningMessage("gh CLI not found in PATH - some MCP server operations may fail"))
 		return
 	}
 
 	// Parse and log the version
 	versionOutput := strings.TrimSpace(string(output))
 	mcpLog.Printf("gh CLI version: %s", versionOutput)
-
-	// Extract just the first line for cleaner logging to stderr
-	firstLine := strings.Split(versionOutput, "\n")[0]
-	fmt.Fprintln(os.Stderr, console.FormatInfoMessage("gh CLI: "+firstLine))
 }
 
 // runMCPServer starts the MCP server on stdio or HTTP transport
@@ -100,19 +94,12 @@ func runMCPServer(port int, cmdPath string, validateActor bool) error {
 
 	if validateActor {
 		mcpLog.Printf("Actor validation enabled (--validate-actor flag)")
-		fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Actor validation enabled"))
 	}
 
 	if actor != "" {
 		mcpLog.Printf("Using actor: %s", actor)
-		fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Actor: "+actor))
 	} else {
 		mcpLog.Print("No actor specified (GITHUB_ACTOR environment variable)")
-		if validateActor {
-			fmt.Fprintln(os.Stderr, console.FormatWarningMessage("No actor specified - logs and audit tools will not be mounted (actor validation enabled)"))
-		} else {
-			fmt.Fprintln(os.Stderr, console.FormatWarningMessage("No actor specified - all tools will be mounted (actor validation disabled)"))
-		}
 	}
 
 	if port > 0 {
@@ -136,10 +123,8 @@ func runMCPServer(port int, cmdPath string, validateActor bool) error {
 	// Log current working directory
 	if cwd, err := os.Getwd(); err == nil {
 		mcpLog.Printf("Current working directory: %s", cwd)
-		fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Current working directory: "+cwd))
 	} else {
 		mcpLog.Printf("WARNING: Failed to get current working directory: %v", err)
-		fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to get current working directory: %v", err)))
 	}
 
 	// Check and log gh CLI version
@@ -150,7 +135,6 @@ func runMCPServer(port int, cmdPath string, validateActor bool) error {
 	// This allows the server to start in test environments or non-repository directories
 	if err := validateMCPServerConfiguration(cmdPath); err != nil {
 		mcpLog.Printf("Configuration validation warning: %v", err)
-		fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Configuration validation warning: %v", err)))
 	}
 
 	// Pre-cache lock-file manifests at startup, before any agent can modify the working tree.
@@ -165,7 +149,6 @@ func runMCPServer(port int, cmdPath string, validateActor bool) error {
 		} else {
 			manifestCacheFile = cacheFile
 			mcpLog.Printf("Manifest cache written to %s (%d entries)", cacheFile, len(manifestCache))
-			fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Pre-cached %d workflow manifest(s) for safe update enforcement", len(manifestCache))))
 			// Clean up the temp file when the server exits
 			defer func() {
 				if removeErr := os.Remove(cacheFile); removeErr != nil && !os.IsNotExist(removeErr) {

pkg/cli/mcp_server_stdio_integration_test.go

@@ -0,0 +1,86 @@
+//go:build integration
+
+package cli
+
+import (
+	"bytes"
+	"context"
+	"errors"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/github/gh-aw/pkg/testutil"
+)
+
+func TestMCPServer_StdioDiagnosticsGoToStderr(t *testing.T) {
+	binaryPath := "../../gh-aw"
+	if _, err := os.Stat(binaryPath); os.IsNotExist(err) {
+		t.Skip("Skipping test: gh-aw binary not found. Run 'make build' first.")
+	}
+
+	tmpDir := testutil.TempDir(t, "mcp-stdio-*")
+	workflowsDir := filepath.Join(tmpDir, ".github", "workflows")
+	if err := os.MkdirAll(workflowsDir, 0755); err != nil {
+		t.Fatalf("Failed to create workflows directory: %v", err)
+	}
+
+	workflowPath := filepath.Join(workflowsDir, "test.md")
+	workflowContent := `---
+on: push
+engine: copilot
+---
+# Test Workflow
+`
+	if err := os.WriteFile(workflowPath, []byte(workflowContent), 0644); err != nil {
+		t.Fatalf("Failed to write workflow file: %v", err)
+	}
+
+	if err := initTestGitRepo(tmpDir); err != nil {
+		t.Fatalf("Failed to initialize git repository: %v", err)
+	}
+
+	originalDir, err := os.Getwd()
+	if err != nil {
+		t.Fatalf("Failed to get current working directory: %v", err)
+	}
+
+	absBinaryPath := filepath.Join(originalDir, binaryPath)
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+
+	cmd := exec.CommandContext(ctx, absBinaryPath, "mcp-server", "--cmd", absBinaryPath)
+	cmd.Dir = tmpDir
+	cmd.Stdin = strings.NewReader("")
+
+	env := make([]string, 0, len(os.Environ()))
+	for _, entry := range os.Environ() {
+		if strings.HasPrefix(entry, "GITHUB_ACTOR=") {
+			continue
+		}
+		env = append(env, entry)
+	}
+	cmd.Env = env
+
+	var stdout bytes.Buffer
+	var stderr bytes.Buffer
+	cmd.Stdout = &stdout
+	cmd.Stderr = &stderr
+
+	err = cmd.Run()
+	if err != nil && !errors.Is(ctx.Err(), context.DeadlineExceeded) {
+		var exitErr *exec.ExitError
+		if !errors.As(err, &exitErr) {
+			t.Fatalf("Failed to run MCP server process: %v", err)
+		}
+	}
+
+	stdoutText := strings.TrimSpace(stdout.String())
+	if stdoutText != "" {
+		t.Fatalf("Expected stdout to remain clean for JSON-RPC, got: %q", stdoutText)
+	}
+
+}

pkg/cli/mcp_validation.go

@@ -50,29 +50,26 @@ func GetBinaryPath() (string, error) {
 // logAndValidateBinaryPath determines the binary path, logs it, and validates it exists.
 // Returns the detected binary path and an error if the path cannot be determined or if the file doesn't exist.
 // This is a helper used by both runMCPServer and validateMCPServerConfiguration.
+// Diagnostics are emitted through the debug logger only.
 func logAndValidateBinaryPath() (string, error) {
 	binaryPath, err := GetBinaryPath()
 	if err != nil {
 		mcpValidationLog.Printf("Warning: failed to get binary path: %v", err)
-		fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Warning: failed to get binary path: %v", err)))
 		return "", err
 	}
 
 	// Check if the binary file exists
 	if _, err := os.Stat(binaryPath); err != nil {
 		if os.IsNotExist(err) {
 			mcpValidationLog.Printf("ERROR: binary file does not exist at path: %s", binaryPath)
-			fmt.Fprintln(os.Stderr, console.FormatErrorMessage("ERROR: binary file does not exist at path: "+binaryPath))
 			return "", fmt.Errorf("binary file does not exist at path: %s", binaryPath)
 		}
 		mcpValidationLog.Printf("Warning: failed to stat binary file at %s: %v", binaryPath, err)
-		fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Warning: failed to stat binary file at %s: %v", binaryPath, err)))
 		return "", err
 	}
 
 	// Log the binary path for debugging
 	mcpValidationLog.Printf("gh-aw binary path: %s", binaryPath)
-	fmt.Fprintln(os.Stderr, console.FormatInfoMessage("gh-aw binary path: "+binaryPath))
 	return binaryPath, nil
 }
 
@@ -222,7 +219,8 @@ func validateServerSecrets(config parser.MCPServerConfig, verbose bool, useActio
 }
 
 // validateMCPServerConfiguration validates that the CLI is properly configured
-// by running the status command as a test
+// by running the status command as a test.
+// Diagnostics are emitted through the debug logger only.
 func validateMCPServerConfiguration(cmdPath string) error {
 	mcpValidationLog.Printf("Validating MCP server configuration: cmdPath=%s", cmdPath)
 
@@ -257,27 +255,20 @@ func validateMCPServerConfiguration(cmdPath string) error {
 		// Check for common error cases
 		if ctx.Err() == context.DeadlineExceeded {
 			mcpValidationLog.Print("Status command timed out")
-			errMsg := "status command timed out - this may indicate a configuration issue"
-			fmt.Fprintln(os.Stderr, console.FormatErrorMessage(errMsg))
 			return errors.New("status command timed out - this may indicate a configuration issue")
 		}
 
 		mcpValidationLog.Printf("Status command failed: %v", err)
 
 		// If the command failed, provide helpful error message
 		if cmdPath != "" {
-			errMsg := fmt.Sprintf("failed to run status command with custom command '%s': %v\nOutput: %s\n\nPlease ensure:\n  - The command path is correct and executable\n  - You are in a git repository with .github/workflows directory", cmdPath, err, string(output))
-			fmt.Fprintln(os.Stderr, console.FormatErrorMessage(errMsg))
 			return fmt.Errorf("failed to run status command with custom command '%s': %w\nOutput: %s\n\nPlease ensure:\n  - The command path is correct and executable\n  - You are in a git repository with .github/workflows directory", cmdPath, err, string(output))
 		}
-		errMsg := fmt.Sprintf("failed to run status command: %v\nOutput: %s\n\nPlease ensure:\n  - gh CLI is installed and in PATH\n  - gh aw extension is installed (run: gh extension install github/gh-aw)\n  - You are in a git repository with .github/workflows directory", err, string(output))
-		fmt.Fprintln(os.Stderr, console.FormatErrorMessage(errMsg))
 		return fmt.Errorf("failed to run status command: %w\nOutput: %s\n\nPlease ensure:\n  - gh CLI is installed and in PATH\n  - gh aw extension is installed (run: gh extension install github/gh-aw)\n  - You are in a git repository with .github/workflows directory", err, string(output))
 	}
 
 	// Status command succeeded - configuration is valid
 	mcpValidationLog.Print("MCP server configuration validated successfully")
-	fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("✅ Configuration validated successfully"))
 	return nil
 }