From 87d183529876672228210174163cc7c6b8686922 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 31 Mar 2026 12:44:56 +0000 Subject: [PATCH] refactor(logger): eliminate withLock duplication across 4 logger types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a package-level withMutexLock helper to global_helpers.go and have each logger's withLock method delegate to it, eliminating 4 identical lock/unlock bodies. Before: each of FileLogger, JSONLLogger, MarkdownLogger, and ToolsLogger contained an identical withLock implementation — 3 lines each, 12 total. After: each delegates to withMutexLock in one line; the canonical implementation lives in one place. Zero behaviour change: same mutex, same defer, same return path. Closes #2908 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- internal/logger/file_logger.go | 4 +--- internal/logger/global_helpers.go | 9 +++++++++ internal/logger/jsonl_logger.go | 4 +--- internal/logger/markdown_logger.go | 4 +--- internal/logger/tools_logger.go | 4 +--- 5 files changed, 13 insertions(+), 12 deletions(-) diff --git a/internal/logger/file_logger.go b/internal/logger/file_logger.go index 9038e9a9..da2c6079 100644 --- a/internal/logger/file_logger.go +++ b/internal/logger/file_logger.go @@ -59,9 +59,7 @@ func InitFileLogger(logDir, fileName string) 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 withMutexLock(&fl.mu, fn) } // Close closes the log file diff --git a/internal/logger/global_helpers.go b/internal/logger/global_helpers.go index 2bec733e..807573d2 100644 --- a/internal/logger/global_helpers.go +++ b/internal/logger/global_helpers.go @@ -16,6 +16,15 @@ package logger import "sync" +// withMutexLock acquires mu, calls fn, and releases mu. +// It is a package-level helper to avoid repeating the lock/unlock preamble in +// per-type withLock methods. +func withMutexLock(mu *sync.Mutex, fn func() error) error { + mu.Lock() + defer mu.Unlock() + return fn() +} + // closableLogger is a constraint for types that have a Close method. // This is satisfied by *FileLogger, *JSONLLogger, *MarkdownLogger, *ServerFileLogger, and *ToolsLogger. type closableLogger interface { diff --git a/internal/logger/jsonl_logger.go b/internal/logger/jsonl_logger.go index cbbb3631..ff14a148 100644 --- a/internal/logger/jsonl_logger.go +++ b/internal/logger/jsonl_logger.go @@ -70,9 +70,7 @@ func InitJSONLLogger(logDir, fileName string) 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 withMutexLock(&jl.mu, fn) } // Close closes the JSONL log file diff --git a/internal/logger/markdown_logger.go b/internal/logger/markdown_logger.go index aaa8e020..7a9d4ffa 100644 --- a/internal/logger/markdown_logger.go +++ b/internal/logger/markdown_logger.go @@ -71,9 +71,7 @@ func (ml *MarkdownLogger) initializeFile() error { // withLock acquires ml.mu, executes fn, then releases ml.mu. // Use this in methods that return an error to avoid repeating the lock/unlock preamble. func (ml *MarkdownLogger) withLock(fn func() error) error { - ml.mu.Lock() - defer ml.mu.Unlock() - return fn() + return withMutexLock(&ml.mu, fn) } // Close closes the log file and writes the closing details tag diff --git a/internal/logger/tools_logger.go b/internal/logger/tools_logger.go index b1f7952f..afd5b1b4 100644 --- a/internal/logger/tools_logger.go +++ b/internal/logger/tools_logger.go @@ -84,9 +84,7 @@ func InitToolsLogger(logDir, fileName string) error { // withLock acquires tl.mu, executes fn, then releases tl.mu. // Use this in methods that return an error to avoid repeating the lock/unlock preamble. func (tl *ToolsLogger) withLock(fn func() error) error { - tl.mu.Lock() - defer tl.mu.Unlock() - return fn() + return withMutexLock(&tl.mu, fn) } // LogTools logs the tools for a specific server