[duplicate-code] Duplicate Code Pattern: ServerFileLogger bypasses shared logger init helpers

[github-actions]

Summary

ServerFileLogger in internal/logger/server_file_logger.go manually reimplements file-initialization logic that is already available as shared helpers in the same package. Specifically, it duplicates initLogFile (from fileutil.go) and does not use the loggerFactory[T] generic pattern used by all five other logger types (FileLogger, JSONLLogger, MarkdownLogger, ToolsLogger, ObservedURLDomainsLogger).

Part of duplicate code analysis: #aw_parent_report1

Duplication Details

Pattern: Manual file-init and directory creation instead of shared helpers

• Severity: Medium
• Occurrences: 2 sites within server_file_logger.go
• Locations:
  • internal/logger/server_file_logger.go lines 38–48 (InitServerFileLogger manually calls os.MkdirAll)
  • internal/logger/server_file_logger.go lines 62–68 (getOrCreateLogger manually calls os.OpenFile)
  • internal/logger/fileutil.go lines 54–65 (initLogFile — the shared helper that already handles both steps)
• Code Sample (duplicated pattern in server_file_logger.go):

  // In InitServerFileLogger (server_file_logger.go:38)
  if err := os.MkdirAll(logDir, 0755); err != nil { ... }
  
  // In getOrCreateLogger (server_file_logger.go:62)
  file, err := os.OpenFile(logPath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644)

• Shared helper already exists (fileutil.go:initLogFile):

  func initLogFile(logDir, fileName string, flags int) (*os.File, error) {
      if err := os.MkdirAll(logDir, 0755); err != nil { ... }
      file, err := os.OpenFile(logPath, flags|os.O_CREATE|os.O_WRONLY, 0644)
      ...
  }

Additionally, InitServerFileLogger does not use the loggerFactory[T] pattern. All five other loggers (fileLoggerFactory, jsonlLoggerFactory, markdownLoggerFactory, toolsLoggerFactory, observedURLDomainsLoggerFactory) use a consistent setup/onError factory registered in global_state.go. ServerFileLogger has its own ad-hoc init with different fallback semantics.

Impact Analysis

• Maintainability: Changes to file init behavior (e.g., permissions, fallback strategy) must be applied in two places
• Bug Risk: The getOrCreateLogger method skips the MkdirAll step, assuming InitServerFileLogger already ran — a fragile implicit dependency that initLogFile would make explicit
• Consistency: New contributors familiar with other logger types will not expect ServerFileLogger to diverge from the factory pattern

Refactoring Recommendations

1. Use initLogFile in getOrCreateLogger

   • Replace the manual os.OpenFile call with initLogFile(sfl.logDir, fileName, os.O_APPEND)
   • Estimated effort: ~30 minutes
   • Benefits: single source of truth for file-init logic, automatic MkdirAll safety

2. Migrate InitServerFileLogger to loggerFactory[*ServerFileLogger]

   • Define a serverFileLoggerFactory var similar to the other five factories
   • Move the fallback logic into the onError field
   • Estimated effort: ~1 hour
   • Benefits: consistent initialization pattern across all logger types

Implementation Checklist

• Replace manual os.OpenFile in getOrCreateLogger with initLogFile
• Create a serverFileLoggerFactory variable using loggerFactory[*ServerFileLogger]
• Refactor InitServerFileLogger to delegate to the factory
• Update tests if needed
• Verify no functionality broken (especially fallback behavior)

Parent Issue

See parent analysis report: #aw_parent_report1

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

• awmgmcpg

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "awmgmcpg"

See Network Configuration for more information.

Generated by Duplicate Code Detector · 88.6 AIC · ⊞ 7.7K · ◷

• expires on Jul 17, 2026, 3:42 AM UTC