Skip to content

Commit 2d28625

Browse files
cm-dyoshikawaclaude
andcommitted
fix: honor config-file silent and verbose in logger configuration
Config-file silent: true was a documented no-op: wrapCommand configured the logger from CLI flags only and Config.getSilent() had no call sites. - ConfigResolver.resolve now re-configures the supplied logger (and the shared fallbackLogger) from the resolved verbose/silent as soon as the config files are merged, so config-file settings apply to everything logged afterwards. CLI flags still win via the existing pick precedence. - Thread the command logger into ConfigResolver.resolve from the convert, install, and gitignore commands (generate and import already did). - Replace the unconfigured module-level ConsoleLogger fallbacks in the qwencode/augmentcode permission translators with a shared fallbackLogger that wrapCommand and ConfigResolver keep configured, and route warnWithFallback(undefined, ...) through it so those warn paths honor silent end to end. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 28c82c5 commit 2d28625

13 files changed

Lines changed: 213 additions & 50 deletions

src/cli/commands/convert.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ describe("convertCommand", () => {
8686

8787
expect(ConfigResolver.resolve).toHaveBeenCalledWith(
8888
expect.objectContaining({ targets: ["cursor", "claudecode"] }),
89+
{ logger: mockLogger },
8990
);
9091
});
9192
});
@@ -101,6 +102,7 @@ describe("convertCommand", () => {
101102
targets: ["cursor", "claudecode"],
102103
features: ["*"],
103104
}),
105+
{ logger: mockLogger },
104106
);
105107

106108
expect(RulesProcessor).toHaveBeenCalledWith(
@@ -125,6 +127,7 @@ describe("convertCommand", () => {
125127
expect.objectContaining({
126128
features: ["rules", "mcp"],
127129
}),
130+
{ logger: mockLogger },
128131
);
129132
});
130133

@@ -172,6 +175,7 @@ describe("convertCommand", () => {
172175

173176
expect(ConfigResolver.resolve).toHaveBeenCalledWith(
174177
expect.objectContaining({ global: true }),
178+
{ logger: mockLogger },
175179
);
176180
});
177181
});

src/cli/commands/convert.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,14 @@ export async function convertCommand(logger: Logger, options: ConvertOptions): P
4444
// Pass both source and destinations as `targets` so per-target feature maps
4545
// in `rulesync.jsonc` are honored for every tool involved. Default features
4646
// to `*` so every feature that both tools support is attempted.
47-
const config = await ConfigResolver.resolve({
48-
...options,
49-
targets: [fromTool, ...toTools],
50-
features: options.features ?? ["*"],
51-
});
47+
const config = await ConfigResolver.resolve(
48+
{
49+
...options,
50+
targets: [fromTool, ...toTools],
51+
features: options.features ?? ["*"],
52+
},
53+
{ logger },
54+
);
5255

5356
const isPreview = config.isPreviewMode();
5457
const modePrefix = isPreview ? "[DRY RUN] " : "";

