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
26 changes: 26 additions & 0 deletions src/config/config-resolver.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,32 @@ describe("config-resolver", () => {
expect(config.getOutputRoots()).toContain(resolve("./app2"));
expect(config.getOutputRoots()).toContain(resolve("./app3"));
});

it("should resolve outputRoots configured per target", async () => {
const configContent = JSON.stringify({
targets: ["copilot", "claudecode"],
outputRoots: {
copilot: "./build/copilot",
claudecode: ["./build/claudecode", "./build/claude-extra"],
},
});
await writeFileContent(join(testDir, "rulesync.jsonc"), configContent);

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

expect(config.getOutputRoots("copilot")).toEqual([resolve("./build/copilot")]);
expect(config.getOutputRoots("claudecode")).toEqual([
resolve("./build/claudecode"),
resolve("./build/claude-extra"),
]);
expect(config.getOutputRoots()).toEqual([
resolve("./build/copilot"),
resolve("./build/claudecode"),
resolve("./build/claude-extra"),
]);
});
});

describe("local configuration (rulesync.local.jsonc)", () => {
Expand Down
28 changes: 22 additions & 6 deletions src/config/config-resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
PartialConfigParams,
RequiredConfigParams,
} from "./config.js";
import type { OutputRoots } from "./config.js";

/**
* CLI-resolvable params exclude `sources` — sources are config-file-only.
Expand Down Expand Up @@ -372,9 +373,9 @@ function getOutputRootsInLightOfGlobal({
outputRoots,
global,
}: {
outputRoots: string[];
outputRoots: OutputRoots;
global: boolean;
}): string[] {
}): OutputRoots {
if (global) {
// When global is true, the base directory is always the home directory
return [getHomeDirectory()];
Expand All @@ -383,11 +384,26 @@ function getOutputRootsInLightOfGlobal({
// Validate the *raw* user input first so traversal patterns like
// `/foo/../bar` cannot slip through `resolve()`'s normalization. Then
// resolve to absolute for downstream consumers.
outputRoots.forEach((outputRoot) => {
validateOutputRoot(outputRoot);
});
if (Array.isArray(outputRoots)) {
outputRoots.forEach((outputRoot) => {
validateOutputRoot(outputRoot);
});

return outputRoots.map((outputRoot) => resolve(outputRoot));
}

const resolvedOutputRoots: OutputRoots = {};
for (const [target, targetOutputRoots] of Object.entries(outputRoots)) {
const roots = Array.isArray(targetOutputRoots) ? targetOutputRoots : [targetOutputRoots];
roots.forEach((outputRoot) => {
validateOutputRoot(outputRoot);
});
resolvedOutputRoots[target as ToolTarget] = Array.isArray(targetOutputRoots)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

resolvedOutputRoots is built as a plain {} and populated via bracket assignment. A config with a key literally named __proto__ would hit the Object.prototype.__proto__ setter instead of creating an own property, so the entry silently disappears from Object.keys() and never reaches validateObjectFormOutputRootKeys's unknown-key check (it just gets dropped instead of raising the intended error). Not exploitable beyond this local object, but building it with Object.create(null) (or explicitly rejecting __proto__/constructor/prototype keys) would make malformed config fail loudly instead of silently.

? roots.map((outputRoot) => resolve(outputRoot))
: resolve(targetOutputRoots);
}

return outputRoots.map((outputRoot) => resolve(outputRoot));
return resolvedOutputRoots;
}

function extractConfigFileTargets(
Expand Down
48 changes: 43 additions & 5 deletions src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,11 @@ const SourceEntrySchema = z.object({
});
export type SourceEntry = z.infer<typeof SourceEntrySchema>;

const ConfigParamsSchema = z.object({
outputRoots: z.array(z.string()),
export const ConfigParamsSchema = z.object({
outputRoots: z.union([
z.array(z.string()),
z.record(z.string(), z.union([z.string(), z.array(z.string())])),
]),
targets: RulesyncConfigTargetsSchema,
features: RulesyncFeaturesSchema,
verbose: z.boolean(),
Expand Down Expand Up @@ -99,7 +102,9 @@ const ConfigParamsSchema = z.object({
// `assertTargetsOrFeaturesProvided`. Programmatic callers constructing
// `Config` directly must respect these invariants.
type InferredConfigParams = z.infer<typeof ConfigParamsSchema>;
export type OutputRoots = string[] | Partial<Record<ToolTarget, string | string[]>>;
export type ConfigParams = Omit<InferredConfigParams, "targets" | "features"> & {
outputRoots: OutputRoots;
targets?: RulesyncConfigTargets;
features?: RulesyncFeatures;
configFileTargets?: ToolTarget[];
Expand All @@ -108,6 +113,7 @@ export type ConfigParams = Omit<InferredConfigParams, "targets" | "features"> &
const PartialConfigParamsSchema = z.partial(ConfigParamsSchema);
type InferredPartialConfigParams = z.infer<typeof PartialConfigParamsSchema>;
export type PartialConfigParams = Omit<InferredPartialConfigParams, "targets" | "features"> & {
outputRoots?: OutputRoots;
targets?: RulesyncConfigTargets;
features?: RulesyncFeatures;
};
Expand All @@ -119,13 +125,15 @@ export const ConfigFileSchema = z.object({
});
type InferredConfigFile = z.infer<typeof ConfigFileSchema>;
export type ConfigFile = Omit<InferredConfigFile, "targets" | "features"> & {
outputRoots?: OutputRoots;
targets?: RulesyncConfigTargets;
features?: RulesyncFeatures;
};

const RequiredConfigParamsSchema = z.required(ConfigParamsSchema);
type InferredRequiredConfigParams = z.infer<typeof RequiredConfigParamsSchema>;
export type RequiredConfigParams = Omit<InferredRequiredConfigParams, "targets" | "features"> & {
outputRoots: OutputRoots;
targets?: RulesyncConfigTargets;
features?: RulesyncFeatures;
};
Expand Down Expand Up @@ -207,7 +215,7 @@ const assertTargetsOrFeaturesProvided = ({
};

export class Config {
private readonly outputRoots: string[];
private readonly outputRoots: OutputRoots;
private readonly targets: RulesyncConfigTargets;
private readonly features: RulesyncFeatures;
/**
Expand Down Expand Up @@ -274,6 +282,7 @@ export class Config {
// Reject unknown keys in the object form of `targets`. Array-form values
// are already validated at the Zod schema level.
this.validateObjectFormTargetKeys(resolvedTargets);
this.validateObjectFormOutputRootKeys(outputRoots);

// Validate conflicting targets (accepts array and object forms)
this.validateConflictingTargets(resolvedTargets);
Expand Down Expand Up @@ -346,6 +355,18 @@ export class Config {
}
}

private validateObjectFormOutputRootKeys(outputRoots: OutputRoots): void {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This duplicates validateObjectFormTargetKeys above almost exactly (same Array.isArray guard, same new Set(ALL_TOOL_TARGETS), same unknown-key loop — differing only in the wildcard handling and the error message). Might be worth factoring into a shared helper, e.g. assertKnownTargetKeys(obj, { allowWildcard, fieldLabel }).

if (Array.isArray(outputRoots)) return;
const validTargets = new Set<string>(ALL_TOOL_TARGETS);
for (const key of Object.keys(outputRoots)) {
if (!validTargets.has(key)) {
throw new Error(
`Unknown outputRoots target '${key}'. Valid targets: ${ALL_TOOL_TARGETS.join(", ")}.`,
);
}
}
}

private validateConflictingTargets(targets: RulesyncConfigTargets): void {
// Wildcard (*) doesn't include legacy targets, so conflicts can only
// occur when both sides of a conflicting pair are explicitly present.
Expand All @@ -366,8 +387,25 @@ export class Config {
}
}

public getOutputRoots(): string[] {
return this.outputRoots;
public getOutputRoots(): string[];
public getOutputRoots(target: ToolTarget): string[];
public getOutputRoots(target?: ToolTarget): string[] {
if (Array.isArray(this.outputRoots)) {
return this.outputRoots;
}

if (target) {
const targetOutputRoots = this.outputRoots[target];
if (targetOutputRoots === undefined) return [];

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

When a target that is present in targets has no key in the per-target outputRoots map, this returns [] and the generate.ts loop for that target silently runs zero times — no warning, no error. Worth either falling back to a default (e.g. process.cwd()) or emitting a logger.warn similar to warnUnsupportedTargets, so a typo in the map does not silently produce no output for a target.

return Array.isArray(targetOutputRoots) ? targetOutputRoots : [targetOutputRoots];
}

const allRoots: string[] = [];
for (const value of Object.values(this.outputRoots)) {
if (value === undefined) continue;
allRoots.push(...(Array.isArray(value) ? value : [value]));
}
return [...new Set(allRoots)];
}

/**
Expand Down
30 changes: 15 additions & 15 deletions src/lib/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -490,8 +490,8 @@ async function generateRulesCore(params: {
})
: new Map<string, ToolTarget>();

for (const outputRoot of config.getOutputRoots()) {
for (const toolTarget of toolTargets) {
for (const toolTarget of toolTargets) {
for (const outputRoot of config.getOutputRoots(toolTarget)) {
// Check if rules feature is enabled for this specific target
if (!config.getFeatures(toolTarget).includes("rules")) {
continue;
Expand Down Expand Up @@ -566,7 +566,7 @@ async function generateIgnoreCore(params: {
continue;
}

for (const outputRoot of config.getOutputRoots()) {
for (const outputRoot of config.getOutputRoots(toolTarget)) {
try {
const processor = new IgnoreProcessor({
// Pass `outputRoot` verbatim. The legacy
Expand Down Expand Up @@ -619,8 +619,8 @@ async function generateMcpCore(params: {
logger,
});

for (const outputRoot of config.getOutputRoots()) {
for (const toolTarget of toolTargets) {
for (const toolTarget of toolTargets) {
for (const outputRoot of config.getOutputRoots(toolTarget)) {
// Check if mcp feature is enabled for this specific target
if (!config.getFeatures(toolTarget).includes("mcp")) {
continue;
Expand Down Expand Up @@ -670,8 +670,8 @@ async function generateCommandsCore(params: {
logger,
});

for (const outputRoot of config.getOutputRoots()) {
for (const toolTarget of toolTargets) {
for (const toolTarget of toolTargets) {
for (const outputRoot of config.getOutputRoots(toolTarget)) {
// Check if commands feature is enabled for this specific target
if (!config.getFeatures(toolTarget).includes("commands")) {
continue;
Expand Down Expand Up @@ -726,8 +726,8 @@ async function generateSubagentsCore(params: {
logger,
});

for (const outputRoot of config.getOutputRoots()) {
for (const toolTarget of toolTargets) {
for (const toolTarget of toolTargets) {
for (const outputRoot of config.getOutputRoots(toolTarget)) {
// Check if subagents feature is enabled for this specific target
if (!config.getFeatures(toolTarget).includes("subagents")) {
continue;
Expand Down Expand Up @@ -778,8 +778,8 @@ async function generateSkillsCore(params: {
logger,
});

for (const outputRoot of config.getOutputRoots()) {
for (const toolTarget of toolTargets) {
for (const toolTarget of toolTargets) {
for (const outputRoot of config.getOutputRoots(toolTarget)) {
// Check if skills feature is enabled for this specific target
if (!config.getFeatures(toolTarget).includes("skills")) {
continue;
Expand Down Expand Up @@ -838,8 +838,8 @@ async function generateHooksCore(params: {
logger,
});

for (const outputRoot of config.getOutputRoots()) {
for (const toolTarget of toolTargets) {
for (const toolTarget of toolTargets) {
for (const outputRoot of config.getOutputRoots(toolTarget)) {
// Check if hooks feature is enabled for this specific target
if (!config.getFeatures(toolTarget).includes("hooks")) {
continue;
Expand Down Expand Up @@ -886,8 +886,8 @@ async function generatePermissionsCore(params: {
const allPaths: string[] = [];
let hasDiff = false;

for (const outputRoot of config.getOutputRoots()) {
for (const toolTarget of intersection(config.getTargets(), supportedPermissionsTargets)) {
for (const toolTarget of intersection(config.getTargets(), supportedPermissionsTargets)) {
for (const outputRoot of config.getOutputRoots(toolTarget)) {
if (!config.getFeatures(toolTarget).includes("permissions")) {
continue;
}
Expand Down
Loading