github
diff --git a/‎internal/config/validation.go‎
Lines changed: 21 additions & 6 deletions b/‎internal/config/validation.go‎
Lines changed: 21 additions & 6 deletions
diff --git a/‎internal/logger/common.go‎
Lines changed: 31 additions & 46 deletions b/‎internal/logger/common.go‎
Lines changed: 31 additions & 46 deletions
diff --git a/‎internal/logger/file_logger.go‎
Lines changed: 36 additions & 30 deletions b/‎internal/logger/file_logger.go‎
Lines changed: 36 additions & 30 deletions
diff --git a/‎internal/logger/jsonl_logger.go‎
Lines changed: 27 additions & 20 deletions b/‎internal/logger/jsonl_logger.go‎
Lines changed: 27 additions & 20 deletions
@@ -20,6 +20,21 @@ var varExprPattern = regexp.MustCompile(`\$\{([A-Za-z_][A-Za-z0-9_]*)\}`)
 
 var logValidation = logger.New("config:validation")
 
+// logValidateServerStart logs the beginning of server config validation.
+func logValidateServerStart(name, serverType string) {
+	logValidation.Printf("Validating server config: name=%s, type=%s", name, serverType)
+}
+
+// logValidateServerPassed logs a successful server config validation.
+func logValidateServerPassed(name string) {
+	logValidation.Printf("Server config validation passed: name=%s", name)
+}
+
+// logValidateServerFailed logs a failed server config validation with the given reason.
+func logValidateServerFailed(name, reason string) {
+	logValidation.Printf("Validation failed: %s, name=%s", reason, name)
+}
+
 // expandVariablesCore is the shared implementation for variable expansion.
 // It works with byte slices and handles the core expansion logic, tracking undefined variables.
 // This eliminates code duplication between expandVariables and ExpandRawJSONVariables.