src/cli/commands/gitignore.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,8 @@ const extractRulesyncManagedEntries = (content: string): string[] => {
166166
export type GitignoreCommandOptions = {
167167
readonly targets?: string[];
168168
readonly features?: RulesyncFeatures;
169+
readonly verbose?: boolean;
170+
readonly silent?: boolean;
169171
};
170172

171173
const groupEntriesByDestination = ({
@@ -211,7 +213,10 @@ export const gitignoreCommand = async (
211213
): Promise<void> => {
212214
const gitignorePath = join(process.cwd(), ".gitignore");
213215
const gitattributesPath = join(process.cwd(), ".gitattributes");
214-
const config = await ConfigResolver.resolve({});
216+
const config = await ConfigResolver.resolve(
217+
{ verbose: options?.verbose, silent: options?.silent },
218+
{ logger },
219+
);
215220

216221
const resolvedEntries = resolveGitignoreEntries({
217222
targets: options?.targets,

src/cli/commands/install.ts

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,14 @@ async function runRulesyncInstall(logger: Logger, options: InstallCommandOptions
4444
// `--mode apm` is required to opt into the APM layout.
4545
const apmExists = await apmManifestExists(projectRoot);
4646

47-
const config = await ConfigResolver.resolve({
48-
configPath: options.configPath,
49-
verbose: options.verbose,
50-
silent: options.silent,
51-
});
47+
const config = await ConfigResolver.resolve(
48+
{
49+
configPath: options.configPath,
50+
verbose: options.verbose,
51+
silent: options.silent,
52+
},
53+
{ logger },
54+
);
5255
const sources = config.getSources();
5356

5457
if (apmExists && sources.length > 0) {
@@ -141,11 +144,14 @@ async function runGhInstall(logger: Logger, options: InstallCommandOptions): Pro
141144
// gh mode reads sources from `rulesync.jsonc`, never from `apm.yml`. The
142145
// disambiguation between rulesync/apm modes lives in `runRulesyncInstall`;
143146
// here the user has already opted into gh mode explicitly.
144-
const config = await ConfigResolver.resolve({
145-
configPath: options.configPath,
146-
verbose: options.verbose,
147-
silent: options.silent,
148-
});
147+
const config = await ConfigResolver.resolve(
148+
{
149+
configPath: options.configPath,
150+
verbose: options.verbose,
151+
silent: options.silent,
152+
},
153+
{ logger },
154+
);
149155
const sources = config.getSources();
150156

151157
if (sources.length === 0) {

src/cli/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,8 @@ const main = async () => {
8181
await gitignoreCommand(logger, {
8282
targets: resolvedTargets ? [...resolvedTargets] : undefined,
8383
features: cliFeatures,
84+
verbose: (options as { verbose?: boolean }).verbose,
85+
silent: (options as { silent?: boolean }).silent,
8486
});
8587
}),
8688
);

src/cli/wrap-command.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { Command } from "commander";
22

33
import { CLIError } from "../types/json-output.js";
44
import { formatError } from "../utils/error.js";
5-
import { ConsoleLogger, JsonLogger, Logger } from "../utils/logger.js";
5+
import { ConsoleLogger, fallbackLogger, JsonLogger, Logger } from "../utils/logger.js";
66

77
export function createLogger({
88
name,
@@ -50,10 +50,15 @@ export function wrapCommand({
5050
const positionalArgs = args.slice(0, -2);
5151
const globalOpts = command.parent?.opts() ?? {};
5252
const logger = loggerFactory({ name, globalOpts, getVersion });
53-
logger.configure({
53+
// Configure from CLI flags first; commands that resolve a config file
54+
// re-configure via `ConfigResolver.resolve` so config-file
55+
// `verbose`/`silent` also apply (CLI flags still win there).
56+
const cliLoggerOptions = {
5457
verbose: Boolean(globalOpts.verbose) || Boolean(options.verbose),
5558
silent: Boolean(globalOpts.silent) || Boolean(options.silent),
56-
});
59+
};
60+
logger.configure(cliLoggerOptions);
61+
fallbackLogger.configure(cliLoggerOptions);
5762

5863
try {
5964
await handler(logger, options, globalOpts, positionalArgs);

src/config/config-resolver.test.ts

Lines changed: 75 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
44

55
import { setupTestDirectory } from "../test-utils/test-directories.js";
66
import { writeFileContent } from "../utils/file.js";
7-
import type { Logger } from "../utils/logger.js";
7+
import { fallbackLogger, type Logger } from "../utils/logger.js";
88
import { ConfigResolver } from "./config-resolver.js";
99

1010
const { getHomeDirectoryMock } = vi.hoisted(() => {
@@ -155,6 +155,73 @@ describe("config-resolver", () => {
155155

156156
expect(config.getSilent()).toBe(true);
157157
});
158+
159+
it("should re-configure the supplied logger from config-file silent/verbose", async () => {
160+
const configContent = JSON.stringify({
161+
outputRoots: ["./"],
162+
silent: true,
163+
verbose: false,
164+
});
165+
await writeFileContent(join(testDir, "rulesync.jsonc"), configContent);
166+
const logger = { warn: vi.fn(), configure: vi.fn() } as unknown as Logger;
167+
const fallbackConfigureSpy = vi.spyOn(fallbackLogger, "configure");
168+
169+
try {
170+
await ConfigResolver.resolve({ configPath: join(testDir, "rulesync.jsonc") }, { logger });
171+
172+
expect(logger.configure).toHaveBeenCalledWith({ verbose: false, silent: true });
173+
expect(fallbackConfigureSpy).toHaveBeenCalledWith({ verbose: false, silent: true });
174+
} finally {
175+
fallbackConfigureSpy.mockRestore();
176+
}
177+
});
178+
179+
it("should re-configure the supplied logger with CLI flags winning over the config file", async () => {
180+
const configContent = JSON.stringify({
181+
outputRoots: ["./"],
182+
silent: false,
183+
verbose: false,
184+
});
185+
await writeFileContent(join(testDir, "rulesync.jsonc"), configContent);
186+
const logger = { warn: vi.fn(), configure: vi.fn() } as unknown as Logger;
187+
188+
await ConfigResolver.resolve(
189+
{ configPath: join(testDir, "rulesync.jsonc"), silent: true, verbose: true },
190+
{ logger },
191+
);
192+
193+
expect(logger.configure).toHaveBeenCalledWith({ verbose: true, silent: true });
194+
});
195+
196+
it("should enable logger verbose from config-file verbose", async () => {
197+
const configContent = JSON.stringify({
198+
outputRoots: ["./"],
199+
verbose: true,
200+
});
201+
await writeFileContent(join(testDir, "rulesync.jsonc"), configContent);
202+
const logger = { warn: vi.fn(), configure: vi.fn() } as unknown as Logger;
203+
204+
await ConfigResolver.resolve({ configPath: join(testDir, "rulesync.jsonc") }, { logger });
205+
206+
expect(logger.configure).toHaveBeenCalledWith({ verbose: true, silent: false });
207+
});
208+
209+
it("should not touch the fallbackLogger when no logger is supplied", async () => {
210+
const configContent = JSON.stringify({
211+
outputRoots: ["./"],
212+
silent: true,
213+
});
214+
await writeFileContent(join(testDir, "rulesync.jsonc"), configContent);
215+
const fallbackConfigureSpy = vi.spyOn(fallbackLogger, "configure");
216+
217+
try {
218+
await ConfigResolver.resolve({ configPath: join(testDir, "rulesync.jsonc") });
219+
220+
expect(fallbackConfigureSpy).not.toHaveBeenCalled();
221+
} finally {
222+
fallbackConfigureSpy.mockRestore();
223+
}
224+
});
158225
});
159226

160227
describe("config file targets (getConfigFileTargets)", () => {
@@ -559,7 +626,7 @@ describe("config-resolver", () => {
559626
join(inputRoot, "rulesync.jsonc"),
560627
JSON.stringify({ outputRoots: ["./"], global: true }),
561628
);
562-
const logger = { warn: vi.fn() } as unknown as Logger;
629+
const logger = { warn: vi.fn(), configure: vi.fn() } as unknown as Logger;
563630

564631
await ConfigResolver.resolve(
565632
{
@@ -572,25 +639,25 @@ describe("config-resolver", () => {
572639
expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('Ignoring "global: true"'));
573640
});
574641

575-
it("should fall back to console.warn when no logger is supplied", async () => {
642+
it("should fall back to the shared fallbackLogger when no logger is supplied", async () => {
576643
const inputRoot = join(testDir, "central-rules");
577644
await writeFileContent(
578645
join(inputRoot, "rulesync.jsonc"),
579646
JSON.stringify({ outputRoots: ["./"], global: true }),
580647
);
581-
const consoleWarnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
648+
const fallbackWarnSpy = vi.spyOn(fallbackLogger, "warn").mockImplementation(() => {});
582649

583650
try {
584651
await ConfigResolver.resolve({
585652
configPath: "rulesync.jsonc",
586653
inputRoot,
587654
});
588655

589-
expect(consoleWarnSpy).toHaveBeenCalledWith(
656+
expect(fallbackWarnSpy).toHaveBeenCalledWith(
590657
expect.stringContaining('Ignoring "global: true"'),
591658
);
592659
} finally {
593-
consoleWarnSpy.mockRestore();
660+
fallbackWarnSpy.mockRestore();
594661
}
595662
});
596663

@@ -604,7 +671,7 @@ describe("config-resolver", () => {
604671
join(testDir, "rulesync.jsonc"),
605672
JSON.stringify({ inputRoot: configuredRoot, global: true }),
606673
);
607-
const logger = { warn: vi.fn() } as unknown as Logger;
674+
const logger = { warn: vi.fn(), configure: vi.fn() } as unknown as Logger;
608675

609676
const config = await ConfigResolver.resolve(
610677
{
@@ -623,7 +690,7 @@ describe("config-resolver", () => {
623690
join(inputRoot, "rulesync.jsonc"),
624691
JSON.stringify({ outputRoots: ["./"], global: true }),
625692
);
626-
const logger = { warn: vi.fn() } as unknown as Logger;
693+
const logger = { warn: vi.fn(), configure: vi.fn() } as unknown as Logger;
627694

628695
await ConfigResolver.resolve(
629696
{

src/config/config-resolver.ts

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import {
1919
resolvePath,
2020
validateOutputRoot,
2121
} from "../utils/file.js";
22-
import { type Logger, warnWithFallback } from "../utils/logger.js";
22+
import { fallbackLogger, type Logger, warnWithFallback } from "../utils/logger.js";
2323
import {
2424
assertTargetsFeaturesExclusive,
2525
Config,
@@ -289,6 +289,28 @@ export class ConfigResolver {
289289
// so the user knows where to look.
290290
assertMergedTargetsFeaturesExclusive({ configByFile, validatedConfigPath, localConfigPath });
291291

292+
// Wire the resolved `verbose`/`silent` into the logger as soon as they are
293+
// known, so config-file settings are honored by every message emitted from
294+
// here on (including the warnings later in this resolution). CLI flags
295+
// still win via `pick`. Only re-configure when the caller threaded a
296+
// logger through — those call sites pass their CLI flags in the params, so
297+
// precedence stays intact. The shared `fallbackLogger` is kept in sync for
298+
// paths that have no logger threaded through.
299+
const resolvedVerbose = pick({
300+
cli: verbose,
301+
file: configByFile.verbose,
302+
fallback: getDefaults().verbose,
303+
});
304+
const resolvedSilent = pick({
305+
cli: silent,
306+
file: configByFile.silent,
307+
fallback: getDefaults().silent,
308+
});
309+
if (logger !== undefined) {
310+
logger.configure({ verbose: resolvedVerbose, silent: resolvedSilent });
311+
fallbackLogger.configure({ verbose: resolvedVerbose, silent: resolvedSilent });
312+
}
313+
292314
// When `inputRoot` is set (from CLI, programmatic args, or a config file)
293315
// the user is decoupling source from output, so "global: true" from the
294316
// config file must not apply unless the caller also explicitly passes
@@ -317,7 +339,7 @@ export class ConfigResolver {
317339
const configParams = {
318340
targets: resolvedTargets,
319341
features: resolvedFeatures,
320-
verbose: pick({ cli: verbose, file: configByFile.verbose, fallback: getDefaults().verbose }),
342+
verbose: resolvedVerbose,
321343
delete: pick({ cli: isDelete, file: configByFile.delete, fallback: getDefaults().delete }),
322344
outputRoots: getOutputRootsInLightOfGlobal({
323345
outputRoots: pick({
@@ -328,7 +350,7 @@ export class ConfigResolver {
328350
global: resolvedGlobal,
329351
}),
330352
global: resolvedGlobal,
331-
silent: pick({ cli: silent, file: configByFile.silent, fallback: getDefaults().silent }),
353+
silent: resolvedSilent,
332354
simulateCommands: pick({
333355
cli: simulateCommands,
334356
file: configByFile.simulateCommands,

src/features/mcp/codexcli-mcp.test.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
55
import { RULESYNC_RELATIVE_DIR_PATH } from "../../constants/rulesync-paths.js";
66
import { setupTestDirectory } from "../../test-utils/test-directories.js";
77
import { ensureDir, writeFileContent } from "../../utils/file.js";
8+
import { fallbackLogger } from "../../utils/logger.js";
89
import { CodexcliMcp } from "./codexcli-mcp.js";
910
import { RulesyncMcp } from "./rulesync-mcp.js";
1011

@@ -341,7 +342,7 @@ args = ["server.js"]
341342
});
342343

343344
it("should normalize invalid Codex MCP server names and warn when the last collision wins", async () => {
344-
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
345+
const warnSpy = vi.spyOn(fallbackLogger, "warn").mockImplementation(() => {});
345346
const rulesyncMcp = new RulesyncMcp({
346347
relativeDirPath: RULESYNC_RELATIVE_DIR_PATH,
347348
relativeFilePath: ".mcp.json",
@@ -371,7 +372,7 @@ args = ["server.js"]
371372
});
372373

373374
it("should keep non-representable server names via a stable hash fallback instead of dropping them", async () => {
374-
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
375+
const warnSpy = vi.spyOn(fallbackLogger, "warn").mockImplementation(() => {});
375376
const rulesyncMcp = new RulesyncMcp({
376377
relativeDirPath: RULESYNC_RELATIVE_DIR_PATH,
377378
relativeFilePath: ".mcp.json",

src/features/permissions/augmentcode-permissions.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import type { PermissionAction, PermissionsConfig } from "../../types/permission
1111
import { readAugmentcodeSettingsWithLocalOverlay } from "../../utils/augmentcode-settings.js";
1212
import { formatError } from "../../utils/error.js";
1313
import { readFileContentOrNull } from "../../utils/file.js";
14-
import { ConsoleLogger, type Logger } from "../../utils/logger.js";
14+
import { fallbackLogger, type Logger } from "../../utils/logger.js";
1515
import { applySharedConfigPatch, sharedConfigFileKey } from "../shared/shared-config-gateway.js";
1616
import { RulesyncPermissions } from "./rulesync-permissions.js";
1717
import {
@@ -22,10 +22,11 @@ import {
2222
type ToolPermissionsSettablePaths,
2323
} from "./tool-permissions.js";
2424

25-
// Module-level logger used by the importing direction (toRulesyncPermissions),
25+
// Shared fallback logger used by the importing direction (toRulesyncPermissions),
2626
// where the instance method has no `logger` parameter. Mirrors the Qwen
27-
// permissions translator's pattern.
28-
const moduleLogger: Logger = new ConsoleLogger();
27+
// permissions translator's pattern; `fallbackLogger` is configured from CLI
28+
// flags and the resolved config, so `silent` is honored.
29+
const moduleLogger: Logger = fallbackLogger;
2930

3031
/**
3132
* AugmentCode CLI uses `.augment/settings.json` (project) or `~/.augment/settings.json` (global).

0 commit comments

Comments
 (0)