Refactor: Consolidate files, eliminate duplicate init patterns with generics, standardize validation naming

internal/logger/global_helpers.go

@@ -1,6 +1,6 @@
 // Package logger provides structured logging for the MCP Gateway.
 //
-// This file contains helper functions for managing global logger state with proper
+// This file contains generic helper functions for managing global logger state with proper
 // mutex handling. These helpers encapsulate common patterns for initializing and
 // closing global loggers (FileLogger, JSONLLogger, MarkdownLogger) to reduce code
 // duplication while maintaining thread safety.
@@ -14,84 +14,102 @@
 // directly by external code. Use the public Init* and Close* functions instead.
 package logger
 
-// This file contains helper functions that encapsulate the common patterns
-// for global logger initialization and cleanup. These helpers reduce code
-// duplication while maintaining type safety.
+import "sync"
 
-// initGlobalFileLogger is a helper that encapsulates the common pattern for
-// initializing a global FileLogger with proper mutex handling.
-func initGlobalFileLogger(logger *FileLogger) {
-	globalLoggerMu.Lock()
-	defer globalLoggerMu.Unlock()
+// closableLogger is a constraint for types that have a Close method.
+// This is satisfied by *FileLogger, *JSONLLogger, and *MarkdownLogger.
+type closableLogger interface {
+	*FileLogger | *JSONLLogger | *MarkdownLogger
+	Close() error
+}
+
+// initGlobalLogger is a generic helper that encapsulates the common pattern for
+// initializing a global logger with proper mutex handling.
+//
+// Type parameters:
+//   - T: Any pointer type that satisfies closableLogger constraint
+//
+// Parameters:
+//   - mu: Mutex to protect access to the global logger
+//   - current: Pointer to the current global logger instance
+//   - newLogger: New logger instance to set as the global logger
+//
+// This function:
+//  1. Acquires the mutex lock
+//  2. Closes any existing logger if present
+//  3. Sets the new logger as the global instance
+//  4. Releases the mutex lock
+func initGlobalLogger[T closableLogger](mu *sync.RWMutex, current *T, newLogger T) {
+	mu.Lock()
+	defer mu.Unlock()
 
-	if globalFileLogger != nil {
-		globalFileLogger.Close()
+	if *current != nil {
+		(*current).Close()
 	}
-	globalFileLogger = logger
+	*current = newLogger
 }
 
-// closeGlobalFileLogger is a helper that encapsulates the common pattern for
-// closing and clearing a global FileLogger with proper mutex handling.
-func closeGlobalFileLogger() error {
-	globalLoggerMu.Lock()
-	defer globalLoggerMu.Unlock()
+// closeGlobalLogger is a generic helper that encapsulates the common pattern for
+// closing and clearing a global logger with proper mutex handling.
+//
+// Type parameters:
+//   - T: Any pointer type that satisfies closableLogger constraint
+//
+// Parameters:
+//   - mu: Mutex to protect access to the global logger
+//   - logger: Pointer to the global logger instance to close
+//
+// Returns:
+//   - error: Any error returned by the logger's Close() method
+//
+// This function:
+//  1. Acquires the mutex lock
+//  2. Closes the logger if it exists
+//  3. Sets the logger pointer to nil
+//  4. Releases the mutex lock
+//  5. Returns any error from the Close() operation
+func closeGlobalLogger[T closableLogger](mu *sync.RWMutex, logger *T) error {
+	mu.Lock()
+	defer mu.Unlock()
 
-	if globalFileLogger != nil {
-		err := globalFileLogger.Close()
-		globalFileLogger = nil
+	if *logger != nil {
+		err := (*logger).Close()
+		var zero T
+		*logger = zero
 		return err
 	}
 	return nil
 }
 
-// initGlobalJSONLLogger is a helper that encapsulates the common pattern for
-// initializing a global JSONLLogger with proper mutex handling.
-func initGlobalJSONLLogger(logger *JSONLLogger) {
-	globalJSONLMu.Lock()
-	defer globalJSONLMu.Unlock()
+// Type-specific helper functions that use the generic implementations above.
+// These maintain backward compatibility with existing code.
 
-	if globalJSONLLogger != nil {
-		globalJSONLLogger.Close()
-	}
-	globalJSONLLogger = logger
+// initGlobalFileLogger initializes the global FileLogger using the generic helper.
+func initGlobalFileLogger(logger *FileLogger) {
+	initGlobalLogger(&globalLoggerMu, &globalFileLogger, logger)
 }
 
-// closeGlobalJSONLLogger is a helper that encapsulates the common pattern for
-// closing and clearing a global JSONLLogger with proper mutex handling.
-func closeGlobalJSONLLogger() error {
-	globalJSONLMu.Lock()
-	defer globalJSONLMu.Unlock()
+// closeGlobalFileLogger closes the global FileLogger using the generic helper.
+func closeGlobalFileLogger() error {
+	return closeGlobalLogger(&globalLoggerMu, &globalFileLogger)
+}
 
-	if globalJSONLLogger != nil {
-		err := globalJSONLLogger.Close()
-		globalJSONLLogger = nil
-		return err
-	}
-	return nil
+// initGlobalJSONLLogger initializes the global JSONLLogger using the generic helper.
+func initGlobalJSONLLogger(logger *JSONLLogger) {
+	initGlobalLogger(&globalJSONLMu, &globalJSONLLogger, logger)
 }
 
-// initGlobalMarkdownLogger is a helper that encapsulates the common pattern for
-// initializing a global MarkdownLogger with proper mutex handling.
-func initGlobalMarkdownLogger(logger *MarkdownLogger) {
-	globalMarkdownMu.Lock()
-	defer globalMarkdownMu.Unlock()
+// closeGlobalJSONLLogger closes the global JSONLLogger using the generic helper.
+func closeGlobalJSONLLogger() error {
+	return closeGlobalLogger(&globalJSONLMu, &globalJSONLLogger)
+}
 
-	if globalMarkdownLogger != nil {
-		globalMarkdownLogger.Close()
-	}
-	globalMarkdownLogger = logger
+// initGlobalMarkdownLogger initializes the global MarkdownLogger using the generic helper.
+func initGlobalMarkdownLogger(logger *MarkdownLogger) {
+	initGlobalLogger(&globalMarkdownMu, &globalMarkdownLogger, logger)
 }
 
-// closeGlobalMarkdownLogger is a helper that encapsulates the common pattern for
-// closing and clearing a global MarkdownLogger with proper mutex handling.
+// closeGlobalMarkdownLogger closes the global MarkdownLogger using the generic helper.
 func closeGlobalMarkdownLogger() error {
-	globalMarkdownMu.Lock()
-	defer globalMarkdownMu.Unlock()
-
-	if globalMarkdownLogger != nil {
-		err := globalMarkdownLogger.Close()
-		globalMarkdownLogger = nil
-		return err
-	}
-	return nil
+	return closeGlobalLogger(&globalMarkdownMu, &globalMarkdownLogger)
 }

internal/logger/rpc_helpers.go

@@ -8,17 +8,36 @@
 // - extractEssentialFields: Extracts key JSON-RPC fields for compact logging
 // - getMapKeys: Utility for extracting map keys without values
 // - isEffectivelyEmpty: Checks if data is effectively empty (e.g., only params: null)
+// - ExtractErrorMessage: Extracts clean error messages from log lines
 //
 // These helpers are used by the RPC logging system to safely and efficiently
 // process message payloads before logging them.
 package logger
 
 import (
 	"encoding/json"
+	"regexp"
+	"strings"
 
 	"github.com/githubnext/gh-aw-mcpg/internal/logger/sanitize"
 )
 
+// Pre-compiled regexes for performance (avoid recompiling in hot paths).
+var (
+	// Timestamp patterns for log cleanup
+	// Pattern 1: ISO 8601 with T or space separator (e.g., "2024-01-01T12:00:00.123Z " or "2024-01-01 12:00:00 ").
+	timestampPattern1 = regexp.MustCompile(`^\d{4}-\d{2}-\d{2}[T\s]\d{2}:\d{2}:\d{2}(\.\d+)?([+-]\d{2}:\d{2}|Z)?\s*`)
+	// Pattern 2: Bracketed date-time (e.g., "[2024-01-01 12:00:00] ").
+	timestampPattern2 = regexp.MustCompile(`^\[\d{4}-\d{2}-\d{2}\s+\d{2}:\d{2}:\d{2}\]\s*`)
+	// Pattern 3: Bracketed time only (e.g., "[12:00:00] ").
+	timestampPattern3 = regexp.MustCompile(`^\[\d{2}:\d{2}:\d{2}\]\s+`)
+	// Pattern 4: Time only with optional milliseconds (e.g., "12:00:00.123 ").
+	timestampPattern4 = regexp.MustCompile(`^\d{2}:\d{2}:\d{2}(\.\d+)?\s+`)
+
+	// Log level pattern for message cleanup (case-insensitive).
+	logLevelPattern = regexp.MustCompile(`(?i)^\[?(ERROR|WARNING|WARN|INFO|DEBUG)\]?\s*[:-]?\s*`)
+)
+
 // truncateAndSanitize truncates the payload to max length and sanitizes secrets
 func truncateAndSanitize(payload string, maxLength int) string {
 	// First sanitize secrets
@@ -93,3 +112,28 @@ func isEffectivelyEmpty(data map[string]interface{}) bool {
 
 	return false
 }
+
+// ExtractErrorMessage extracts a clean error message from a log line.
+// It removes timestamps, log level prefixes, and other common noise.
+// If the message is longer than 200 characters, it will be truncated.
+func ExtractErrorMessage(line string) string {
+	// Remove common timestamp patterns using pre-compiled regexes
+	cleanedLine := line
+	cleanedLine = timestampPattern1.ReplaceAllString(cleanedLine, "")
+	cleanedLine = timestampPattern2.ReplaceAllString(cleanedLine, "")
+	cleanedLine = timestampPattern3.ReplaceAllString(cleanedLine, "")
+	cleanedLine = timestampPattern4.ReplaceAllString(cleanedLine, "")
+
+	// Remove common log level prefixes using pre-compiled regex
+	cleanedLine = logLevelPattern.ReplaceAllString(cleanedLine, "")
+
+	// Trim whitespace
+	cleanedLine = strings.TrimSpace(cleanedLine)
+
+	// If the line is too long (>200 chars), truncate it
+	if len(cleanedLine) > 200 {
+		cleanedLine = cleanedLine[:197] + "..."
+	}
+
+	return cleanedLine
+}