Part of duplicate code analysis: #2907
Summary
Four logger types each contain an identical withLock() method body. Any change to the locking strategy (e.g., adding tracing, switching to RWMutex, adding timeout logic) must be applied in all four places.
Duplication Details
Pattern: Identical withLock() method across 4 logger types
-
Severity: High
-
Occurrences: 4 (one per logger type)
-
Locations:
internal/logger/file_logger.go (lines ~61–65)
internal/logger/jsonl_logger.go (lines ~72–76)
internal/logger/markdown_logger.go (lines ~73–77)
internal/logger/tools_logger.go (lines ~72–76)
-
Code Sample (verbatim across all 4 files, only receiver/type name differs):
func (fl *FileLogger) withLock(fn func() error) error {
fl.mu.Lock()
defer fl.mu.Unlock()
return fn()
}
Impact Analysis
- Maintainability: Any locking change requires 4 identical edits
- Bug Risk: Inconsistent fixes if one copy diverges (e.g., only 3 of 4 get a deadlock fix)
- Code Bloat: ~20 lines of pure duplication
Refactoring Recommendations
-
Add a package-level helper in internal/logger/global_helpers.go (or a new lock_helpers.go):
// withMutexLock acquires mu, calls fn, and releases mu.
func withMutexLock(mu *sync.Mutex, fn func() error) error {
mu.Lock()
defer mu.Unlock()
return fn()
}
Then each logger's withLock delegates in one line:
func (fl *FileLogger) withLock(fn func() error) error {
return withMutexLock(&fl.mu, fn)
}
Estimated effort: ~30 minutes. Zero behavior change.
-
Alternative: Remove the per-type withLock wrappers entirely and call withMutexLock directly at each call site.
Implementation Checklist
Parent Issue
See parent analysis report: #2907
Related to #2907
Generated by Duplicate Code Detector · ◷
Part of duplicate code analysis: #2907
Summary
Four logger types each contain an identical
withLock()method body. Any change to the locking strategy (e.g., adding tracing, switching toRWMutex, adding timeout logic) must be applied in all four places.Duplication Details
Pattern: Identical
withLock()method across 4 logger typesSeverity: High
Occurrences: 4 (one per logger type)
Locations:
internal/logger/file_logger.go(lines ~61–65)internal/logger/jsonl_logger.go(lines ~72–76)internal/logger/markdown_logger.go(lines ~73–77)internal/logger/tools_logger.go(lines ~72–76)Code Sample (verbatim across all 4 files, only receiver/type name differs):
Impact Analysis
Refactoring Recommendations
Add a package-level helper in
internal/logger/global_helpers.go(or a newlock_helpers.go):Then each logger's
withLockdelegates in one line:Estimated effort: ~30 minutes. Zero behavior change.
Alternative: Remove the per-type
withLockwrappers entirely and callwithMutexLockdirectly at each call site.Implementation Checklist
withMutexLockhelperFileLogger.withLockJSONLLogger.withLockMarkdownLogger.withLockToolsLogger.withLockmake testto verify no regressionsParent Issue
See parent analysis report: #2907
Related to #2907