Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/cli/commands/convert.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ describe("convertCommand", () => {

expect(ConfigResolver.resolve).toHaveBeenCalledWith(
expect.objectContaining({ targets: ["cursor", "claudecode"] }),
{ logger: mockLogger },
);
});
});
Expand All @@ -101,6 +102,7 @@ describe("convertCommand", () => {
targets: ["cursor", "claudecode"],
features: ["*"],
}),
{ logger: mockLogger },
);

expect(RulesProcessor).toHaveBeenCalledWith(
Expand All @@ -125,6 +127,7 @@ describe("convertCommand", () => {
expect.objectContaining({
features: ["rules", "mcp"],
}),
{ logger: mockLogger },
);
});

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

expect(ConfigResolver.resolve).toHaveBeenCalledWith(
expect.objectContaining({ global: true }),
{ logger: mockLogger },
);
});
});
Expand Down
13 changes: 8 additions & 5 deletions src/cli/commands/convert.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,14 @@ export async function convertCommand(logger: Logger, options: ConvertOptions): P
// Pass both source and destinations as `targets` so per-target feature maps
// in `rulesync.jsonc` are honored for every tool involved. Default features
// to `*` so every feature that both tools support is attempted.
const config = await ConfigResolver.resolve({
...options,
targets: [fromTool, ...toTools],
features: options.features ?? ["*"],
});
const config = await ConfigResolver.resolve(
{
...options,
targets: [fromTool, ...toTools],
features: options.features ?? ["*"],
},
{ logger },
);

