[duplicate-code] Duplicate Code Pattern: withLock delegation method across 4 logger types

[github-actions]

Part of duplicate code analysis: #3685

Summary

Four logger types each define an identical withLock method that simply delegates to the package-level withMutexLock helper. The body of each method is byte-for-byte the same except for the receiver name.

Duplication Details

Pattern: Per-type withLock delegation methods

• 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 (identical in all 4 files, receiver name varies):

  // withLock acquires xx.mu, executes fn, then releases xx.mu.
  // Use this in methods that return an error to avoid repeating the lock/unlock preamble.
  func (xx *XxxLogger) withLock(fn func() error) error {
      return withMutexLock(&xx.mu, fn)
  }

Impact Analysis

• Maintainability: New logger types must copy the method; forgetting to add it produces a compile error only if Close() uses it — otherwise the omission is silent.
• Bug Risk: Very low — single-line body, no logic to diverge.
• Code Bloat: ~20 lines of near-identical code across 4 files.

Refactoring Recommendations

1. Embed a lockedFile base struct that holds the sync.Mutex and provides withLock:

   // In global_helpers.go or a new file locked_base.go
   type lockedBase struct{ mu sync.Mutex }
   func (b *lockedBase) withLock(fn func() error) error { return withMutexLock(&b.mu, fn) }

   Each logger embeds lockedBase instead of a bare sync.Mutex, gaining withLock for free.

   • Estimated effort: ~30 min
   • Benefits: New logger types automatically get withLock; removes 4 identical method definitions.

2. Alternative: accept the duplication — The existing common.go documentation already explains the pattern; the methods are trivially small and unlikely to diverge. Given the low risk, this is a valid "do nothing" option.

Implementation Checklist

• Review duplication findings
• Decide: embed lockedBase or document as intentional
• If refactoring: update all 4 logger structs and remove redundant methods
• Run make agent-finished to verify no regressions

Parent Issue

See parent analysis report: #3685
Related to #3685

Generated by Duplicate Code Detector · ● 1.6M · ◷

• expires on Apr 20, 2026, 6:16 AM UTC

[lpcox]

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