Consolidate logger initialization to use generic helpers (#815)

`ServerFileLogger` was manually implementing global state management
that other loggers already handled via generic helpers in
`global_helpers.go`. This created 23 lines of duplicate code and made
fallback behavior strategies unclear.

## Changes

**Extended generic helpers to ServerFileLogger**
- Added `*ServerFileLogger` to `closableLogger` constraint
- Replaced manual `initGlobalServerFileLogger()` and
`CloseServerFileLogger()` with calls to generic `initGlobalLogger()` and
`closeGlobalLogger()`
- Eliminates 23 lines of duplicate mutex handling and cleanup logic

**Documented fallback strategies**
Added 130-line "Initialization Pattern for Logger Types" section to
`common.go` explaining when each logger uses fallbacks:
- `FileLogger`: stdout fallback for critical operational logs
- `MarkdownLogger`: silent fallback for optional preview logs  
- `JSONLLogger`: strict error mode for structured data
- `ServerFileLogger`: unified logger fallback for per-server logs

## Example

Before (manual implementation):
```go
func initGlobalServerFileLogger(logger *ServerFileLogger) {
    globalServerLoggerMu.Lock()
    defer globalServerLoggerMu.Unlock()
    if globalServerFileLogger != nil {
        globalServerFileLogger.Close()
    }
    globalServerFileLogger = logger
}
```

After (using generic helper):
```go
func initGlobalServerFileLogger(logger *ServerFileLogger) {
    initGlobalLogger(&globalServerLoggerMu, &globalServerFileLogger, logger)
}
```

All loggers now use the same thread-safe initialization pattern.
Documentation provides clear guidance for adding new logger types.

> [!WARNING]
>
> <details>
> <summary>Firewall rules blocked me from connecting to one or more
addresses (expand for details)</summary>
>
> #### I tried to connect to the following addresses, but was blocked by
firewall rules:
>
> - `example.com`
> - Triggering command: `/tmp/go-build2696183341/b271/launcher.test
/tmp/go-build2696183341/b271/launcher.test
-test.testlogfile=/tmp/go-build2696183341/b271/testlog.txt
-test.paniconexit0 -test.timeout=10m0s -test.v=true 1636032/b166/_pkg_.a
go x_amd64/compile` (dns block)
> - `invalid-host-that-does-not-exist-12345.com`
> - Triggering command: `/tmp/go-build2696183341/b259/config.test
/tmp/go-build2696183341/b259/config.test
-test.testlogfile=/tmp/go-build2696183341/b259/testlog.txt
-test.paniconexit0 -test.timeout=10m0s -test.v=true 1636032/b153/_pkg_.a
.cfg x_amd64/vet` (dns block)
> - `nonexistent.local`
> - Triggering command: `/tmp/go-build2696183341/b271/launcher.test
/tmp/go-build2696183341/b271/launcher.test
-test.testlogfile=/tmp/go-build2696183341/b271/testlog.txt
-test.paniconexit0 -test.timeout=10m0s -test.v=true 1636032/b166/_pkg_.a
go x_amd64/compile` (dns block)
> - `slow.example.com`
> - Triggering command: `/tmp/go-build2696183341/b271/launcher.test
/tmp/go-build2696183341/b271/launcher.test
-test.testlogfile=/tmp/go-build2696183341/b271/testlog.txt
-test.paniconexit0 -test.timeout=10m0s -test.v=true 1636032/b166/_pkg_.a
go x_amd64/compile` (dns block)
> - `this-host-does-not-exist-12345.com`
> - Triggering command: `/tmp/go-build2696183341/b280/mcp.test
/tmp/go-build2696183341/b280/mcp.test
-test.testlogfile=/tmp/go-build2696183341/b280/testlog.txt
-test.paniconexit0 -test.timeout=10m0s -test.v=true
ache/go/1.25.6/x64/src/runtime/c-g .cfg x_amd64/vet` (dns block)
>
> If you need me to access, download, or install something from one of
these locations, you can either:
>
> - Configure [Actions setup
steps](https://gh.io/copilot/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](https://github.com/github/gh-aw-mcpg/settings/copilot/coding_agent)
(admins only)
>
> </details>

<!-- START COPILOT ORIGINAL PROMPT -->



<details>

<summary>Original prompt</summary>


----

*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: #797*

## Summary

Four logger files (`file_logger.go`, `jsonl_logger.go`,
`markdown_logger.go`, `server_file_logger.go`) share nearly identical
initialization patterns for file handling, error management, and global
state setup. This creates significant maintenance overhead and increases
the risk of inconsistent behavior across loggers.

## Duplication Details

### Pattern: Logger Initialization Structure
- **Severity**: High
- **Occurrences**: 4 instances (FileLogger, JSONLLogger, MarkdownLogger,
ServerFileLogger)
- **Locations**:
  - `internal/logger/file_logger.go` (lines 28-60, 63-68)
  - `internal/logger/jsonl_logger.go` (lines 38-66, 69-74)
  - `internal/logger/markdown_logger.go` (lines 28-54, 72-89)
  - `internal/logger/server_file_logger.go` (lines 70-114)

### Duplicated Components

**1. File Initialization Pattern** (~40 lines per logger)
Each logger implements nearly identical logic:
```go
func Init*Logger(logDir, fileName string) error {
    logger, err := initLogger(
        logDir, fileName, os.O_APPEND, // or os.O_TRUNC
        // Setup function
        func(file *os.File, logDir, fileName string) (*Logger, error) {
            // Create logger struct
            // Initialize fields
            // Return logger
        },
        // Error handler
        func(err error, logDir, fileName string) (*Logger, error) {
            // Fallback logic or error return
        },
    )
    // Initialize global logger
    return err
}
```

**2. Close Method Pattern** (~8 lines per logger)
```go
func (l *Logger) Close() error {
    l.mu.Lock()
    defer l.mu.Unlock()
    return closeLogFile(l.logFile, &l.mu, "loggerType")
}
```

**3. Global Logger State Pattern** (~12 lines per logger)
```go
var (
    global*Logger **Logger
    global*Mu     sync.RWMutex
)

func initGlobal*Logger(logger **Logger) {
    initGlobalLogger(&global*Mu, &global*Logger, logger)
}

func closeGlobal*Logger() error {
    return closeGlobalLogger(&global*Mu, &global*Logger)
}
```

### Code Sample: File Logger
```go
// InitFileLogger initializes the global file logger
func InitFileLogger(logDir, fileName string) error {
    logger, err := initLogger(
        logDir, fileName, os.O_APPEND,
        func(file *os.File, logDir, fileName string) (*FileLogger, error) {
            fl := &FileLogger{
                logDir:   logDir,
                fileName: fileName,
                logFile:  file,
                logger:   log.New(file, "", 0),
            }
            log.Printf("Logging to file: %s", filepath.Join(logDir, fileName))
            return fl, nil
        },
        func(err error, logDir, fileName string) (*FileLogger, error) {
            // Fallback to stdout...
            return fl, nil
        },
    )
    initGlobalFileLogger(logger)
    return err
}
```

Similar structures exist in all 4 logger files with only minor
variations.

## Impact Analysis

### Maintainability
- **High Risk**: Changes to initialization logic must be replicated
across 4 files
- **Inconsistency**: Different error handling strategies across loggers
(some fallback to stdout, others return errors)
- **Code Bloat**: ~120+ lines of similar initialization code

### Bug Risk
- **Medium Risk**: Bug fixes must be applied to all 4 loggers
independently
- **Historical Issues**: The codebase already recognized this pattern
and created `common.go` with `initLogger()` helper, but only partial
consolidation occurred
- **Fallback Inconsistency**: FileLogger/MarkdownLogger fallback to
stdout, JSONLLogger doesn't - easy to miss

### Code Bloat
- **Total Lines**: ~120+ lines of duplicated initialization patterns
- **Percentage**: ~16% of logger package code (727 total lines across 4
files)

## Refactoring Recommendations

### 1. Extract Logger Factory Pattern
**Effort**: Medium (4-6 hours)

Create a generic logger factory that handles common initialization:

```go
// LoggerConfig defines common logger configuration
type LoggerConfig struct {
    LogDir        string
    FileName      string
    FileFlags     int  // os.O_APPEND, os.O_TRUNC, etc.
    AllowFallback bool // Whether to fallback to stdout on error
}

// LoggerFactory creates and initializes loggers with common patterns
type LoggerFactory[T closableLogger] struct {
    config LoggerConfig
}

func (f *LoggerFactory[T]) Create(
    setupFunc func(*os.File, string, string) (T, error),
    fallbackFunc func(error, string, string) (T, error),
) (T, error) {
    // Common initialization logic
    // Replaces duplicate initLogger calls
}
```

**Benefits**:
- Centralized file handling and error management
- Consistent fallback behavior
- Reduces initialization code by ~80 lines
- Single source of trut...

</details>


> **Custom agent used: agentic-workflows**
> GitHub Agentic Workflows (gh-aw) - Create, debug, and upgrade
AI-powered workflows with intelligent prompt routing



<!-- START COPILOT CODING AGENT SUFFIX -->

- Fixes #798

<!-- START COPILOT CODING AGENT TIPS -->
---

💡 You can make Copilot smarter by setting up custom instructions,
customizing its development environment and configuring Model Context
Protocol (MCP) servers. Learn more [Copilot coding agent
tips](https://gh.io/copilot-coding-agent-tips) in the docs.

Author: lpcox

internal/logger/common.go

@@ -72,6 +72,128 @@ import (
 //
 // When adding a new logger type, follow this pattern to ensure consistent behavior.
 
+// Initialization Pattern for Logger Types
+//
+// The logger package follows a consistent initialization pattern across all logger types,
+// with support for customizable fallback behavior. This section documents the pattern
+// and explains the different fallback strategies used by each logger type.
+//
+// Standard Initialization Pattern:
+//
+// All logger types use the initLogger() generic helper function for initialization:
+//
+//	func Init*Logger(logDir, fileName string) error {
+//	    logger, err := initLogger(
+//	        logDir, fileName, fileFlags,
+//	        setupFunc,    // Configure logger after file is opened
+//	        errorHandler, // Handle initialization failures
+//	    )
+//	    initGlobal*Logger(logger)
+//	    return err
+//	}
+//
+// The initLogger() helper:
+//  1. Attempts to create the log directory (if needed)
+//  2. Opens the log file with specified flags (os.O_APPEND, os.O_TRUNC, etc.)
+//  3. Calls setupFunc to configure the logger instance
+//  4. On error, calls errorHandler to implement fallback behavior
+//  5. Returns the initialized logger and any error
+//
+// Fallback Behavior Strategies:
+//
+// Different logger types implement different fallback strategies based on their purpose:
+//
+// 1. FileLogger - Stdout Fallback:
+//    - Purpose: Operational logs must always be visible
+//    - Fallback: Redirects to stdout if log directory/file creation fails
+//    - Error: Returns nil (never fails, always provides output)
+//    - Use case: Critical operational messages that must be seen
+//
+//    Example error handler:
+//    func(err error, logDir, fileName string) (*FileLogger, error) {
+//        log.Printf("WARNING: Failed to initialize log file: %v", err)
+//        log.Printf("WARNING: Falling back to stdout for logging")
+//        return &FileLogger{
+//            logDir:      logDir,
+//            fileName:    fileName,
+//            useFallback: true,
+//            logger:      log.New(os.Stdout, "", 0),
+//        }, nil
+//    }
+//
+// 2. MarkdownLogger - Silent Fallback:
+//    - Purpose: GitHub workflow preview logs (optional enhancement)
+//    - Fallback: Sets useFallback flag, produces no output
+//    - Error: Returns nil (never fails, silently disables)
+//    - Use case: Nice-to-have logs that shouldn't block operations
+//
+//    Example error handler:
+//    func(err error, logDir, fileName string) (*MarkdownLogger, error) {
+//        return &MarkdownLogger{
+//            logDir:      logDir,
+//            fileName:    fileName,
+//            useFallback: true,
+//        }, nil
+//    }
+//
+// 3. JSONLLogger - Strict Mode:
+//    - Purpose: Machine-readable RPC message logs for analysis
+//    - Fallback: None - returns error immediately
+//    - Error: Returns error to caller
+//    - Use case: Structured data that requires file storage
+//
+//    Example error handler:
+//    func(err error, logDir, fileName string) (*JSONLLogger, error) {
+//        return nil, err
+//    }
+//
+// 4. ServerFileLogger - Unified Fallback:
+//    - Purpose: Per-server log files for troubleshooting
+//    - Fallback: Sets useFallback flag, logs to unified logger only
+//    - Error: Returns nil (never fails, falls back to unified logging)
+//    - Use case: Per-server logs are helpful but not required
+//
+//    Note: ServerFileLogger doesn't use initLogger() because it creates
+//    files on-demand, but follows the same fallback philosophy.
+//
+// Global Logger Management:
+//
+// After initialization, all logger types register themselves as global loggers
+// using the generic initGlobal*Logger() helpers from global_helpers.go:
+//
+//  - initGlobalFileLogger()
+//  - initGlobalJSONLLogger()
+//  - initGlobalMarkdownLogger()
+//  - initGlobalServerFileLogger()
+//
+// These helpers ensure thread-safe initialization with proper cleanup of any
+// existing logger instance.
+//
+// When to Use Each Logger Type:
+//
+// - FileLogger: Required operational logs (startup, errors, warnings)
+// - MarkdownLogger: Optional GitHub workflow preview logs
+// - JSONLLogger: Structured RPC message logs for tooling/analysis
+// - ServerFileLogger: Per-server troubleshooting logs
+//
+// Adding a New Logger Type:
+//
+// When adding a new logger type:
+//  1. Implement Close() method following the Close Pattern (above)
+//  2. Add type to closableLogger constraint in global_helpers.go
+//  3. Use initLogger() for initialization with appropriate fallback strategy
+//  4. Add initGlobal*Logger() and closeGlobal*Logger() helpers
+//  5. Document the fallback strategy and use case
+//
+// This consistent pattern ensures:
+//  - Predictable behavior across all loggers
+//  - Easy to understand fallback strategies
+//  - Minimal code duplication
+//  - Type-safe global logger management
+//
+// See file_logger.go, jsonl_logger.go, markdown_logger.go, and server_file_logger.go
+// for complete implementation examples.
+
 // closeLogFile is a common helper for closing log files with consistent error handling.
 // It syncs buffered data before closing and handles errors appropriately.
 // The mutex should already be held by the caller.
@@ -148,7 +270,7 @@ type loggerErrorHandlerFunc[T closableLogger] func(err error, logDir, fileName s
 //   - fileName: Name of the log file
 //   - flags: File opening flags (e.g., os.O_APPEND, os.O_TRUNC)
 //   - setup: Function to configure the logger after the file is opened
-//   - onError: Function to handle initialization errors (can return fallback or error)
+//   - onError: Function to handle initialization errors (implements fallback strategy)
 //
 // Returns:
 //   - T: The initialized logger instance
@@ -157,7 +279,13 @@ type loggerErrorHandlerFunc[T closableLogger] func(err error, logDir, fileName s
 // This function:
 //  1. Attempts to open the log file with the specified flags
 //  2. If successful, calls the setup function to configure the logger
-//  3. If unsuccessful, calls the error handler to decide on fallback behavior
+//  3. If unsuccessful, calls the error handler to implement the logger's fallback strategy
+//
+// The onError handler determines the fallback behavior. See "Initialization Pattern for Logger Types"
+// documentation above for details on fallback strategies:
+//   - FileLogger: Falls back to stdout
+//   - MarkdownLogger: Silent fallback (no output)
+//   - JSONLLogger: Returns error (no fallback)
 func initLogger[T closableLogger](
 	logDir, fileName string,
 	flags int,

internal/logger/global_helpers.go

@@ -17,9 +17,9 @@ package logger
 import "sync"
 
 // closableLogger is a constraint for types that have a Close method.
-// This is satisfied by *FileLogger, *JSONLLogger, and *MarkdownLogger.
+// This is satisfied by *FileLogger, *JSONLLogger, *MarkdownLogger, and *ServerFileLogger.
 type closableLogger interface {
-	*FileLogger | *JSONLLogger | *MarkdownLogger
+	*FileLogger | *JSONLLogger | *MarkdownLogger | *ServerFileLogger
 	Close() error
 }
 
@@ -113,3 +113,13 @@ func initGlobalMarkdownLogger(logger *MarkdownLogger) {
 func closeGlobalMarkdownLogger() error {
 	return closeGlobalLogger(&globalMarkdownMu, &globalMarkdownLogger)
 }
+
+// initGlobalServerFileLogger initializes the global ServerFileLogger using the generic helper.
+func initGlobalServerFileLogger(logger *ServerFileLogger) {
+	initGlobalLogger(&globalServerLoggerMu, &globalServerFileLogger, logger)
+}
+
+// closeGlobalServerFileLogger closes the global ServerFileLogger using the generic helper.
+func closeGlobalServerFileLogger() error {
+	return closeGlobalLogger(&globalServerLoggerMu, &globalServerFileLogger)
+}

internal/logger/server_file_logger.go

@@ -212,24 +212,5 @@ func LogDebugWithServer(serverID, category, format string, args ...interface{})
 
 // CloseServerFileLogger closes the global server file logger
 func CloseServerFileLogger() error {
-	globalServerLoggerMu.Lock()
-	defer globalServerLoggerMu.Unlock()
-
-	if globalServerFileLogger != nil {
-		err := globalServerFileLogger.Close()
-		globalServerFileLogger = nil
-		return err
-	}
-	return nil
-}
-
-// Helper function for initializing the global server file logger
-func initGlobalServerFileLogger(logger *ServerFileLogger) {
-	globalServerLoggerMu.Lock()
-	defer globalServerLoggerMu.Unlock()
-
-	if globalServerFileLogger != nil {
-		globalServerFileLogger.Close()
-	}
-	globalServerFileLogger = logger
+	return closeGlobalServerFileLogger()
 }