fix(acr): RUL resolves rule glob against the rule's own project root, not the scan root (M-BUG-9)
A rule's paths:/globs: pattern scopes relative to the directory containing the rule's .claude/, not the outer scan target. countGlobMatches globbed against the scan root and collectProjectFiles' depth>4 cutoff never reached deep matching files, so a live rule in a nested repo (e.g. a marketplace checkout under ~/.claude) was wrongly flagged "matches no files / never activates" (high) — a false F-grade for any user with rules in a nested repo. Same scope-conflation family as M-BUG-1/2/8. - deriveProjectRoot(ruleAbsPath): parent of the rule's .claude segment. - collect + glob per project root (cached), relative to that root — so a nested repo's rule resolves against its own tree, where its files live. - user-global rules (root === HOME) skip the no-match check: they scope against whatever project is active at runtime, not a fixed tree, so "matches 0 files here" is not a dead-rule signal (and avoids a HOME walk). TDD: 2 failing tests (nested-repo false-positive + HOME guard) -> green. Full suite 1307/0; frozen v5.0.0 + default-output snapshots unchanged (RUL appears in none; the fix is a no-op when projectRoot === targetPath, i.e. the common single-repo scan). Dogfooding C6: clears the 2 ktg-privat false positives on the real machine and surfaces a previously-hidden genuine dead rule (false negative) in the bundled optimal-setup example. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EnUvKEqyEa1m9gy6Aqhdqq
This commit is contained in:
parent
4ad1875b31
commit
18af5a24e9
2 changed files with 140 additions and 19 deletions
|
|
@ -10,7 +10,7 @@ import { SEVERITY } from './lib/severity.mjs';
|
||||||
import { parseFrontmatter } from './lib/yaml-parser.mjs';
|
import { parseFrontmatter } from './lib/yaml-parser.mjs';
|
||||||
import { lineCount, truncate } from './lib/string-utils.mjs';
|
import { lineCount, truncate } from './lib/string-utils.mjs';
|
||||||
import { readdir, stat } from 'node:fs/promises';
|
import { readdir, stat } from 'node:fs/promises';
|
||||||
import { join, resolve, relative } from 'node:path';
|
import { join, resolve, relative, sep } from 'node:path';
|
||||||
|
|
||||||
const SCANNER = 'RUL';
|
const SCANNER = 'RUL';
|
||||||
|
|
||||||
|
|
@ -30,8 +30,16 @@ export async function scan(targetPath, discovery) {
|
||||||
return scannerResult(SCANNER, 'skipped', [], 0, Date.now() - start);
|
return scannerResult(SCANNER, 'skipped', [], 0, Date.now() - start);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Collect all real files in the project for glob matching
|
// Rule path patterns scope relative to the rule's OWN project root (the dir
|
||||||
const projectFiles = await collectProjectFiles(targetPath);
|
// containing its .claude/), not the outer scan root. Resolve + cache per root.
|
||||||
|
const home = process.env.HOME || process.env.USERPROFILE || '';
|
||||||
|
const projectFilesByRoot = new Map();
|
||||||
|
async function projectFilesFor(root) {
|
||||||
|
if (!projectFilesByRoot.has(root)) {
|
||||||
|
projectFilesByRoot.set(root, await collectProjectFiles(root));
|
||||||
|
}
|
||||||
|
return projectFilesByRoot.get(root);
|
||||||
|
}
|
||||||
|
|
||||||
for (const file of ruleFiles) {
|
for (const file of ruleFiles) {
|
||||||
const content = await readTextFile(file.absPath);
|
const content = await readTextFile(file.absPath);
|
||||||
|
|
@ -74,22 +82,32 @@ export async function scan(targetPath, discovery) {
|
||||||
if (paths) {
|
if (paths) {
|
||||||
const patterns = Array.isArray(paths) ? paths : [paths];
|
const patterns = Array.isArray(paths) ? paths : [paths];
|
||||||
|
|
||||||
for (const pattern of patterns) {
|
// A rule scopes relative to its own project root (parent of its .claude/),
|
||||||
if (typeof pattern !== 'string') continue;
|
// not the scan root. User-global rules (root === HOME) match against the
|
||||||
|
// active project at runtime, so "matches no files here" is not meaningful.
|
||||||
|
const projectRoot = deriveProjectRoot(file.absPath) || targetPath;
|
||||||
|
const isUserGlobal = home && projectRoot === home;
|
||||||
|
|
||||||
// Check if pattern matches any real files
|
if (!isUserGlobal) {
|
||||||
const matchCount = countGlobMatches(pattern, projectFiles, targetPath);
|
const projectFiles = await projectFilesFor(projectRoot);
|
||||||
if (matchCount === 0) {
|
|
||||||
findings.push(finding({
|
for (const pattern of patterns) {
|
||||||
scanner: SCANNER,
|
if (typeof pattern !== 'string') continue;
|
||||||
severity: SEVERITY.high,
|
|
||||||
title: 'Rule path pattern matches no files',
|
// Check if pattern matches any real files (relative to the rule's root)
|
||||||
description: `${file.relPath}: pattern "${pattern}" matches 0 files. This rule will never activate.`,
|
const matchCount = countGlobMatches(pattern, projectFiles, projectRoot);
|
||||||
file: file.absPath,
|
if (matchCount === 0) {
|
||||||
evidence: `paths: "${pattern}"`,
|
findings.push(finding({
|
||||||
recommendation: 'Check the glob pattern. Common issues: wrong directory name, missing **, incorrect extension.',
|
scanner: SCANNER,
|
||||||
autoFixable: false,
|
severity: SEVERITY.high,
|
||||||
}));
|
title: 'Rule path pattern matches no files',
|
||||||
|
description: `${file.relPath}: pattern "${pattern}" matches 0 files. This rule will never activate.`,
|
||||||
|
file: file.absPath,
|
||||||
|
evidence: `paths: "${pattern}"`,
|
||||||
|
recommendation: 'Check the glob pattern. Common issues: wrong directory name, missing **, incorrect extension.',
|
||||||
|
autoFixable: false,
|
||||||
|
}));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -195,6 +213,18 @@ async function collectProjectFiles(targetPath, depth = 0) {
|
||||||
* @param {string} basePath
|
* @param {string} basePath
|
||||||
* @returns {number}
|
* @returns {number}
|
||||||
*/
|
*/
|
||||||
|
/**
|
||||||
|
* Resolve the project root a rule scopes against: the directory containing the
|
||||||
|
* `.claude/` dir the rule lives under. `/a/b/.claude/rules/x.md` → `/a/b`.
|
||||||
|
* Returns null if the path has no `.claude` segment.
|
||||||
|
*/
|
||||||
|
function deriveProjectRoot(ruleAbsPath) {
|
||||||
|
const parts = ruleAbsPath.split(sep);
|
||||||
|
const idx = parts.lastIndexOf('.claude');
|
||||||
|
if (idx <= 0) return null;
|
||||||
|
return parts.slice(0, idx).join(sep);
|
||||||
|
}
|
||||||
|
|
||||||
function countGlobMatches(pattern, files, basePath) {
|
function countGlobMatches(pattern, files, basePath) {
|
||||||
try {
|
try {
|
||||||
const regex = globToRegex(pattern);
|
const regex = globToRegex(pattern);
|
||||||
|
|
|
||||||
|
|
@ -200,3 +200,94 @@ describe('RUL — block-sequence-scoped rule is correctly scoped (parser regress
|
||||||
assert.equal(f.severity, 'low');
|
assert.equal(f.severity, 'low');
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('RUL — nested-repo rule glob resolves to its own project root (M-BUG-9)', () => {
|
||||||
|
// A rule living in a NESTED repo's .claude/rules/ scopes its paths: pattern
|
||||||
|
// relative to that nested repo's root — NOT relative to the outer scan root.
|
||||||
|
// Before the fix, countGlobMatches globbed against the scan root and
|
||||||
|
// collectProjectFiles' depth>4 cutoff never reached the deep matching files,
|
||||||
|
// so a live rule was wrongly flagged "matches no files / never activates".
|
||||||
|
// (Real-machine surface: ~/.claude/plugins/marketplaces/ktg-privat/.claude/rules/.)
|
||||||
|
let tmpRoot;
|
||||||
|
let result;
|
||||||
|
|
||||||
|
async function writeNestedRepo(scanRoot) {
|
||||||
|
// Nested repo sits 3 dirs below the scan root → its matching files land
|
||||||
|
// past the old depth>4 cutoff when walked from the scan root.
|
||||||
|
const nested = join(scanRoot, 'level1', 'level2', 'nested-repo');
|
||||||
|
await mkdir(join(nested, '.claude', 'rules'), { recursive: true });
|
||||||
|
await mkdir(join(nested, 'plugins', 'app'), { recursive: true });
|
||||||
|
await mkdir(join(nested, 'plugins', 'app', 'hooks'), { recursive: true });
|
||||||
|
// Files that the rule patterns match — relative to the NESTED repo root.
|
||||||
|
await writeFile(join(nested, 'plugins', 'app', 'CLAUDE.md'), '# App\n', 'utf8');
|
||||||
|
await writeFile(join(nested, 'plugins', 'app', 'hooks', 'hooks.json'), '{}\n', 'utf8');
|
||||||
|
// Two rules mirroring the real ktg-privat ones.
|
||||||
|
await writeFile(
|
||||||
|
join(nested, '.claude', 'rules', 'plugin-convention.md'),
|
||||||
|
'---\npaths: "plugins/*/CLAUDE.md"\n---\n\n# Convention\nbody\n',
|
||||||
|
'utf8',
|
||||||
|
);
|
||||||
|
await writeFile(
|
||||||
|
join(nested, '.claude', 'rules', 'hook-format.md'),
|
||||||
|
'---\npaths: "**/hooks/hooks.json"\n---\n\n# Hook format\nbody\n',
|
||||||
|
'utf8',
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
beforeEach(async () => {
|
||||||
|
resetCounter();
|
||||||
|
tmpRoot = await mkdtemp(join(tmpdir(), 'ca-rul-nested-'));
|
||||||
|
await writeNestedRepo(tmpRoot);
|
||||||
|
const discovery = await discoverConfigFiles(tmpRoot);
|
||||||
|
result = await scan(tmpRoot, discovery);
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(async () => {
|
||||||
|
if (tmpRoot) await rm(tmpRoot, { recursive: true, force: true });
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does NOT flag the nested rules as "matches no files"', () => {
|
||||||
|
const dead = result.findings.filter(f => f.title.includes('matches no files'));
|
||||||
|
assert.equal(
|
||||||
|
dead.length,
|
||||||
|
0,
|
||||||
|
`nested-repo rules matched real files but were flagged dead: ${dead.map(f => f.evidence).join(' | ')}`,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('RUL — user-global rule is not flagged "matches no files" (M-BUG-9 guard)', () => {
|
||||||
|
// A user-global ~/.claude/rules/ rule scopes against whatever project is
|
||||||
|
// active at runtime, not a fixed tree — so "matches 0 files here" is not a
|
||||||
|
// meaningful dead-rule signal and must not be flagged (and must not trigger a
|
||||||
|
// HOME-wide file walk).
|
||||||
|
let tmpHome;
|
||||||
|
let savedHome;
|
||||||
|
let result;
|
||||||
|
|
||||||
|
beforeEach(async () => {
|
||||||
|
resetCounter();
|
||||||
|
tmpHome = await mkdtemp(join(tmpdir(), 'ca-rul-home-'));
|
||||||
|
savedHome = process.env.HOME;
|
||||||
|
process.env.HOME = tmpHome;
|
||||||
|
await mkdir(join(tmpHome, '.claude', 'rules'), { recursive: true });
|
||||||
|
await writeFile(
|
||||||
|
join(tmpHome, '.claude', 'rules', 'global.md'),
|
||||||
|
'---\npaths: "src/**/*.rs"\n---\n\n# Global rust rule\nbody\n',
|
||||||
|
'utf8',
|
||||||
|
);
|
||||||
|
// Scan from HOME so the rule is discovered AND its derived project root === HOME.
|
||||||
|
const discovery = await discoverConfigFiles(tmpHome);
|
||||||
|
result = await scan(tmpHome, discovery);
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(async () => {
|
||||||
|
process.env.HOME = savedHome;
|
||||||
|
if (tmpHome) await rm(tmpHome, { recursive: true, force: true });
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does NOT flag a user-global rule as "matches no files"', () => {
|
||||||
|
const dead = result.findings.filter(f => f.title.includes('matches no files'));
|
||||||
|
assert.equal(dead.length, 0, `user-global rule wrongly flagged dead: ${dead.map(f => f.evidence).join(' | ')}`);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue