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
38 changes: 38 additions & 0 deletions src/config/config-resolver.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,44 @@ describe("config-resolver", () => {
});
});

describe("config file targets (getConfigFileTargets)", () => {
it("expands wildcard targets ['*'] to the full non-legacy target list", async () => {
// Regression for #1981 / #1894: a `targets: ["*"]` config must not collapse
// to an empty config-file target list, otherwise root-file ownership in
// `generate --check` is computed against no targets and the bug reproduces.
const configContent = JSON.stringify({ outputRoots: ["./"], targets: ["*"] });
await writeFileContent(join(testDir, "rulesync.jsonc"), configContent);

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

const configFileTargets = config.getConfigFileTargets();
expect(configFileTargets.length).toBeGreaterThan(1);
expect(configFileTargets).toContain("claudecode");
expect(configFileTargets).toContain("codexcli");
// Legacy targets are excluded from wildcard expansion (must be explicit).
expect(configFileTargets).not.toContain("claudecode-legacy");
expect(configFileTargets).not.toContain("antigravity");
});

it("keeps the full config-file target list even when CLI -t selects one target", async () => {
const configContent = JSON.stringify({ outputRoots: ["./"], targets: ["*"] });
await writeFileContent(join(testDir, "rulesync.jsonc"), configContent);

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

// CLI -t narrows the generated targets, but config-file ownership must
// still see every target the config lists.
expect(config.getTargets()).toEqual(["codexcli"]);
expect(config.getConfigFileTargets().length).toBeGreaterThan(1);
expect(config.getConfigFileTargets()).toContain("claudecode");
});
});

