From c98a09ae3dc3ee9777c8d35b483ebf12a5de6ce4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 16 Jan 2026 03:20:41 +0000 Subject: [PATCH 1/2] Initial plan From e72b9ab12bde767281a08ff1249290e367a66903 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 16 Jan 2026 03:33:33 +0000 Subject: [PATCH 2/2] Extract duplicate global logger init/close patterns into helpers - Created global_patterns.go with 6 helper functions - Extracted mutex lock/unlock/check/close/nil patterns - Reduced duplication in FileLogger, JSONLLogger, MarkdownLogger - Init*Logger functions now call initGlobal*Logger helpers - Close*Logger functions now call closeGlobal*Logger helpers - Each helper encapsulates the mutex handling and close-then-assign pattern - Maintains type safety with specific helpers for each logger type - All existing tests pass Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- internal/logger/file_logger.go | 22 ++------ internal/logger/global_patterns.go | 83 ++++++++++++++++++++++++++++++ internal/logger/jsonl_logger.go | 20 +------ internal/logger/markdown_logger.go | 22 ++------ 4 files changed, 91 insertions(+), 56 deletions(-) create mode 100644 internal/logger/global_patterns.go diff --git a/internal/logger/file_logger.go b/internal/logger/file_logger.go index 3b7ee7c1a..f01917db5 100644 --- a/internal/logger/file_logger.go +++ b/internal/logger/file_logger.go @@ -28,14 +28,6 @@ var ( // 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 { - globalLoggerMu.Lock() - defer globalLoggerMu.Unlock() - - if globalFileLogger != nil { - // Close existing logger - globalFileLogger.Close() - } - fl := &FileLogger{ logDir: logDir, fileName: fileName, @@ -49,7 +41,7 @@ func InitFileLogger(logDir, fileName string) error { log.Printf("WARNING: Falling back to stdout for logging") fl.useFallback = true fl.logger = log.New(os.Stdout, "", 0) // We'll add our own timestamp - globalFileLogger = fl + initGlobalFileLogger(fl) return nil } @@ -58,7 +50,7 @@ func InitFileLogger(logDir, fileName string) error { log.Printf("Logging to file: %s", filepath.Join(logDir, fileName)) - globalFileLogger = fl + initGlobalFileLogger(fl) return nil } @@ -155,13 +147,5 @@ func LogDebug(category, format string, args ...interface{}) { // CloseGlobalLogger closes the global file logger func CloseGlobalLogger() error { - globalLoggerMu.Lock() - defer globalLoggerMu.Unlock() - - if globalFileLogger != nil { - err := globalFileLogger.Close() - globalFileLogger = nil - return err - } - return nil + return closeGlobalFileLogger() } diff --git a/internal/logger/global_patterns.go b/internal/logger/global_patterns.go new file mode 100644 index 000000000..896fe89e4 --- /dev/null +++ b/internal/logger/global_patterns.go @@ -0,0 +1,83 @@ +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. + +// 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() + + if globalFileLogger != nil { + globalFileLogger.Close() + } + globalFileLogger = logger +} + +// 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() + + if globalFileLogger != nil { + err := globalFileLogger.Close() + globalFileLogger = nil + 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() + + if globalJSONLLogger != nil { + globalJSONLLogger.Close() + } + globalJSONLLogger = 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() + + if globalJSONLLogger != nil { + err := globalJSONLLogger.Close() + globalJSONLLogger = nil + return err + } + return nil +} + +// 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() + + if globalMarkdownLogger != nil { + globalMarkdownLogger.Close() + } + globalMarkdownLogger = logger +} + +// closeGlobalMarkdownLogger is a helper that encapsulates the common pattern for +// closing and clearing a global MarkdownLogger with proper mutex handling. +func closeGlobalMarkdownLogger() error { + globalMarkdownMu.Lock() + defer globalMarkdownMu.Unlock() + + if globalMarkdownLogger != nil { + err := globalMarkdownLogger.Close() + globalMarkdownLogger = nil + return err + } + return nil +} diff --git a/internal/logger/jsonl_logger.go b/internal/logger/jsonl_logger.go index 4e485d19d..03097d662 100644 --- a/internal/logger/jsonl_logger.go +++ b/internal/logger/jsonl_logger.go @@ -37,14 +37,6 @@ type JSONLRPCMessage struct { // InitJSONLLogger initializes the global JSONL logger func InitJSONLLogger(logDir, fileName string) error { - globalJSONLMu.Lock() - defer globalJSONLMu.Unlock() - - if globalJSONLLogger != nil { - // Close existing logger - globalJSONLLogger.Close() - } - jl := &JSONLLogger{ logDir: logDir, fileName: fileName, @@ -59,7 +51,7 @@ func InitJSONLLogger(logDir, fileName string) error { jl.logFile = file jl.encoder = json.NewEncoder(file) - globalJSONLLogger = jl + initGlobalJSONLLogger(jl) return nil } @@ -103,15 +95,7 @@ func (jl *JSONLLogger) LogMessage(entry *JSONLRPCMessage) error { // CloseJSONLLogger closes the global JSONL logger func CloseJSONLLogger() error { - globalJSONLMu.Lock() - defer globalJSONLMu.Unlock() - - if globalJSONLLogger != nil { - err := globalJSONLLogger.Close() - globalJSONLLogger = nil - return err - } - return nil + return closeGlobalJSONLLogger() } // LogRPCMessageJSONL logs an RPC message to the global JSONL logger diff --git a/internal/logger/markdown_logger.go b/internal/logger/markdown_logger.go index f601f5453..8c8490209 100644 --- a/internal/logger/markdown_logger.go +++ b/internal/logger/markdown_logger.go @@ -26,14 +26,6 @@ var ( // InitMarkdownLogger initializes the global markdown logger func InitMarkdownLogger(logDir, fileName string) error { - globalMarkdownMu.Lock() - defer globalMarkdownMu.Unlock() - - if globalMarkdownLogger != nil { - // Close existing logger - globalMarkdownLogger.Close() - } - ml := &MarkdownLogger{ logDir: logDir, fileName: fileName, @@ -44,14 +36,14 @@ func InitMarkdownLogger(logDir, fileName string) error { if err != nil { // File initialization failed - set fallback mode ml.useFallback = true - globalMarkdownLogger = ml + initGlobalMarkdownLogger(ml) return nil } ml.logFile = file ml.initialized = false // Will be initialized on first write - globalMarkdownLogger = ml + initGlobalMarkdownLogger(ml) return nil } @@ -222,13 +214,5 @@ func LogDebugMd(category, format string, args ...interface{}) { // CloseMarkdownLogger closes the global markdown logger func CloseMarkdownLogger() error { - globalMarkdownMu.Lock() - defer globalMarkdownMu.Unlock() - - if globalMarkdownLogger != nil { - err := globalMarkdownLogger.Close() - globalMarkdownLogger = nil - return err - } - return nil + return closeGlobalMarkdownLogger() }