[duplicate-code] Duplicate Code Pattern: Identical withLock methods in logger structs

[github-actions]

Part of duplicate code analysis: #3735

Summary

Four logger structs in internal/logger/ each define an identical 3-line withLock method that simply delegates to the package-level withMutexLock helper. The bodies are byte-for-byte identical except for the receiver name and the struct field referenced.

Duplication Details

Pattern: Identical withLock method bodies across 4 logger structs

• Severity: Low
• Occurrences: 4
• Locations:
  • internal/logger/file_logger.go (lines 59–63)
  • internal/logger/jsonl_logger.go (lines 70–74)
  • internal/logger/markdown_logger.go (lines 71–75)
  • internal/logger/tools_logger.go (lines 84–88)
• Code Sample (all 4 are structurally identical):

// 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)
}

Each struct (FileLogger, JSONLLogger, MarkdownLogger, ToolsLogger) embeds a mu sync.Mutex field and an identical 3-line withLock wrapper.

Impact Analysis

• Maintainability: Any future change to the withLock pattern (e.g., adding timeout support, tracing) must be applied to 4 separate methods
• Bug Risk: Low — the delegation to withMutexLock is correct in all 4 cases; the risk is future divergence if one is modified
• Code Bloat: Minimal — 12 lines of identical method bodies

Refactoring Recommendations

1. Extract an embedded lockedLogger struct
   • Create a shared lockedLogger struct (or mutexBase) in internal/logger/common.go that holds mu sync.Mutex and provides the withLock method
   • Each logger struct embeds lockedLogger instead of having its own mu sync.Mutex + withLock
   • Example:

// In common.go
type lockedLogger struct {
    mu sync.Mutex
}

func (l *lockedLogger) withLock(fn func() error) error {
    return withMutexLock(&l.mu, fn)
}

Then each logger:

type FileLogger struct {
    lockedLogger           // replaces mu sync.Mutex + withLock
    logFile *os.File
    logger  *log.Logger
}

2. Estimated effort: 30–60 minutes
3. Benefits: Single point of change for locking strategy; consistent withLock semantics

Implementation Checklist

• Add lockedLogger (or equivalent) embedded struct to internal/logger/common.go
• Remove mu sync.Mutex and withLock from FileLogger, JSONLLogger, MarkdownLogger, ToolsLogger
• Update any direct fl.mu.Lock() / fl.mu.Unlock() calls to use the embedded field (e.g., fl.lockedLogger.mu)
• Run make test-unit to confirm no regressions
• Update tests if needed

Parent Issue

See parent analysis report: #3735
Related to #3735

Generated by Duplicate Code Detector · ● 2.4M · ◷

• expires on Apr 21, 2026, 6:13 AM UTC

[lpcox]

Fixed by PR #3766 — withLock methods replaced by embedded lockable struct.