refactor: emit conflicting-flags warning once at CLI parsing time

Address review findings: BaseLogger.configure no longer warns on
verbose+silent — with the logger now re-configured from resolved config
values, the warning fired up to four times, leaked raw console.warn into
--json mode via the fallbackLogger, and mislabeled a config-file silent
as a CLI flag conflict. The warning moved to warnOnConflictingFlags,
called once in wrapCommand with JSON-mode suppression. Silent still
always wins over verbose regardless of source. Also restore
fallbackLogger state in tests and document the process-global caveat.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: cm-dyoshikawa

src/cli/wrap-command.ts

@@ -2,7 +2,13 @@ import { Command } from "commander";
 
 import { CLIError } from "../types/json-output.js";
 import { formatError } from "../utils/error.js";
-import { ConsoleLogger, fallbackLogger, JsonLogger, Logger } from "../utils/logger.js";
+import {
+  ConsoleLogger,
+  fallbackLogger,
+  JsonLogger,
+  Logger,
+  warnOnConflictingFlags,
+} from "../utils/logger.js";
 
 export function createLogger({
   name,
@@ -57,6 +63,7 @@ export function wrapCommand({
       verbose: Boolean(globalOpts.verbose) || Boolean(options.verbose),
       silent: Boolean(globalOpts.silent) || Boolean(options.silent),
     };
+    warnOnConflictingFlags({ ...cliLoggerOptions, jsonMode: logger.jsonMode });
     logger.configure(cliLoggerOptions);
     fallbackLogger.configure(cliLoggerOptions);
 

src/config/config-resolver.test.ts

@@ -173,6 +173,7 @@ describe("config-resolver", () => {
         expect(fallbackConfigureSpy).toHaveBeenCalledWith({ verbose: false, silent: true });
       } finally {
         fallbackConfigureSpy.mockRestore();
+        fallbackLogger.configure({ verbose: false, silent: false });
       }
     });
 
@@ -185,12 +186,16 @@ describe("config-resolver", () => {
       await writeFileContent(join(testDir, "rulesync.jsonc"), configContent);
       const logger = { warn: vi.fn(), configure: vi.fn() } as unknown as Logger;
 
-      await ConfigResolver.resolve(
-        { configPath: join(testDir, "rulesync.jsonc"), silent: true, verbose: true },
-        { logger },
-      );
+      try {
+        await ConfigResolver.resolve(
+          { configPath: join(testDir, "rulesync.jsonc"), silent: true, verbose: true },
+          { logger },
+        );
 
-      expect(logger.configure).toHaveBeenCalledWith({ verbose: true, silent: true });
+        expect(logger.configure).toHaveBeenCalledWith({ verbose: true, silent: true });
+      } finally {
+        fallbackLogger.configure({ verbose: false, silent: false });
+      }
     });
 
     it("should enable logger verbose from config-file verbose", async () => {
@@ -201,9 +206,13 @@ describe("config-resolver", () => {
       await writeFileContent(join(testDir, "rulesync.jsonc"), configContent);
       const logger = { warn: vi.fn(), configure: vi.fn() } as unknown as Logger;
 
-      await ConfigResolver.resolve({ configPath: join(testDir, "rulesync.jsonc") }, { logger });
+      try {
+        await ConfigResolver.resolve({ configPath: join(testDir, "rulesync.jsonc") }, { logger });
 
-      expect(logger.configure).toHaveBeenCalledWith({ verbose: true, silent: false });
+        expect(logger.configure).toHaveBeenCalledWith({ verbose: true, silent: false });
+      } finally {
+        fallbackLogger.configure({ verbose: false, silent: false });
+      }
     });
 
     it("should not touch the fallbackLogger when no logger is supplied", async () => {

src/config/config-resolver.ts

@@ -295,7 +295,10 @@ export class ConfigResolver {
     // still win via `pick`. Only re-configure when the caller threaded a
     // logger through — those call sites pass their CLI flags in the params, so
     // precedence stays intact. The shared `fallbackLogger` is kept in sync for
-    // paths that have no logger threaded through.
+    // paths that have no logger threaded through. Note: `fallbackLogger` is
+    // process-global state — do not call `resolve` with a logger from a
+    // long-lived process (e.g. the MCP server) where one repository's config
+    // would leak into unrelated later operations.
     const resolvedVerbose = pick({
       cli: verbose,
       file: configByFile.verbose,

src/utils/logger.test.ts

@@ -1,7 +1,13 @@
 import { beforeEach, describe, expect, it, vi } from "vitest";
 
 import type { Logger } from "./logger.js";
-import { ConsoleLogger, fallbackLogger, JsonLogger, warnWithFallback } from "./logger.js";
+import {
+  ConsoleLogger,
+  fallbackLogger,
+  JsonLogger,
+  warnOnConflictingFlags,
+  warnWithFallback,
+} from "./logger.js";
 
 // Mock vitest module
 vi.mock("./vitest.js", () => ({
@@ -65,14 +71,14 @@ describe("ConsoleLogger", () => {
   });
 
   describe("configure()", () => {
-    it("should warn when both verbose and silent are enabled", () => {
+    it("should not warn when both verbose and silent are enabled (warning lives in warnOnConflictingFlags)", () => {
       const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
 
       logger.configure({ verbose: true, silent: true });
 
-      expect(warnSpy).toHaveBeenCalledWith(
-        "Both --verbose and --silent specified; --silent takes precedence",
-      );
+      expect(warnSpy).not.toHaveBeenCalled();
+      expect(logger.silent).toBe(true);
+      expect(logger.verbose).toBe(false);
 
       warnSpy.mockRestore();
     });
@@ -443,3 +449,44 @@ describe("warnWithFallback", () => {
     }
   });
 });
+
+describe("warnOnConflictingFlags", () => {
+  it("warns once when both flags are set outside JSON mode", () => {
+    const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
+
+    try {
+      warnOnConflictingFlags({ verbose: true, silent: true, jsonMode: false });
+
+      expect(warnSpy).toHaveBeenCalledExactlyOnceWith(
+        "Both --verbose and --silent specified; --silent takes precedence",
+      );
+    } finally {
+      warnSpy.mockRestore();
+    }
+  });
+
+  it("does not warn in JSON mode", () => {
+    const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
+
+    try {
+      warnOnConflictingFlags({ verbose: true, silent: true, jsonMode: true });
+
+      expect(warnSpy).not.toHaveBeenCalled();
+    } finally {
+      warnSpy.mockRestore();
+    }
+  });
+
+  it("does not warn when only one flag is set", () => {
+    const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
+
+    try {
+      warnOnConflictingFlags({ verbose: true, silent: false, jsonMode: false });
+      warnOnConflictingFlags({ verbose: false, silent: true, jsonMode: false });
+
+      expect(warnSpy).not.toHaveBeenCalled();
+    } finally {
+      warnSpy.mockRestore();
+    }
+  });
+});

src/utils/logger.ts

@@ -46,20 +46,15 @@ abstract class BaseLogger {
     return this._silent;
   }
 
+  // Silent always wins over verbose, regardless of where each value came
+  // from (CLI flag or config file). The user-facing warning about the
+  // conflicting CLI flags lives in `warnOnConflictingFlags`, emitted once at
+  // CLI-flag parsing time — not here, since `configure` may be called again
+  // with config-file-derived values.
   configure({ verbose, silent }: { verbose: boolean; silent: boolean }): void {
-    if (verbose && silent) {
-      this._silent = false;
-      if (!isEnvTest()) {
-        this.onConflictingFlags();
-      }
-    }
     this._silent = silent;
     this._verbose = verbose && !silent;
   }
-
-  protected onConflictingFlags(): void {
-    console.warn("Both --verbose and --silent specified; --silent takes precedence");
-  }
 }
 
 /**
@@ -141,11 +136,6 @@ export class JsonLogger extends BaseLogger implements Logger {
     this._version = version;
   }
 
-  // Suppress raw console.warn in JSON mode to avoid non-JSON text on stderr
-  protected override onConflictingFlags(): void {
-    // No-op: conflicting flags warning is silently ignored in JSON mode
-  }
-
   get jsonMode(): boolean {
     return true;
   }
@@ -226,6 +216,27 @@ export class JsonLogger extends BaseLogger implements Logger {
   }
 }
 
+/**
+ * Warn once when both `--verbose` and `--silent` were passed on the command
+ * line. Called at CLI-flag parsing time only (`wrapCommand`), so re-configuring
+ * a logger from config-file values never re-triggers it. Suppressed in JSON
+ * mode to keep non-JSON text off stderr, matching the former JsonLogger
+ * behavior.
+ */
+export function warnOnConflictingFlags({
+  verbose,
+  silent,
+  jsonMode,
+}: {
+  verbose: boolean;
+  silent: boolean;
+  jsonMode: boolean;
+}): void {
+  if (!verbose || !silent || jsonMode || isEnvTest()) return;
+  // oxlint-disable-next-line no-console
+  console.warn("Both --verbose and --silent specified; --silent takes precedence");
+}
+
 /**
  * Shared fallback logger for code paths that have no command logger threaded
  * through (module-level translators, `warnWithFallback(undefined, ...)`).