Refactor logger initialization with generic function to eliminate duplication

[Copilot]

Problem

Three logger initialization functions (InitFileLogger, InitJSONLLogger, InitMarkdownLogger) contained ~60-75 lines of duplicated boilerplate with only minor variations in error handling and field setup.

Changes

Added generic initLogger function (internal/logger/common.go)

• Uses Go generics with closableLogger constraint
• Accepts setup and error handler callbacks to parameterize logger-specific behavior
• Centralizes: file opening, logger setup, error handling

Refactored Init functions to use generic

• InitFileLogger: Error handler returns fallback logger (stdout)
• InitJSONLLogger: Error handler returns nil, err (fail-fast, conditional global init)
• InitMarkdownLogger: Error handler returns fallback logger (no stdout redirect)

Added tests (internal/logger/common_test.go)

• 8 test functions covering success, error handling, file flags, setup errors

Example

Before (duplicated across 3 files):

func InitJSONLLogger(logDir, fileName string) error {
    jl := &JSONLLogger{logDir: logDir, fileName: fileName}
    file, err := initLogFile(logDir, fileName, os.O_APPEND)
    if err != nil {
        return err
    }
    jl.logFile = file
    jl.encoder = json.NewEncoder(file)
    initGlobalJSONLLogger(jl)
    return nil
}

After (behavior parameterized):

func InitJSONLLogger(logDir, fileName string) error {
    logger, err := initLogger(
        logDir, fileName, os.O_APPEND,
        func(file *os.File, logDir, fileName string) (*JSONLLogger, error) {
            return &JSONLLogger{logDir: logDir, fileName: fileName, logFile: file, encoder: json.NewEncoder(file)}, nil
        },
        func(err error, logDir, fileName string) (*JSONLLogger, error) {
            return nil, err  // Fail-fast, no fallback
        },
    )
    if err == nil {
        initGlobalJSONLLogger(logger)
    }
    return err
}

Impact

• Eliminates ~45 lines of duplicated initialization logic
• Initialization changes now require updates in one place
• All existing behavior preserved (verified by existing test suite)

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• go.googlesource.com
  • Triggering command: `/update-job-proxy /update-job-proxy /home/REDACTED/work/_temp/runtime-logs/mkcert/rootCA.pem --updater-env NODE_EXTRA_CA_CERTS=/usr/local/share/ca-certificates/dbot-ca.crt /tmp�� nI_BWDnVB 9120902/b171/_x0-tests by/ce9cdaacc52f7340dcf9ee487e097b2c67f2d543876c7b925e05a5076d42d39c/log.json 64/src/net ap 64/pkg/tool/linu--verify ortcfg 5e05�� aw-mcpg/internal/launcher/connection_pool.go aw-mcpg/internal/launcher/launcher.go 64/pkg/tool/linux_amd64/link /server/auth.go tialization

• Ad-d 64/pkg/tool/linu-c 64/pkg/tool/linux_amd64/link` (dns block)

• Triggering command: /update-job-proxy /update-job-proxy DROP 671f1b40 test -e _and_Research_In--depth=1 /home/REDACTED/wororigin /opt/go/bin/testrefs/tags/v1.10.2:refs/tags/v1.10.2 64/pkg/tool/linu/usr/lib/git-core/git 671f1b40 table.d/chrony-o/home/dependabot/go/pkg/mod/cache/vcs/fb978822a4944776ca42509272a7325dad7bcbd80ced596132267b9bc8b28db8/shallow.lock test -e mSign_ECC_Root_C--pack_header=2,68 table.d/chrony-onoffline /bin/test rt 816b640640d473de--wait cli/dependabot/d-t test (dns block)
• go.yaml.in
  • Triggering command: `/update-job-proxy /update-job-proxy /home/REDACTED/work/_temp/runtime-logs/mkcert/rootCA.pem --updater-env NODE_EXTRA_CA_CERTS=/usr/local/share/ca-certificates/dbot-ca.crt /tmp�� nI_BWDnVB 9120902/b171/_x0-tests by/ce9cdaacc52f7340dcf9ee487e097b2c67f2d543876c7b925e05a5076d42d39c/log.json 64/src/net ap 64/pkg/tool/linu--verify ortcfg 5e05�� aw-mcpg/internal/launcher/connection_pool.go aw-mcpg/internal/launcher/launcher.go 64/pkg/tool/linux_amd64/link /server/auth.go tialization

• Ad-d 64/pkg/tool/linu-c 64/pkg/tool/linux_amd64/link` (dns block)

