Skip to content

WIP: Fix duplicate logging#122

Merged
joshua-quek-sonarsource merged 1 commit into
mainfrom
contributors/joshuaquek/fix-98-additional-bug-fixes
Apr 21, 2026
Merged

WIP: Fix duplicate logging#122
joshua-quek-sonarsource merged 1 commit into
mainfrom
contributors/joshuaquek/fix-98-additional-bug-fixes

Conversation

@joshua-quek-sonarsource

@joshua-quek-sonarsource joshua-quek-sonarsource commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

Note

Low Risk
Low risk: moves enableFileLogging calls from command entrypoints into the underlying action handlers; main impact is log file initialization timing/naming for migrate, transfer, sync-metadata, and verify. Potential risk is only around unexpected logger transport state or log file paths if handlers are invoked in new ways.

Overview
Fixes duplicate/early log file setup by moving enableFileLogging() out of CLI command .action() blocks and into the corresponding handle*Action functions for migrate, transfer, sync-metadata, and verify.

Each action now enables file logging with a command-specific name (e.g. migrate, transfer, verify) just before logSystemInfo(), so file transports are configured once per run and after heap sizing is applied.

Reviewed by Cursor Bugbot for commit ad818c9. Bugbot is set up for automated code reviews on this repo. Configure here.

@joshua-quek-sonarsource
joshua-quek-sonarsource merged commit baf1920 into main Apr 21, 2026
1 of 2 checks passed
@sonar-review-alpha

sonar-review-alpha Bot commented Apr 21, 2026

Copy link
Copy Markdown

Summary

Problem: File logging was being enabled multiple times, causing duplicate log entries.

Solution: Moved enableFileLogging() calls from the top-level command handlers (index.js files) into the actual action handlers (handle-*-action.js files) where the work happens. This ensures logging is initialized exactly once per command execution.

Changes: Affected 4 commands:

  • migrate: enableFileLogging('migrate') moved from index.js → handle-migrate-action.js
  • sync-metadata: enableFileLogging('sync-metadata') moved from index.js → handle-sync-metadata-action.js
  • transfer: enableFileLogging('transfer') moved from index.js → handle-transfer-action.js
  • verify: enableFileLogging('verify') moved from index.js → handle-verify-action.js

Also made .debugging/test-migrate-verify.sh executable.

What reviewers should know

Review focus:

  • Verify that each command's logging setup is now called exactly once per execution
  • Check that enableFileLogging() is called early in the action handlers, right after ensureHeapSize() and before logSystemInfo()
  • Confirm the import changes are consistent: removed from index.js, added to helper files
  • Test a migration/sync/transfer/verify flow to confirm logging is not duplicated and files are created correctly

Pattern: All four commands follow the same refactor pattern—this is consistent and intentional, not a mistake.


  • Generate Walkthrough
  • Generate Diagram

🗣️ Give feedback

@joshua-quek-sonarsource
joshua-quek-sonarsource deleted the contributors/joshuaquek/fix-98-additional-bug-fixes branch April 21, 2026 19:29

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Reviewed by Cursor Bugbot for commit ad818c9. Configure here.

...(options.maxMemory && { maxMemoryMB: options.maxMemory }),
});
ensureHeapSize(perfConfig.maxMemoryMB);
enableFileLogging('transfer');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File logging enabled after early log messages and error-prone steps

Medium Severity

Moving enableFileLogging from the index files into the handler functions means it now runs after several logger.info calls and error-prone operations like config loading. In handle-transfer-action.js, two banner messages on lines 20–21 are emitted before enableFileLogging on line 33, so they won't appear in file logs. Similarly, banner messages in each command's index.js (e.g., '=== CloudVoyager - Full Organization Migration ===') are now logged before the handler even calls enableFileLogging. Worse, if config loading or ensureHeapSize throws, enableFileLogging is never reached, so the error logged by handleCommandError in the catch block won't be captured in any log file.

Additional Locations (2)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit ad818c9. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants