Refactor logging to use AsyncLocalStorage context instead of prop drilling

[acreeger]

Note to iloom

Execute in TRIVIAL mode due to plan being specified below:

Problem

When --json flag is used with CLI commands, all logging should go to stderr so that only valid JSON is output to stdout. The current implementation requires passing a custom logger through every layer of the call stack ("prop drilling"):

cli.ts → StartCommand → LoomManager → DatabaseManager → NeonProvider
                     → IssueTrackerFactory → GitHubService
                     → installDependencies()

This approach:

• Requires modifying constructors and method signatures throughout the codebase
• Is error-prone (easy to forget to pass the logger somewhere)
• Makes the code harder to maintain
• Couples unrelated components to logging concerns

Solution

Use Node.js's built-in AsyncLocalStorage to create a React Context-like pattern for logging. This allows any code in the async call stack to access the correct logger without explicit parameter passing.

Implementation Plan

1. Create logger context module

Create src/utils/logger-context.ts:

import { AsyncLocalStorage } from 'node:async_hooks'
import { logger as defaultLogger, type Logger } from './logger.js'

const loggerStorage = new AsyncLocalStorage<Logger>()

// Get current logger from context, or fall back to default
export function getLogger(): Logger {
  return loggerStorage.getStore() ?? defaultLogger
}

// Run code with a specific logger in context
export function withLogger<T>(logger: Logger, fn: () => T | Promise<T>): T | Promise<T> {
  return loggerStorage.run(logger, fn)
}

2. Update CLI entry point

Wrap command execution in withLogger() when --json flag is used:

// cli.ts
if (options.json) {
  const jsonLogger = createStderrLogger()
  await withLogger(jsonLogger, () => command.execute())
} else {
  await command.execute()
}

3. Update all logging call sites

Replace direct logger imports with getLogger() calls in:

• src/lib/GitHubService.ts
• src/lib/IssueTrackerFactory.ts
• src/lib/LoomManager.ts
• src/lib/DatabaseManager.ts
• src/lib/providers/NeonProvider.ts
• src/utils/package-manager.ts
• src/utils/neon-helpers.ts
• src/commands/start.ts
• src/commands/finish.ts
• src/commands/cleanup.ts
• Any other files using the logger

4. Remove prop drilling code

Revert/remove the custom logger parameters added to:

• Constructor parameters (logger?: Logger)
• Method parameters
• Instance properties (private logger: Logger)

5. Update tests

• Add tests for logger-context.ts
• Verify that getLogger() returns the correct logger in different contexts
• Test that withLogger() properly scopes the logger to the async context

Benefits

1. Cleaner API: No need to pass loggers through constructors/methods
2. Less error-prone: Can't forget to pass the logger
3. Easier to maintain: Adding logging to new code just requires getLogger()
4. Built-in: Uses Node.js native AsyncLocalStorage, no external dependencies
5. Familiar pattern: Similar to React Context for frontend developers

Files Changed (Current Prop Drilling Approach to Revert)

The following files were modified with the prop drilling approach and should be reverted/refactored:

• src/cli.ts - passes jsonLogger to IssueTrackerFactory
• src/commands/finish.ts - passes logger to installDependencies
• src/commands/start.ts - passes logger to createNeonProviderFromSettings
• src/lib/GitHubService.ts - added logger constructor param
• src/lib/IssueTrackerFactory.ts - added logger param to create()
• src/lib/LoomManager.ts - passes logger to installDependencies
• src/lib/providers/NeonProvider.ts - added logger constructor param
• src/utils/neon-helpers.ts - added logger param
• src/utils/package-manager.ts - added customLogger param

Testing

1. Run il start <issue> --json and verify only JSON goes to stdout
2. Run il start <issue> --json 2>/dev/null | jq . to confirm valid JSON
3. Run without --json and verify normal logging behavior

[acreeger]

Implementation Complete - Issue #307

Summary

Refactored logging to use AsyncLocalStorage context instead of prop drilling. Created a new logger-context.ts module that provides getLogger() and withLogger() functions, allowing any code in the async call stack to access the correct logger without explicit parameter passing.

Changes Made

• src/utils/logger-context.ts: New module with AsyncLocalStorage-based logger context
• src/cli.ts: Updated to wrap command execution in withLogger() for JSON mode
• Command files: Updated to use getLogger() and removed logger constructor params
• Lib modules: Updated all classes to use getLogger() and removed prop drilling

Validation Results

• Tests: 2781 passed | 1 skipped (2782)
• Typecheck: Passed
• Lint: Passed
• Build: Passed

---

Detailed Changes by File (click to expand)

Files Created

src/utils/logger-context.ts (NEW)

Purpose: AsyncLocalStorage-based logger context for JSON mode support

src/utils/logger-context.test.ts (NEW)

Purpose: Unit tests for logger context (9 test cases)

Files Modified

src/cli.ts

Changes: Wrap command execution in withLogger() when --json flag is used; removed logger parameter passing

src/commands/start.ts

Changes: Use getLogger() instead of this.logger; removed logger constructor param and prop drilling

src/commands/finish.ts

Changes: Use getLogger() instead of logger import

src/commands/cleanup.ts

Changes: Use getLogger() instead of logger import

src/commands/add-issue.ts

Changes: Use getLogger() instead of this.logger; removed logger constructor param

src/commands/enhance.ts

Changes: Use getLogger() instead of this.logger; removed logger constructor param

src/lib/GitHubService.ts

Changes: Use getLogger() instead of logger import

src/lib/IssueTrackerFactory.ts

Changes: Use getLogger() instead of logger import

src/lib/LoomManager.ts

Changes: Use getLogger(); removed logger constructor param and private property

src/lib/LoomLauncher.ts

Changes: Use getLogger(); removed logger constructor param and private property

src/lib/DatabaseManager.ts

Changes: Use getLogger(); removed logger constructor param and private property

src/lib/EnvironmentManager.ts

Changes: Use getLogger(); removed logger constructor param and private property

src/lib/CLIIsolationManager.ts

Changes: Use getLogger(); removed logger constructor param and private property

src/lib/MetadataManager.ts

Changes: Use getLogger(); removed logger constructor param and private property

src/lib/BranchNamingService.ts

Changes: Use getLogger(); removed logger option from constructor

src/lib/IssueEnhancementService.ts

Changes: Use getLogger(); removed logger constructor param and private property

src/lib/providers/NeonProvider.ts

Changes: Use getLogger() instead of logger import

src/utils/package-manager.ts

Changes: Use getLogger() instead of logger import

tests/lib/DatabaseManager.test.ts

Changes: Updated mock to include logger export for getLogger() compatibility

Dependencies Added

None