Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 1 addition & 7 deletions internal/logger/file_logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ import (

// FileLogger manages logging to a file with fallback to stdout
type FileLogger struct {
lockable
logFile *os.File
logger *log.Logger
mu sync.Mutex
logDir string
fileName string
useFallback bool
Expand Down Expand Up @@ -56,12 +56,6 @@ func InitFileLogger(logDir, fileName string) error {
return err
}

// 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 {
return withMutexLock(&fl.mu, fn)
}

// Close closes the log file
func (fl *FileLogger) Close() error {
return fl.withLock(func() error {
Expand Down
44 changes: 42 additions & 2 deletions internal/logger/global_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,30 @@ package logger

import "sync"

// lockable provides a mutex and a withLock helper method. Embed this struct
// in logger types that need a sync.Mutex plus a withLock convenience method
// to eliminate the repeated per-type withLock boilerplate.
//
// Usage:
//
// type MyLogger struct {
// lockable
// // other fields...
// }
//
// The embedded withLock method is promoted, so code in this package can write
// myLogger.withLock(fn) directly. The embedded mu field is also promoted, but
// because it is unexported it is only directly accessible within the logger package.
type lockable struct {
mu sync.Mutex
}

// withLock acquires l.mu, executes fn, then releases l.mu.
func (l *lockable) withLock(fn func() error) error {
return withMutexLock(&l.mu, fn)
}

// withMutexLock acquires mu, calls fn, and releases mu.
// This is the single implementation of the per-type withLock pattern
// used by FileLogger, JSONLLogger, MarkdownLogger, and ToolsLogger.
func withMutexLock(mu *sync.Mutex, fn func() error) error {
mu.Lock()
defer mu.Unlock()
Expand Down Expand Up @@ -122,3 +143,22 @@ func closeGlobalLogger[T closableLogger](mu *sync.RWMutex, logger *T) error {
}
return nil
}

// CloseAllLoggers closes all global loggers in a single call.
// Returns the first error encountered, but attempts to close every logger.
func CloseAllLoggers() error {
closers := []func() error{
CloseGlobalLogger,
CloseJSONLLogger,
CloseMarkdownLogger,
CloseToolsLogger,
CloseServerFileLogger,
}
var firstErr error
for _, fn := range closers {
if err := fn(); err != nil && firstErr == nil {
firstErr = err
}
}
return firstErr
}
Comment on lines +147 to +164
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CloseAllLoggers introduces new exported behavior (ordering, first-error semantics, and “attempt all closes”). Please add unit tests to lock in: (1) all CloseXxx functions are invoked even if an earlier one returns an error, and (2) the first error is returned. Also consider updating the file-level comment that says helpers here “should not be called directly by external code”, since this function is intended as a public entry point.

Copilot uses AI. Check for mistakes.
8 changes: 1 addition & 7 deletions internal/logger/jsonl_logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import (

// JSONLLogger manages logging RPC messages to a JSONL file (one JSON object per line)
type JSONLLogger struct {
lockable
logFile *os.File
mu sync.Mutex
logDir string
fileName string
encoder *json.Encoder
Expand Down Expand Up @@ -67,12 +67,6 @@ func InitJSONLLogger(logDir, fileName string) error {
return err
}

// 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 {
return withMutexLock(&jl.mu, fn)
}

// Close closes the JSONL log file
func (jl *JSONLLogger) Close() error {
return jl.withLock(func() error {
Expand Down
8 changes: 1 addition & 7 deletions internal/logger/markdown_logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ import (

// MarkdownLogger manages logging to a markdown file for GitHub workflow previews
type MarkdownLogger struct {
lockable
logFile *os.File
mu sync.Mutex
logDir string
fileName string
useFallback bool
Expand Down Expand Up @@ -68,12 +68,6 @@ func (ml *MarkdownLogger) initializeFile() error {
return nil
}

// 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 {
return withMutexLock(&ml.mu, fn)
}

// Close closes the log file and writes the closing details tag
func (ml *MarkdownLogger) Close() error {
return ml.withLock(func() error {
Expand Down
8 changes: 1 addition & 7 deletions internal/logger/tools_logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ type ToolsData struct {

// ToolsLogger manages logging of MCP server tools to a JSON file
type ToolsLogger struct {
lockable
logDir string
fileName string
data *ToolsData
mu sync.Mutex
useFallback bool
}

Expand Down Expand Up @@ -81,12 +81,6 @@ func InitToolsLogger(logDir, fileName string) error {
return err
}

// 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 {
return withMutexLock(&tl.mu, fn)
}

// LogTools logs the tools for a specific server
func (tl *ToolsLogger) LogTools(serverID string, tools []ToolInfo) error {
return tl.withLock(func() error {
Expand Down
Loading