Merge remote-tracking branch 'origin/fix/console-log-policy' into fix/DX-10224-bulk-error-visibility

Author: harshithad0703

.talismanrc

@@ -5,3 +5,9 @@ fileignoreconfig:
   version: '1.0'
 - filename: .github/workflows/release-production-pipeline.yml
   checksum: dd858a2c2a3297c5651c2843ddae73ad0776a0386329200d01b051ddc489871e
+- filename: packages/contentstack-utilities/src/progress-summary/cli-progress-manager.ts
+  checksum: be7e833dc0abbbb211dc71f7970d8a1c680860f978dc2b871bcc9a30a659f436
+- filename: packages/contentstack-utilities/test/unit/logger.test.ts
+  checksum: ca0bbc2838a0b6a069d8fc4874e465dd30dd35377a6202eb005eb43e5ff5d871
+- filename: packages/contentstack-utilities/test/unit/cliProgressManager.test.ts
+  checksum: 6df6c21e1188b077a83a0464aecc4c628941a3fbb3bdea99db18d30b5bd3cf71

packages/contentstack-utilities/.mocharc.json

@@ -1,6 +1,7 @@
 {
     "require": [
       "test/helpers/init.js",
+      "test/helpers/mock-ora.js",
       "ts-node/register",
       "source-map-support/register",
       "test/helpers/mocha-root-hooks.js"

packages/contentstack-utilities/src/constants/logging.ts

@@ -15,5 +15,3 @@ export const levelColors = {
   info: 'white',
   debug: 'blue',
 };
-
-export const PROGRESS_SUPPORTED_MODULES = ['export', 'import', 'audit', 'import-setup', 'clone', 'bulk-operations'] as const;

packages/contentstack-utilities/src/helpers.ts

@@ -255,12 +255,16 @@ const sensitiveKeys = [
   /delivery[-._]?token/i,
 ];
 
+/**
+ * @deprecated No-op, kept only so the plugins that still call it keep compiling.
+ *
+ * Console visibility is no longer derived from `log.progressSupportedModule` — it is a
+ * process-wide policy resolved once by the core CLI's `console-policy` init hook (see
+ * `logger/console-policy.ts`), so there is nothing left to clear. Remove this export a
+ * release after the plugin call sites are gone.
+ */
 export function clearProgressModuleSetting(): void {
-  const logConfig = configHandler.get('log') || {};
-  if (logConfig?.progressSupportedModule) {
-    delete logConfig.progressSupportedModule;
-    configHandler.set('log', logConfig);
-  }
+  // Intentionally empty.
 }
 
 /**

packages/contentstack-utilities/src/index.ts

@@ -82,6 +82,10 @@ export type { ChalkInstance } from './chalk';
 export { Logger };
 export { default as authenticationHandler } from './authentication-handler';
 export { v2Logger as log, cliErrorHandler, handleAndLogError, getLogPath, getSessionLogPath } from './logger/log';
+// NOTE Only the reader is exported. `setConsoleLogPolicy` stays off the index so that
+// nothing downstream of the core CLI's `console-policy` init hook can override the
+// decision — the hook imports the setter by deep path.
+export { isConsoleLogEnabled } from './logger/console-policy';
 export {
   CLIProgressManager,
   SummaryManager,

packages/contentstack-utilities/src/interfaces/index.ts

@@ -140,6 +140,11 @@ export interface ProcessProgress {
 }
 
 export interface ProgressManagerOptions {
+  /**
+   * Defaults to the process-wide console-log policy (`isConsoleLogEnabled()`), which is
+   * what production code should rely on. Pass it explicitly only to drive the two modes
+   * directly, e.g. from tests.
+   */
   showConsoleLogs?: boolean;
   total?: number;
   moduleName?: string;

packages/contentstack-utilities/src/logger/console-policy.ts

@@ -0,0 +1,37 @@
+/**
+ * Console-log policy.
+ *
+ * Whether the CLI writes log lines to the console is a single process-wide
+ * decision, resolved once from static inputs (env var → user config → the
+ * plugin's `csdxConfig.showConsoleLogs` declaration → `false`) by the
+ * `console-policy` init hook in the core CLI, before any command code runs.
+ *
+ * The default is `false`: the logger writes diagnostics to files and knows
+ * nothing about the screen. Console output is an opt-in verbosity feature, and
+ * because the progress UI and the log stream are two consumers of one terminal,
+ * enabling it also turns the progress UI off (see `CLIProgressManager`).
+ *
+ * This module holds no disk state, so it is free to consult per message. The
+ * decision is deliberately *not* frozen on first read — freezing a value that
+ * arrives late is the bug this policy replaces.
+ *
+ * `setConsoleLogPolicy` is intentionally absent from the package index. The core
+ * CLI imports it by deep path (`@contentstack/cli-utilities/lib/logger/console-policy`);
+ * a plugin importing from the index has no setter to call, which makes "nothing
+ * downstream may override the policy" structural rather than a runtime lock.
+ */
+
+let enabled = false;
+
+export function setConsoleLogPolicy(value: boolean): void {
+  enabled = value;
+}
+
+export function isConsoleLogEnabled(): boolean {
+  return enabled;
+}
+
+/** Test-only: restore the default (files-only) policy between cases. */
+export function resetConsoleLogPolicy(): void {
+  enabled = false;
+}

packages/contentstack-utilities/src/logger/logger.ts

@@ -2,10 +2,10 @@ import traverse from 'traverse';
 import { klona } from 'klona/full';
 import { normalize } from 'path';
 import * as winston from 'winston';
-import { levelColors, logLevels, PROGRESS_SUPPORTED_MODULES } from '../constants/logging';
+import { levelColors, logLevels } from '../constants/logging';
 import { LoggerConfig, LogLevel, LogType } from '../interfaces/index';
-import { configHandler } from '..';
 import { getSessionLogPath } from './session-path';
+import { isConsoleLogEnabled } from './console-policy';
 
 export default class Logger {
   private loggers: Record<string, winston.Logger>;
@@ -69,39 +69,31 @@ export default class Logger {
       }),
     ];
 
-    // Determine console logging based on configuration
-    let showConsoleLogs = true;
-    if (configHandler && typeof configHandler.get === 'function') {
-      const logConfig = configHandler.get('log') || {};
-      const currentModule = logConfig.progressSupportedModule;
-      const hasProgressSupport = currentModule && PROGRESS_SUPPORTED_MODULES.includes(currentModule);
-
-      if (hasProgressSupport) {
-        // Plugin has progress bars - respect user's explicit setting, or default to false (show progress bars)
-        showConsoleLogs = logConfig.showConsoleLogs ?? false;
-      } else {
-        // Plugin doesn't have progress support - always show console logs
-        showConsoleLogs = true;
-      }
-    }
-
-    if (showConsoleLogs) {
-      transports.push(
-        new winston.transports.Console({
-          format: winston.format.combine(
-            winston.format.timestamp({ format: 'YYYY-MM-DD HH:mm:ss' }),
-            winston.format.printf((info) => {
-              // Apply full redaction for console (user-facing)
-              const redactedInfo = this.redact(info, true);
-              const colorizer = winston.format.colorize();
-              const levelText = redactedInfo.level.toUpperCase();
-              const { timestamp, message } = redactedInfo;
-              return colorizer.colorize(redactedInfo.level, `[${timestamp}] ${levelText}: ${message}`);
-            }),
-          ),
-        }),
-      );
-    }
+    // The Console transport is always attached; whether it emits is decided per
+    // message by the console-log policy, never by the transport list. The filter
+    // below is the FIRST format in the chain so that a `false` return short-circuits
+    // the write before timestamp/printf run.
+    //
+    // It must return `info` (not `{}`) on the enabled path: logform's `combine` feeds
+    // each format's return value into the next, so returning a fresh object would
+    // *replace* `info` and the printf below would emit blank lines instead of the
+    // message. Returning `false` is winston-transport's documented skip signal.
+    transports.push(
+      new winston.transports.Console({
+        format: winston.format.combine(
+          winston.format((info) => (isConsoleLogEnabled() ? info : false))(),
+          winston.format.timestamp({ format: 'YYYY-MM-DD HH:mm:ss' }),
+          winston.format.printf((info) => {
+            // Apply full redaction for console (user-facing)
+            const redactedInfo = this.redact(info, true);
+            const colorizer = winston.format.colorize();
+            const levelText = redactedInfo.level.toUpperCase();
+            const { timestamp, message } = redactedInfo;
+            return colorizer.colorize(redactedInfo.level, `[${timestamp}] ${levelText}: ${message}`);
+          }),
+        ),
+      }),
+    );
 
     return winston.createLogger({
       levels: logLevels,

packages/contentstack-utilities/src/progress-summary/cli-progress-manager.ts

@@ -3,8 +3,8 @@ import ora, { Ora } from 'ora';
 import ProgressBar from 'cli-progress';
 import SummaryManager from './summary-manager';
 import { ProcessProgress, ProgressManagerOptions, Failure } from '../interfaces';
-import { configHandler } from '..';
 import { ProgressStrategyRegistry } from './progress-strategy';
+import { isConsoleLogEnabled } from '../logger/console-policy';
 
 interface ProgressCallback {
   onModuleStart?: (moduleName: string) => void;
@@ -37,7 +37,7 @@ export default class CLIProgressManager {
   private branchName: string;
 
   constructor({
-    showConsoleLogs = false,
+    showConsoleLogs = isConsoleLogEnabled(),
     total = 0,
     moduleName = 'Module',
     enableNestedProgress = false,
@@ -70,7 +70,7 @@ export default class CLIProgressManager {
     CLIProgressManager.globalSummary = new SummaryManager({ operationName, context: { branchName } });
 
     // Only show header if console logs are disabled (progress UI mode)
-    if (!configHandler.get('log')?.showConsoleLogs) {
+    if (!isConsoleLogEnabled()) {
       CLIProgressManager.displayOperationHeader(branchName, headerTitle);
     }
 
@@ -167,32 +167,44 @@ export default class CLIProgressManager {
 
   /**
    * Create a simple progress manager (no nested processes)
+   *
+   * @param _showConsoleLogs Deprecated and ignored. Console visibility is a process-wide
+   *   policy (`isConsoleLogEnabled()`), which the constructor reads itself — a caller must
+   *   not be able to override it. The parameter is retained only so the plugins still
+   *   passing it keep compiling, and is removed once those call sites are gone.
    */
-  static createSimple(moduleName: string, total?: number, showConsoleLogs = false): CLIProgressManager {
+  static createSimple(moduleName: string, total?: number, _showConsoleLogs?: boolean): CLIProgressManager {
     return new CLIProgressManager({
       moduleName: moduleName.toUpperCase(),
       total: total || 0,
-      showConsoleLogs,
       enableNestedProgress: false,
     });
   }
 
   /**
    * Create a nested progress manager (with sub-processes)
+   *
+   * @param _showConsoleLogs Deprecated and ignored — see `createSimple`.
    */
-  static createNested(moduleName: string, showConsoleLogs = false): CLIProgressManager {
+  static createNested(moduleName: string, _showConsoleLogs?: boolean): CLIProgressManager {
     return new CLIProgressManager({
       moduleName: moduleName.toUpperCase(),
       total: 0,
-      showConsoleLogs,
       enableNestedProgress: true,
     });
   }
 
   /**
-   * Show a loading spinner before initializing progress
+   * Show a loading spinner while an async operation runs.
+   *
+   * A spinner is part of the progress UI, so it is skipped entirely when console logs own
+   * the terminal — callers do not need to branch on the policy themselves.
    */
   static async withLoadingSpinner<T>(message: string, asyncOperation: () => Promise<T>): Promise<T> {
+    if (isConsoleLogEnabled()) {
+      return asyncOperation();
+    }
+
     const spinner = ora(message).start();
     try {
       const result = await asyncOperation();
@@ -288,6 +300,10 @@ export default class CLIProgressManager {
   }
 
   private initializeProgress(): void {
+    // Console logs and the progress UI are mutually exclusive: exactly one of them owns
+    // the terminal. When console logging is on, no renderer is created at all — not
+    // suppressed, not buffered, never constructed. Everything below is therefore reached
+    // only in progress-UI mode.
     if (this.showConsoleLogs) {
       return;
     }
@@ -305,13 +321,9 @@ export default class CLIProgressManager {
         ProgressBar.Presets.shades_classic,
       );
 
-      if (!this.showConsoleLogs) {
-        console.log(getChalk().bold.cyan(`\n${this.moduleName}:`));
-      }
+      console.log(getChalk().bold.cyan(`\n${this.moduleName}:`));
     } else if (this.total > 0) {
-      if (!this.showConsoleLogs) {
-        console.log(getChalk().bold.cyan(`\n${this.moduleName}:`));
-      }
+      console.log(getChalk().bold.cyan(`\n${this.moduleName}:`));
 
       this.progressBar = new ProgressBar.SingleBar({
         format: ' {label} |' + getChalk().cyan('{bar}') + '| {percentage}% | {value}/{total} | {status}',

packages/contentstack-utilities/test/helpers/mock-ora.js

@@ -0,0 +1,37 @@
+/**
+ * Install the `ora` mock before any test file is loaded.
+ *
+ * Mocha loads every test file up front, so a `Module.prototype.require` interception set up
+ * inside one test file is already too late: an alphabetically earlier file will have pulled
+ * `cli-progress-manager` — and with it the real `ora` — into the require cache, and the
+ * module-scope binding it captured can no longer be replaced. Intercepting here, from a
+ * `--require` entry, is the only point that runs before all of them, which is what makes
+ * spinner assertions deterministic whether a file runs alone or as part of the suite.
+ */
+const sinon = require('sinon');
+const Module = require('module');
+
+const mockOraInstance = {
+  start: sinon.stub().returnsThis(),
+  stop: sinon.stub().returnsThis(),
+  succeed: sinon.stub().returnsThis(),
+  fail: sinon.stub().returnsThis(),
+  warn: sinon.stub().returnsThis(),
+  info: sinon.stub().returnsThis(),
+  text: '',
+  color: 'cyan',
+  isSpinning: false,
+};
+
+const mockOra = sinon.stub().returns(mockOraInstance);
+mockOra.promise = sinon.stub().returns(mockOraInstance);
+
+const originalRequire = Module.prototype.require;
+Module.prototype.require = function (id) {
+  if (id === 'ora') {
+    return mockOra;
+  }
+  return originalRequire.apply(this, arguments);
+};
+
+module.exports = { mockOra, mockOraInstance };