const isPreview = config.isPreviewMode();
const modePrefix = isPreview ? "[DRY RUN] " : "";
Expand Down
7 changes: 6 additions & 1 deletion src/cli/commands/gitignore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,8 @@ const extractRulesyncManagedEntries = (content: string): string[] => {
export type GitignoreCommandOptions = {
readonly targets?: string[];
readonly features?: RulesyncFeatures;
readonly verbose?: boolean;
readonly silent?: boolean;
};

const groupEntriesByDestination = ({
Expand Down Expand Up @@ -211,7 +213,10 @@ export const gitignoreCommand = async (
): Promise<void> => {
const gitignorePath = join(process.cwd(), ".gitignore");
const gitattributesPath = join(process.cwd(), ".gitattributes");
const config = await ConfigResolver.resolve({});
const config = await ConfigResolver.resolve(
{ verbose: options?.verbose, silent: options?.silent },
{ logger },
);

const resolvedEntries = resolveGitignoreEntries({
targets: options?.targets,
Expand Down
26 changes: 16 additions & 10 deletions src/cli/commands/install.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,14 @@ async function runRulesyncInstall(logger: Logger, options: InstallCommandOptions
// `--mode apm` is required to opt into the APM layout.
const apmExists = await apmManifestExists(projectRoot);

const config = await ConfigResolver.resolve({
configPath: options.configPath,
verbose: options.verbose,
silent: options.silent,
});
const config = await ConfigResolver.resolve(
{
configPath: options.configPath,
verbose: options.verbose,
silent: options.silent,
},
{ logger },
);
const sources = config.getSources();

if (apmExists && sources.length > 0) {
Expand Down Expand Up @@ -141,11 +144,14 @@ async function runGhInstall(logger: Logger, options: InstallCommandOptions): Pro
// gh mode reads sources from `rulesync.jsonc`, never from `apm.yml`. The
// disambiguation between rulesync/apm modes lives in `runRulesyncInstall`;
// here the user has already opted into gh mode explicitly.
const config = await ConfigResolver.resolve({
configPath: options.configPath,
verbose: options.verbose,
silent: options.silent,
});
const config = await ConfigResolver.resolve(
{
configPath: options.configPath,
verbose: options.verbose,
silent: options.silent,
},
{ logger },
);
const sources = config.getSources();

if (sources.length === 0) {
Expand Down
2 changes: 2 additions & 0 deletions src/cli/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ const main = async () => {
await gitignoreCommand(logger, {
targets: resolvedTargets ? [...resolvedTargets] : undefined,
features: cliFeatures,
verbose: (options as { verbose?: boolean }).verbose,
silent: (options as { silent?: boolean }).silent,
});
Comment on lines 82 to 86
}),
);
Expand Down
18 changes: 15 additions & 3 deletions src/cli/wrap-command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,13 @@ import { Command } from "commander";

import { CLIError } from "../types/json-output.js";
import { formatError } from "../utils/error.js";
import { ConsoleLogger, JsonLogger, Logger } from "../utils/logger.js";
import {
ConsoleLogger,
fallbackLogger,
JsonLogger,
Logger,
warnOnConflictingFlags,
} from "../utils/logger.js";

export function createLogger({
name,
Expand Down Expand Up @@ -50,10 +56,16 @@ export function wrapCommand({
const positionalArgs = args.slice(0, -2);
const globalOpts = command.parent?.opts() ?? {};
const logger = loggerFactory({ name, globalOpts, getVersion });
logger.configure({
// Configure from CLI flags first; commands that resolve a config file
// re-configure via `ConfigResolver.resolve` so config-file
// `verbose`/`silent` also apply (CLI flags still win there).
const cliLoggerOptions = {
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);

try {
await handler(logger, options, globalOpts, positionalArgs);
Expand Down
92 changes: 84 additions & 8 deletions src/config/config-resolver.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";

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

const { getHomeDirectoryMock } = vi.hoisted(() => {
Expand Down Expand Up @@ -155,6 +155,82 @@ describe("config-resolver", () => {

expect(config.getSilent()).toBe(true);
});

it("should re-configure the supplied logger from config-file silent/verbose", async () => {
const configContent = JSON.stringify({
outputRoots: ["./"],
silent: true,
verbose: false,
});
await writeFileContent(join(testDir, "rulesync.jsonc"), configContent);
const logger = { warn: vi.fn(), configure: vi.fn() } as unknown as Logger;
const fallbackConfigureSpy = vi.spyOn(fallbackLogger, "configure");

try {
await ConfigResolver.resolve({ configPath: join(testDir, "rulesync.jsonc") }, { logger });

expect(logger.configure).toHaveBeenCalledWith({ verbose: false, silent: true });
expect(fallbackConfigureSpy).toHaveBeenCalledWith({ verbose: false, silent: true });
} finally {
fallbackConfigureSpy.mockRestore();
fallbackLogger.configure({ verbose: false, silent: false });
}
});

it("should re-configure the supplied logger with CLI flags winning over the config file", async () => {
const configContent = JSON.stringify({
outputRoots: ["./"],
silent: false,
verbose: false,
});
await writeFileContent(join(testDir, "rulesync.jsonc"), configContent);
const logger = { warn: vi.fn(), configure: vi.fn() } as unknown as Logger;

try {
await ConfigResolver.resolve(
{ configPath: join(testDir, "rulesync.jsonc"), silent: true, verbose: true },
{ logger },
);

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 () => {
const configContent = JSON.stringify({
outputRoots: ["./"],
verbose: true,
});
await writeFileContent(join(testDir, "rulesync.jsonc"), configContent);
const logger = { warn: vi.fn(), configure: vi.fn() } as unknown as Logger;

try {
await ConfigResolver.resolve({ configPath: join(testDir, "rulesync.jsonc") }, { logger });

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 () => {
const configContent = JSON.stringify({
outputRoots: ["./"],
silent: true,
});
await writeFileContent(join(testDir, "rulesync.jsonc"), configContent);
const fallbackConfigureSpy = vi.spyOn(fallbackLogger, "configure");

try {
await ConfigResolver.resolve({ configPath: join(testDir, "rulesync.jsonc") });

expect(fallbackConfigureSpy).not.toHaveBeenCalled();
} finally {
fallbackConfigureSpy.mockRestore();
}
});
});

describe("config file targets (getConfigFileTargets)", () => {
Expand Down Expand Up @@ -559,7 +635,7 @@ describe("config-resolver", () => {
join(inputRoot, "rulesync.jsonc"),
JSON.stringify({ outputRoots: ["./"], global: true }),
);
const logger = { warn: vi.fn() } as unknown as Logger;
const logger = { warn: vi.fn(), configure: vi.fn() } as unknown as Logger;

await ConfigResolver.resolve(
{
Expand All @@ -572,25 +648,25 @@ describe("config-resolver", () => {
expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('Ignoring "global: true"'));
});

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

try {
await ConfigResolver.resolve({
configPath: "rulesync.jsonc",
inputRoot,
});

expect(consoleWarnSpy).toHaveBeenCalledWith(
expect(fallbackWarnSpy).toHaveBeenCalledWith(
expect.stringContaining('Ignoring "global: true"'),
);
} finally {
consoleWarnSpy.mockRestore();
fallbackWarnSpy.mockRestore();
}
});

Expand All @@ -604,7 +680,7 @@ describe("config-resolver", () => {
join(testDir, "rulesync.jsonc"),
JSON.stringify({ inputRoot: configuredRoot, global: true }),
);
const logger = { warn: vi.fn() } as unknown as Logger;
const logger = { warn: vi.fn(), configure: vi.fn() } as unknown as Logger;

const config = await ConfigResolver.resolve(
{
Expand All @@ -623,7 +699,7 @@ describe("config-resolver", () => {
join(inputRoot, "rulesync.jsonc"),
JSON.stringify({ outputRoots: ["./"], global: true }),
);
const logger = { warn: vi.fn() } as unknown as Logger;
const logger = { warn: vi.fn(), configure: vi.fn() } as unknown as Logger;

await ConfigResolver.resolve(
{
Expand Down
31 changes: 28 additions & 3 deletions src/config/config-resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
resolvePath,
validateOutputRoot,
} from "../utils/file.js";
import { type Logger, warnWithFallback } from "../utils/logger.js";
import { fallbackLogger, type Logger, warnWithFallback } from "../utils/logger.js";
import {
assertTargetsFeaturesExclusive,
Config,
Expand Down Expand Up @@ -289,6 +289,31 @@ export class ConfigResolver {
// so the user knows where to look.
assertMergedTargetsFeaturesExclusive({ configByFile, validatedConfigPath, localConfigPath });

// Wire the resolved `verbose`/`silent` into the logger as soon as they are
// known, so config-file settings are honored by every message emitted from
// here on (including the warnings later in this resolution). CLI flags
// 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. 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,
fallback: getDefaults().verbose,
});
const resolvedSilent = pick({
cli: silent,
file: configByFile.silent,
fallback: getDefaults().silent,
});
if (logger !== undefined) {
logger.configure({ verbose: resolvedVerbose, silent: resolvedSilent });
fallbackLogger.configure({ verbose: resolvedVerbose, silent: resolvedSilent });
}

// When `inputRoot` is set (from CLI, programmatic args, or a config file)
// the user is decoupling source from output, so "global: true" from the
// config file must not apply unless the caller also explicitly passes
Expand Down Expand Up @@ -317,7 +342,7 @@ export class ConfigResolver {
const configParams = {
targets: resolvedTargets,
features: resolvedFeatures,
verbose: pick({ cli: verbose, file: configByFile.verbose, fallback: getDefaults().verbose }),
verbose: resolvedVerbose,
delete: pick({ cli: isDelete, file: configByFile.delete, fallback: getDefaults().delete }),
outputRoots: getOutputRootsInLightOfGlobal({
outputRoots: pick({
Expand All @@ -328,7 +353,7 @@ export class ConfigResolver {
global: resolvedGlobal,
}),
global: resolvedGlobal,
silent: pick({ cli: silent, file: configByFile.silent, fallback: getDefaults().silent }),
silent: resolvedSilent,
simulateCommands: pick({
cli: simulateCommands,
file: configByFile.simulateCommands,
Expand Down
5 changes: 3 additions & 2 deletions src/features/mcp/codexcli-mcp.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { RULESYNC_RELATIVE_DIR_PATH } from "../../constants/rulesync-paths.js";
import { setupTestDirectory } from "../../test-utils/test-directories.js";
import { ensureDir, writeFileContent } from "../../utils/file.js";
import { fallbackLogger } from "../../utils/logger.js";
import { CodexcliMcp } from "./codexcli-mcp.js";
import { RulesyncMcp } from "./rulesync-mcp.js";

Expand Down Expand Up @@ -341,7 +342,7 @@ args = ["server.js"]
});

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

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