Part of duplicate code analysis: #3685
Summary
Every logger type exposes a package-level CloseXxxLogger() function that is a single-line wrapper around closeGlobalLogger(...). Five such functions exist, each body being structurally identical.
Duplication Details
Pattern: CloseXxxLogger single-line global close functions
- Severity: Low
- Occurrences: 5
- Locations:
internal/logger/file_logger.go:155 — CloseGlobalLogger()
internal/logger/jsonl_logger.go:109 — CloseJSONLLogger()
internal/logger/markdown_logger.go:217 — CloseMarkdownLogger()
internal/logger/tools_logger.go:151 — CloseToolsLogger()
internal/logger/server_file_logger.go:179 — CloseServerFileLogger()
- Code Sample:
// CloseXxxLogger closes the global Xxx logger
func CloseXxxLogger() error {
return closeGlobalLogger(&globalXxxMu, &globalXxxLogger)
}
Impact Analysis
- Maintainability: Adding a new logger type requires remembering to add a
CloseXxxLogger function; there is no central registry that ensures all loggers are closed.
- Bug Risk: Low for existing loggers. Medium risk that a new logger's
Close function is omitted from the shutdown sequence in cmd/root.go.
- Code Bloat: ~15 lines of near-identical boilerplate.
Refactoring Recommendations
-
Introduce a logger registry — Register each global logger at Init time into a slice; provide a single CloseAll() error function that iterates and closes them. Remove the 5 individual CloseXxxLogger functions (or keep them as thin aliases for backward compatibility).
// global_helpers.go
var globalLoggers []io.Closer
func registerLogger(l io.Closer) { globalLoggers = append(globalLoggers, l) }
func CloseAll() error {
var firstErr error
for _, l := range globalLoggers {
if err := l.Close(); err != nil && firstErr == nil { firstErr = err }
}
return firstErr
}
- Estimated effort: ~1 hour
- Benefits: New loggers automatically participate in shutdown; removes 5 near-identical functions; simplifies
cmd/root.go shutdown sequence.
-
Accept the pattern — The functions are trivial and adding a new logger is infrequent. Document that cmd/root.go must be updated whenever a new logger is added.
Implementation Checklist
Parent Issue
See parent analysis report: #3685
Related to #3685
Generated by Duplicate Code Detector · ● 1.6M · ◷
Part of duplicate code analysis: #3685
Summary
Every logger type exposes a package-level
CloseXxxLogger()function that is a single-line wrapper aroundcloseGlobalLogger(...). Five such functions exist, each body being structurally identical.Duplication Details
Pattern:
CloseXxxLoggersingle-line global close functionsinternal/logger/file_logger.go:155—CloseGlobalLogger()internal/logger/jsonl_logger.go:109—CloseJSONLLogger()internal/logger/markdown_logger.go:217—CloseMarkdownLogger()internal/logger/tools_logger.go:151—CloseToolsLogger()internal/logger/server_file_logger.go:179—CloseServerFileLogger()Impact Analysis
CloseXxxLoggerfunction; there is no central registry that ensures all loggers are closed.Closefunction is omitted from the shutdown sequence incmd/root.go.Refactoring Recommendations
Introduce a logger registry — Register each global logger at
Inittime into a slice; provide a singleCloseAll() errorfunction that iterates and closes them. Remove the 5 individualCloseXxxLoggerfunctions (or keep them as thin aliases for backward compatibility).cmd/root.goshutdown sequence.Accept the pattern — The functions are trivial and adding a new logger is infrequent. Document that
cmd/root.gomust be updated whenever a new logger is added.Implementation Checklist
cmd/root.goshutdown sequence to useCloseAll()make agent-finishedto verify no regressionsParent Issue
See parent analysis report: #3685
Related to #3685