@@ -105,7 +120,7 @@ func validateMounts(mounts []string, jsonPath string) error {
 
 // validateServerConfigWithCustomSchemas validates a server configuration with custom schema support
 func validateServerConfigWithCustomSchemas(name string, server *StdinServerConfig, customSchemas map[string]interface{}) error {
-	logValidation.Printf("Validating server config: name=%s, type=%s", name, server.Type)
+	logValidateServerStart(name, server.Type)
 	jsonPath := fmt.Sprintf("mcpServers.%s", name)
 
 	// Validate type (empty defaults to stdio)
@@ -134,7 +149,7 @@ func validateStandardServerConfig(name string, server *StdinServerConfig, jsonPa
 	// For stdio servers, container is required
 	if server.Type == "stdio" || server.Type == "local" {
 		if server.Container == "" {
-			logValidation.Printf("Validation failed: stdio server missing container field, name=%s", name)
+			logValidateServerFailed(name, "stdio server missing container field")
 			return rules.MissingRequired("container", "stdio", jsonPath, "Add a 'container' field (e.g., \"ghcr.io/owner/image:tag\")")
 		}
 
@@ -150,16 +165,16 @@ func validateStandardServerConfig(name string, server *StdinServerConfig, jsonPa
 	// For HTTP servers, url is required and mounts are not allowed
 	if server.Type == "http" {
 		if server.URL == "" {
-			logValidation.Printf("Validation failed: HTTP server missing url field, name=%s", name)
+			logValidateServerFailed(name, "HTTP server missing url field")
 			return rules.MissingRequired("url", "HTTP", jsonPath, "Add a 'url' field (e.g., \"https://example.com/mcp\")")
 		}
 		if len(server.Mounts) > 0 {
-			logValidation.Printf("Validation failed: HTTP server has mounts field, name=%s", name)
+			logValidateServerFailed(name, "HTTP server has mounts field")
 			return rules.UnsupportedField("mounts", "mounts are only supported for stdio (containerized) servers", jsonPath, "Remove the 'mounts' field from HTTP server configuration; mounts only apply to stdio servers")
 		}
 	}
 
-	logValidation.Printf("Server config validation passed: name=%s", name)
+	logValidateServerPassed(name)
 	return nil
 }
 
@@ -403,7 +418,7 @@ func validateTOMLStdioContainerization(servers map[string]*ServerConfig) error {
 
 			// Check if command is Docker
 			if cfg.Command != "docker" {
-				logValidation.Printf("Validation failed: stdio server using non-Docker command, name=%s, command=%s", name, cfg.Command)
+				logValidateServerFailed(name, fmt.Sprintf("stdio server using non-Docker command, command=%s", cfg.Command))
 				return fmt.Errorf(
 					"server '%s': stdio servers must use containerized execution (command must be 'docker', got '%s'). "+
 						"This is required by MCP Gateway Specification Section 3.2.1 (Containerization Requirement). "+
 
@@ -11,27 +11,24 @@ import (
 
 // Close Pattern for Logger Types
 //
-// All logger types in this package should implement their Close() method using this pattern:
+// All logger types in this package implement their Close() method using the withLock
+// helper to ensure consistent mutex handling:
 //
 //	func (l *Logger) Close() error {
-//	    l.mu.Lock()
-//	    defer l.mu.Unlock()
-//
-//	    // Optional: Perform cleanup before closing (e.g., write footer)
-//	    // if l.logFile != nil {
-//	    //     if err := writeCleanup(); err != nil {
-//	    //         return closeLogFile(l.logFile, &l.mu, "loggerName")
-//	    //     }
-//	    // }
-//
-//	    return closeLogFile(l.logFile, &l.mu, "loggerName")
+//	    return l.withLock(func() error {
+//	        // Optional: Perform cleanup before closing (e.g., write footer)
+//	        return closeLogFile(l.logFile, &l.mu, "loggerName")
+//	    })
 //	}
 //
+// The withLock helper (defined on each logger type) acquires the mutex, executes the
+// callback, then releases the mutex — ensuring the lock is always released via defer.
+//
 // Why this pattern?
 //
-//  1. Mutex protection: Acquire lock at method entry to ensure thread-safe cleanup
-//  2. Deferred unlock: Use defer to release lock even if errors occur
-//  3. Optional cleanup: Logger-specific cleanup (like MarkdownLogger's footer) goes before closeLogFile
+//  1. Consistent locking: withLock enforces acquire-on-enter / release-on-exit
+//  2. Deferred unlock: Implemented inside withLock using defer, so it's never forgotten
+//  3. Optional cleanup: Logger-specific cleanup (like MarkdownLogger's footer) goes inside the callback
 //  4. Shared helper: Always delegate to closeLogFile() for consistent sync and close behavior
 //  5. Error handling: Return errors from closeLogFile to indicate serious issues
 //
@@ -40,38 +37,28 @@ import (
 // Simple Close() with no cleanup (FileLogger, JSONLLogger):
 //
 //	func (fl *FileLogger) Close() error {
-//	    fl.mu.Lock()
-//	    defer fl.mu.Unlock()
-//	    return closeLogFile(fl.logFile, &fl.mu, "file")
+//	    return fl.withLock(func() error {
+//	        return closeLogFile(fl.logFile, &fl.mu, "file")
+//	    })
 //	}
 //
 // Close() with custom cleanup (MarkdownLogger):
 //
 //	func (ml *MarkdownLogger) Close() error {
-//	    ml.mu.Lock()
-//	    defer ml.mu.Unlock()
-//
-//	    if ml.logFile != nil {
-//	        // Write closing details tag before closing
-//	        footer := "\n</details>\n"
-//	        if _, err := ml.logFile.WriteString(footer); err != nil {
-//	            // Even if footer write fails, try to close the file properly
+//	    return ml.withLock(func() error {
+//	        if ml.logFile != nil {
+//	            footer := "\n</details>\n"
+//	            if _, err := ml.logFile.WriteString(footer); err != nil {
+//	                return closeLogFile(ml.logFile, &ml.mu, "markdown")
+//	            }
 //	            return closeLogFile(ml.logFile, &ml.mu, "markdown")
 //	        }
-//
-//	        // Footer written successfully, now close
-//	        return closeLogFile(ml.logFile, &ml.mu, "markdown")
-//	    }
-//	    return nil
+//	        return nil
+//	    })
 //	}
 //
-// This pattern is intentionally duplicated across logger types rather than abstracted:
-//   - It's a standard Go idiom for wrapper methods
-//   - The duplication is minimal (5-14 lines per type)
-//   - Each logger can customize cleanup as needed
-//   - The shared closeLogFile() helper eliminates complex logic duplication
-//
-// When adding a new logger type, follow this pattern to ensure consistent behavior.
+// When adding a new logger type, add a withLock helper and follow this pattern to ensure
+// consistent, safe Close() behavior.
 
 // Initialization Pattern for Logger Types
 //
@@ -81,23 +68,21 @@ import (
 //
 // Standard Initialization Pattern:
 //
-// All logger types use the initLogger() generic helper function for initialization:
+// All logger types use the initLogger() generic helper function for initialization.
+// The setup and error-handler callbacks are defined as named package-level functions
+// (e.g., setupFileLogger, handleFileLoggerError) to aid readability and testability:
 //
 //	func Init*Logger(logDir, fileName string) error {
-//	    logger, err := initLogger(
-//	        logDir, fileName, fileFlags,
-//	        setupFunc,    // Configure logger after file is opened
-//	        errorHandler, // Handle initialization failures
-//	    )
+//	    logger, err := initLogger(logDir, fileName, fileFlags, setup*Logger, handle*LoggerError)
 //	    initGlobal*Logger(logger)
 //	    return err
 //	}
 //
 // The initLogger() helper:
 //  1. Attempts to create the log directory (if needed)
 //  2. Opens the log file with specified flags (os.O_APPEND, os.O_TRUNC, etc.)
-//  3. Calls setupFunc to configure the logger instance
-//  4. On error, calls errorHandler to implement fallback behavior
+//  3. Calls setup*Logger to configure the logger instance
+//  4. On error, calls handle*LoggerError to implement fallback behavior
 //  5. Returns the initialized logger and any error
 //
 // Fallback Behavior Strategies:
 
@@ -23,46 +23,52 @@ var (
 	globalLoggerMu   sync.RWMutex
 )
 
+// setupFileLogger configures a FileLogger after the log file has been opened.
+func setupFileLogger(file *os.File, logDir, fileName string) (*FileLogger, error) {
+	fl := &FileLogger{
+		logDir:   logDir,
+		fileName: fileName,
+		logFile:  file,
+		logger:   log.New(file, "", 0),
+	}
+	log.Printf("Logging to file: %s", filepath.Join(logDir, fileName))
+	return fl, nil
+}
+
+// handleFileLoggerError falls back to stdout when the log file cannot be opened.
+func handleFileLoggerError(err error, logDir, fileName string) (*FileLogger, error) {
+	log.Printf("WARNING: Failed to initialize log file: %v", err)
+	log.Printf("WARNING: Falling back to stdout for logging")
+	fl := &FileLogger{
+		logDir:      logDir,
+		fileName:    fileName,
+		useFallback: true,
+		logger:      log.New(os.Stdout, "", 0),
+	}
+	return fl, nil
+}
+
 // InitFileLogger initializes the global file logger
 // If the log directory doesn't exist and can't be created, falls back to stdout
 func InitFileLogger(logDir, fileName string) error {
-	logger, err := initLogger(
-		logDir, fileName, os.O_APPEND,
-		// Setup function: configure the logger after file is opened
-		func(file *os.File, logDir, fileName string) (*FileLogger, error) {
-			fl := &FileLogger{
-				logDir:   logDir,
-				fileName: fileName,
-				logFile:  file,
-				logger:   log.New(file, "", 0),
-			}
-			log.Printf("Logging to file: %s", filepath.Join(logDir, fileName))
-			return fl, nil
-		},
-		// Error handler: fallback to stdout on error
-		func(err error, logDir, fileName string) (*FileLogger, error) {
-			log.Printf("WARNING: Failed to initialize log file: %v", err)
-			log.Printf("WARNING: Falling back to stdout for logging")
-			fl := &FileLogger{
-				logDir:      logDir,
-				fileName:    fileName,
-				useFallback: true,
-				logger:      log.New(os.Stdout, "", 0), // We'll add our own timestamp
-			}
-			return fl, nil
-		},
-	)
-
+	logger, err := initLogger(logDir, fileName, os.O_APPEND, setupFileLogger, handleFileLoggerError)
 	initGlobalFileLogger(logger)
 	return err
 }
 
-// Close closes the log file
-func (fl *FileLogger) Close() error {
+// withLock acquires fl.mu, executes fn, then releases fl.mu.
+// Use this in methods that return an error to avoid repeating the lock/unlock preamble.
+func (fl *FileLogger) withLock(fn func() error) error {
 	fl.mu.Lock()
 	defer fl.mu.Unlock()
+	return fn()
+}
 
-	return closeLogFile(fl.logFile, &fl.mu, "file")
+// Close closes the log file
+func (fl *FileLogger) Close() error {
+	return fl.withLock(func() error {
+		return closeLogFile(fl.logFile, &fl.mu, "file")
+	})
 }
 
 // LogLevel represents the severity of a log message
 
@@ -37,25 +37,25 @@ type JSONLRPCMessage struct {
 	Payload        json.RawMessage `json:"payload"` // Full sanitized payload as raw JSON
 }
 
+// setupJSONLLogger configures a JSONLLogger after the log file has been opened.
+func setupJSONLLogger(file *os.File, logDir, fileName string) (*JSONLLogger, error) {
+	jl := &JSONLLogger{
+		logDir:   logDir,
+		fileName: fileName,
+		logFile:  file,
+		encoder:  json.NewEncoder(file),
+	}
+	return jl, nil
+}
+
+// handleJSONLLoggerError returns the error immediately — JSONLLogger has no fallback mode.
+func handleJSONLLoggerError(err error, _ string, _ string) (*JSONLLogger, error) {
+	return nil, err
+}
+
 // InitJSONLLogger initializes the global JSONL logger
 func InitJSONLLogger(logDir, fileName string) error {
-	logger, err := initLogger(
-		logDir, fileName, os.O_APPEND,
-		// Setup function: configure the logger after file is opened
-		func(file *os.File, logDir, fileName string) (*JSONLLogger, error) {
-			jl := &JSONLLogger{
-				logDir:   logDir,
-				fileName: fileName,
-				logFile:  file,
-				encoder:  json.NewEncoder(file),
-			}
-			return jl, nil
-		},
-		// Error handler: return error immediately (no fallback)
-		func(err error, logDir, fileName string) (*JSONLLogger, error) {
-			return nil, err
-		},
-	)
+	logger, err := initLogger(logDir, fileName, os.O_APPEND, setupJSONLLogger, handleJSONLLoggerError)
 
 	// Only initialize global logger if successful (no error)
 	// Unlike FileLogger/MarkdownLogger which return fallback loggers,
@@ -67,12 +67,19 @@ func InitJSONLLogger(logDir, fileName string) error {
 	return err
 }
 
-// Close closes the JSONL log file
-func (jl *JSONLLogger) Close() error {
+// withLock acquires jl.mu, executes fn, then releases jl.mu.
+// Use this in methods that return an error to avoid repeating the lock/unlock preamble.
+func (jl *JSONLLogger) withLock(fn func() error) error {
 	jl.mu.Lock()
 	defer jl.mu.Unlock()
+	return fn()
+}
 
-	return closeLogFile(jl.logFile, &jl.mu, "JSONL")
+// Close closes the JSONL log file
+func (jl *JSONLLogger) Close() error {
+	return jl.withLock(func() error {
+		return closeLogFile(jl.logFile, &jl.mu, "JSONL")
+	})
 }
 
 // LogMessage logs an RPC message to the JSONL file