• Triggering command: /update-job-proxy /update-job-proxy DROP 671f1b40 test -e _and_Research_In--depth=1 /home/REDACTED/wororigin /opt/go/bin/testrefs/tags/v1.10.2:refs/tags/v1.10.2 64/pkg/tool/linu/usr/lib/git-core/git 671f1b40 table.d/chrony-o/home/dependabot/go/pkg/mod/cache/vcs/fb978822a4944776ca42509272a7325dad7bcbd80ced596132267b9bc8b28db8/shallow.lock test -e mSign_ECC_Root_C--pack_header=2,68 table.d/chrony-onoffline /bin/test rt 816b640640d473de--wait cli/dependabot/d-t test (dns block)
• gopkg.in
  • Triggering command: /update-job-proxy /update-job-proxy DROP 671f1b40 test -e _and_Research_In--depth=1 /home/REDACTED/wororigin /opt/go/bin/testrefs/tags/v1.10.2:refs/tags/v1.10.2 64/pkg/tool/linu/usr/lib/git-core/git 671f1b40 table.d/chrony-o/home/dependabot/go/pkg/mod/cache/vcs/fb978822a4944776ca42509272a7325dad7bcbd80ced596132267b9bc8b28db8/shallow.lock test -e mSign_ECC_Root_C--pack_header=2,68 table.d/chrony-onoffline /bin/test rt 816b640640d473de--wait cli/dependabot/d-t test (dns block)
• nonexistent.local
  • Triggering command: /tmp/go-build2119120902/b269/launcher.test /tmp/go-build2119120902/b269/launcher.test -test.testlogfile=/tmp/go-build2119120902/b269/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true go (dns block)