describe("base directory resolution", () => {
it("should load configured outputRoots from file", async () => {
const configContent = JSON.stringify({
Expand Down
10 changes: 10 additions & 0 deletions src/config/config-resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
ConfigFile,
ConfigFileSchema,
ConfigParams,
expandWildcardTargets,
PartialConfigParams,
RequiredConfigParams,
} from "./config.js";
Expand Down Expand Up @@ -359,5 +360,14 @@ function extractConfigFileTargets(
if (isRulesyncConfigTargetsObject(targets)) {
return Object.keys(targets).filter((key): key is ToolTarget => validTargets.has(key));
}
// The wildcard form `["*"]` lists every (non-legacy) target in the config
// file. Expand it via the shared helper (also used by `Config.getTargets()`)
// so the returned list is the full config-file target set rather than an
// empty array. An empty result would make `getConfigFileTargets()` fall back
// to the CLI-filtered `getTargets()`, breaking root-file ownership
// computation for the very common `targets: ["*"]` form (see #1981 / #1894).
if (targets.includes("*")) {
return expandWildcardTargets();
}
return targets.filter((key): key is ToolTarget => key !== "*" && validTargets.has(key));
}
19 changes: 13 additions & 6 deletions src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,18 @@ const CONFLICTING_TARGET_PAIRS: Array<[string, string]> = [
* Legacy targets that should NOT be included in wildcard (*) expansion.
* These targets must be explicitly specified.
*/
const LEGACY_TARGETS = ["augmentcode-legacy", "claudecode-legacy", "antigravity"] as const;
export const LEGACY_TARGETS = ["augmentcode-legacy", "claudecode-legacy", "antigravity"] as const;

/**
* Expand the wildcard target (`*`) to every non-legacy tool target. Legacy
* targets are excluded because they must be requested explicitly. Shared by
* `Config.getTargets()` and `extractConfigFileTargets()` so the two never drift.
*/
export function expandWildcardTargets(): ToolTarget[] {
return ALL_TOOL_TARGETS.filter(
(target) => !LEGACY_TARGETS.includes(target as (typeof LEGACY_TARGETS)[number]),
);
}

/**
* Validates that the user-authored config does not double-define the
Expand Down Expand Up @@ -422,11 +433,7 @@ export class Config {
}

if (arrayTargets.includes("*")) {
// Exclude legacy targets from wildcard expansion
// Legacy targets must be explicitly specified
return ALL_TOOL_TARGETS.filter(
(target) => !LEGACY_TARGETS.includes(target as (typeof LEGACY_TARGETS)[number]),
);
return expandWildcardTargets();
}

return arrayTargets.filter((target): target is ToolTarget => target !== "*");
Expand Down
57 changes: 57 additions & 0 deletions src/e2e/e2e-rules.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,63 @@ globs: ["src/**/*"]
expect(stderr).toBe("");
expect(stdout).toContain("All files are up to date.");
});

it("should attribute rovodev's mirrored ./AGENTS.md so a non-owning target passes check (#1981 #2)", async () => {
const testDir = getTestDir();

const rootRuleContent = `---
root: true
targets: ["*"]
description: "Root rule"
---

# Root Rule
`;
await writeFileContent(
join(testDir, RULESYNC_RULES_RELATIVE_DIR_PATH, RULESYNC_OVERVIEW_FILE_NAME),
rootRuleContent,
);

await writeFileContent(
join(testDir, RULESYNC_CONFIG_RELATIVE_FILE_PATH),
JSON.stringify(
{
targets: {
codexcli: ["rules"],
rovodev: ["rules"],
},
},
null,
2,
),
);

// rovodev is last in config order and mirrors its root rule to the project
// root ./AGENTS.md, so that mirrored file ends up on disk with rovodev's
// content (an "Additional Conventions" preamble codexcli never emits).
await runGenerate({
target: "codexcli,rovodev",
features: "rules",
env: { NODE_ENV: "e2e" },
});

const agentsMd = await readFileContent(join(testDir, "AGENTS.md"));
expect(agentsMd).toContain("Additional Conventions");

// Check codexcli only. Even though codexcli's own root output is ./AGENTS.md,
// rovodev owns the on-disk mirror, so the file is skipped and the check
// passes. Without crediting the mirror to rovodev, codexcli would be treated
// as the owner and fail on the content mismatch.
const { stdout, stderr } = await runGenerate({
target: "codexcli",
features: "rules",
check: true,
env: { NODE_ENV: "e2e" },
});

expect(stderr).toBe("");
expect(stdout).toContain("All files are up to date.");
});
});

describe("E2E: rules (import)", () => {
Expand Down
39 changes: 37 additions & 2 deletions src/lib/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { join } from "node:path";
import { intersection } from "es-toolkit";

import { Config } from "../config/config.js";
import { AGENTSMD_RULE_FILE_NAME } from "../constants/agentsmd-paths.js";
import { RULESYNC_RELATIVE_DIR_PATH } from "../constants/rulesync-paths.js";
import { CommandsProcessor } from "../features/commands/commands-processor.js";
import { HooksProcessor } from "../features/hooks/hooks-processor.js";
Expand Down Expand Up @@ -249,18 +250,52 @@ export async function generate(params: {
};
}

// Maps every root-rule file path a target actually emits to that target, so
// `generate --check` can skip root files a (CLI-selected) target does not own.
//
// Ownership is "last target in config order wins": the loop iterates the config
// file's full target list and `Map.set` overwrites, so the final writer in
// config order owns a shared path — consistent with generation write order,
// where the last target's content is what ends up on disk.
//
// Note: a single ownership decision is applied uniformly across all output
// roots (paths are output-root-relative). Multi-output-root `--check` would
// need per-output-root keying; that is out of scope here.
function computeRootFileOwnership(params: {
targets: ToolTarget[];
global: boolean;
}): Map<string, ToolTarget> {
const ownerByPath = new Map<string, ToolTarget>();
const register = (
relativeDirPath: string,
relativeFilePath: string,
target: ToolTarget,
): void => {
ownerByPath.set(toPosixPath(join(relativeDirPath, relativeFilePath)), target);
};
for (const target of params.targets) {
const factory = RulesProcessor.getFactory(target);
if (!factory) continue;
const paths = factory.class.getSettablePaths({ global: params.global });
if ("root" in paths && paths.root) {
const rootPath = toPosixPath(join(paths.root.relativeDirPath, paths.root.relativeFilePath));
ownerByPath.set(rootPath, target);
register(paths.root.relativeDirPath, paths.root.relativeFilePath, target);
}
// Secondary/fallback root locations a target recognizes are attributed to
// it as well, so a shared collision at one of those paths is skipped for
// non-owning targets.
if ("alternativeRoots" in paths && paths.alternativeRoots) {
for (const alt of paths.alternativeRoots) {
register(alt.relativeDirPath, alt.relativeFilePath, target);
}
}
// Some targets (e.g. rovodev) mirror their primary root — which lives in a
// subdirectory — to a project-root `./AGENTS.md` at generation time (project
// scope only). That mirror is exactly the shared-collision path, so it must
// be attributed to the target too, otherwise ownership/skip decisions invert.
// (For rovodev this overlaps its `alternativeRoots` today; the explicit
// block keeps ownership correct even if that alt root is ever removed.)
if (!params.global && factory.meta.mirrorsRootToAgentsMd) {
register(".", AGENTSMD_RULE_FILE_NAME, target);
}
}
return ownerByPath;
Expand Down
Loading