• this-host-does-not-exist-12345.com
  • Triggering command: /tmp/go-build2119120902/b278/mcp.test /tmp/go-build2119120902/b278/mcp.test -test.testlogfile=/tmp/go-build2119120902/b278/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true -c=4 -nolocalimports -importcfg /tmp/go-build2119120902/b245/importcfg -pack /home/REDACTED/go/pkg/mod/github.com/modelcontextprotocol/go-sdk@v1.1.0/jsonrpc/jsonrpc.go tmp/go-build conf�� go r 64/pkg/tool/linux_amd64/cgo (dns block)
  • Triggering command: /tmp/go-build3317713436/b278/mcp.test /tmp/go-build3317713436/b278/mcp.test -test.testlogfile=/tmp/go-build3317713436/b278/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true jIDs/HnGvDfcVk98iBtSvjIDs -fPIC x_amd64/vet -pthread -Wl,--no-gc-sectshow -aw-mcpg/ x_amd64/vet -ato�� pkg/mod/github.com/modelcontextp-errorsas -buildtags ache/go/1.25.6/x64/pkg/tool/linu-nilfunc -errorsas -ifaceassert -nilfunc ache/go/1.25.6/x64/pkg/tool/linu-tests (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

Original prompt

---

This section details on the original issue you should resolve

<issue_title>[duplicate-code] Duplicate Code Pattern: Logger Initialization Boilerplate</issue_title>
<issue_description># 🔍 Duplicate Code Pattern: Logger Initialization Boilerplate

Part of duplicate code analysis: #435

Summary

The three logger initialization functions (InitFileLogger, InitJSONLLogger, InitMarkdownLogger) follow nearly identical structural patterns with only minor variations in error handling and field setup. This creates ~60-75 lines of duplicated boilerplate code across the logger package.

Duplication Details

Pattern: Logger Initialization Functions

• Severity: High
• Occurrences: 3 instances (FileLogger, JSONLLogger, MarkdownLogger)
• Locations:
  • internal/logger/file_logger.go (lines 30-54, 25 lines)
  • internal/logger/jsonl_logger.go (lines 39-56, 18 lines)
  • internal/logger/markdown_logger.go (lines 28-48, 21 lines)

Code Structure (All Three Functions):

func Init*Logger(logDir, fileName string) error {
    // 1. Create logger struct
    *l := &*Logger{
        logDir:   logDir,
        fileName: fileName,
    }
    
    // 2. Call common initLogFile()
    file, err := initLogFile(logDir, fileName, <FLAGS>)
    if err != nil {
        // 3. ERROR HANDLING (differs per logger)
        // ...
    }
    
    // 4. Set logger-specific fields
    *l.logFile = file
    // ... additional setup
    
    // 5. Register as global logger
    initGlobal*Logger(*l)
    return nil
}

Specific Differences:

FileLogger (25 lines):

• Flags: os.O_APPEND
• Error handling: Fallback to stdout with warnings
• Additional setup: Creates log.Logger wrapper
• Prints success message

JSONLLogger (18 lines):

• Flags: os.O_APPEND
• Error handling: Returns error immediately (no fallback)
• Additional setup: Creates JSON encoder
• No success message

MarkdownLogger (21 lines):

• Flags: os.O_TRUNC
• Error handling: Sets fallback flag but no stdout redirect
• Additional setup: Sets initialized flag to false
• No success message

Impact Analysis

Maintainability

• Adding a new logger: Requires copying ~20-25 lines of boilerplate with subtle variations
• Changing initialization logic: Must update 3 separate functions
• Error handling inconsistency: Different error strategies make debugging difficult

Bug Risk

• Inconsistent behavior: FileLogger falls back to stdout, JSONLLogger fails hard, MarkdownLogger fails silently
• Easy to miss: When fixing a bug in one Init function, easy to forget the other two
• Testing burden: Each Init function needs separate test cases for the same logic

Code Bloat

• ~60 lines of duplicated logic that could be reduced to ~20 lines + configuration

Refactoring Recommendations

Option 1: Functional Options Pattern (Recommended)

Extract common initialization logic with customization via options:

// Common initialization function
func initLogger[T closableLogger](
    logDir, fileName string,
    flags int,
    setup func(*os.File) (T, error),
    onError func(error) (T, error),
) (T, error) {
    file, err := initLogFile(logDir, fileName, flags)
    if err != nil {
        return onError(err)
    }
    
    logger, err := setup(file)
    if err != nil {
        file.Close()
        var zero T
        return zero, err
    }
    
    return logger, nil
}

// Usage:
func InitFileLogger(logDir, fileName string) error {
    logger, err := initLogger(
        logDir, fileName, os.O_APPEND,
        func(file *os.File) (*FileLogger, error) {
            fl := &FileLogger{
                logDir: logDir, fileName: fileName,
                logFile: file,
                logger: log.New(file, "", 0),
            }
            return fl, nil
        },
        func(err error) (*FileLogger, error) {
            // Fallback to stdout
            fl := &FileLogger{
                logDir: logDir, fileName: fileName,
                useFallback: true,
                logger: log.New(os.Stdout, "", 0),
            }
            return fl, nil
        },
    )
    initGlobalFileLogger(logger)
    return err
}

Benefits:

• Reduces duplication by ~40 lines
• Maintains flexibility for logger-specific behavior
• Makes error handling strategies explicit
• Easier to test common logic

Estimated effort: 2-3 hours

• Implementation: 1.5 hours
• Testing: 1 hour
• Code review: 30 minutes

Option 2: Template Method with Interfaces

Define an interface for logger initialization and use a base implementation:

type LoggerInitializer interface {
    GetFlags() int
    SetupLogger(*os.File) error
    HandleInitError(error) error
}

func initializeLogger(init LoggerInitializer, logDir, fileName string) error {
    // Common initialization logic
}

Benefits:

• Traditional OOP approach, familiar pattern
• Clear separation of concerns

Drawbacks:

• More boil...

• Fixes [duplicate-code] Duplicate Code Pattern: Logger Initialization Boilerplate #436

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.