mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-12 14:16:28 +03:00
Allow context roots to be declared in .impeccable/config.json (decoupled from package managers) (#307)
* Allow context roots to be declared in .impeccable/config.json Monorepo detection previously read workspace roots only from package managers (package.json workspaces, pnpm-workspace.yaml, lerna.json), coupling "where design context lives" to the dependency graph. Add a `contextRoots` glob list to .impeccable/config.json / config.local.json so non-JS repos -- and design-context boundaries that don't match packages -- can declare nested PRODUCT.md/DESIGN.md roots directly. The new source is folded into readWorkspacePatterns(), so detection, project resolution, and the app picker pick it up unchanged. Negation and config.local.json extension work for free. * Define projectRoots composition with package workspaces Address review feedback on #307: - Rename the config key contextRoots -> projectRoots: the globs establish project boundaries and app-picker targets, not just where context files live. - Make cross-source precedence explicit: a path matched by any impeccable pattern, positive or negated, is governed by the impeccable group alone; package-manager patterns fill in the paths it does not match, and `!` negations apply only within their own source. readWorkspacePatterns() becomes readProjectPatternGroups() / readProjectPatterns(), with package workspaces as one discovery source. - Drop app-picker candidates that would resolve elsewhere: a package workspace subsumed by a broader impeccable boundary is no longer listed, since choosing it would silently resolve to that boundary. - Add five composition tests and document the key in the config and context reference pages (path relativity, glob and negation syntax, shared/local merge, precedence).
This commit is contained in:
@@ -12,7 +12,8 @@ Use config for:
|
||||
|
||||
- detector ignores shared by `npx impeccable detect` and the design hook;
|
||||
- private local ignores that should not be committed;
|
||||
- hook lifecycle settings such as enabled, quiet mode, and audit logging.
|
||||
- hook lifecycle settings such as enabled, quiet mode, and audit logging;
|
||||
- project roots, for repos where design boundaries are not declared by a package manager.
|
||||
|
||||
Use `PRODUCT.md` and `DESIGN.md` for product and design intent. See [Design Context](/docs/context).
|
||||
|
||||
@@ -52,6 +53,24 @@ npx impeccable ignores add-file "src/private-experiment/**" --local
|
||||
|
||||
Local settings go into `.impeccable/config.local.json`, which Impeccable keeps out of git.
|
||||
|
||||
## Project Roots
|
||||
|
||||
Impeccable normally finds nested projects through package-manager workspace declarations: `package.json` workspaces, `pnpm-workspace.yaml`, or `lerna.json`. When those files do not exist, or when design boundaries do not line up with packages, declare the roots directly:
|
||||
|
||||
```json
|
||||
{
|
||||
"projectRoots": ["docs/design/skins/*"]
|
||||
}
|
||||
```
|
||||
|
||||
Each matched folder becomes its own project: it can carry its own `PRODUCT.md` and `DESIGN.md`, it appears in the app picker, and it falls back to the repo root per file for any context it does not define. See [Design Context](/docs/context).
|
||||
|
||||
How the patterns behave:
|
||||
|
||||
- Patterns are relative to the repo root and use the same glob syntax as `package.json` workspaces, including `*`, `**`, and `!` negation.
|
||||
- `projectRoots` in `config.local.json` extends the shared list, so one developer can add private roots without committing them.
|
||||
- A path matched by any `projectRoots` pattern, positive or negated, is governed by this config alone. Package-manager workspaces apply only to paths these patterns do not match, and each source's `!` negations apply only to its own patterns. So `"!apps/internal"` here hides a package workspace from Impeccable, while a package-level negation never hides a folder that `projectRoots` declares.
|
||||
|
||||
## Value ignores
|
||||
|
||||
Prefer value ignores when a rule reports a specific value:
|
||||
|
||||
@@ -107,6 +107,7 @@ Treat context files like any other design artifact: review them in code review w
|
||||
<div>
|
||||
<p>For normal projects, put <code>PRODUCT.md</code> and <code>DESIGN.md</code> in the project root.</p>
|
||||
<p>Skill commands look in the root first. If root context is missing, they also check <code>.agents/context/</code> and <code>docs/</code>.</p>
|
||||
<p>In a monorepo, each workspace child resolves its own <code>PRODUCT.md</code> and <code>DESIGN.md</code> first, then falls back to the repo root per file. Project boundaries come from package-manager workspace declarations, or from <code>projectRoots</code> globs in <code>.impeccable/config.json</code> when no package manager declares them. See <a href="/docs/config">Config and ignores</a>.</p>
|
||||
<p>The detector's design-system rules use the same root-first behavior for <code>DESIGN.md</code>. For generated design metadata, the primary path is <code>.impeccable/design.json</code>. Legacy <code>DESIGN.json</code> files are still accepted as fallbacks, but new projects should use <code>.impeccable/design.json</code>.</p>
|
||||
</div>
|
||||
</details>
|
||||
|
||||
+62
-19
@@ -13,7 +13,9 @@
|
||||
* canonical context files in an ordinary repo (issue #376).
|
||||
* 2. Active project .agents/context/ then docs/
|
||||
* 3. Repo root context, using the same order, as a per-file fallback
|
||||
* whenever the active project is nested below it
|
||||
* whenever the active project is nested below it (a repo counts as a
|
||||
* monorepo when a package manager declares workspaces, or
|
||||
* `.impeccable/config.json` declares `projectRoots`)
|
||||
* 4. $IMPECCABLE_CONTEXT_DIR (absolute or cwd-relative) — power-user
|
||||
* escape hatch, only consulted when defaults are empty
|
||||
* 5. Active project root as a "nothing found" default
|
||||
@@ -242,7 +244,7 @@ function findMonorepoRoot(startDir) {
|
||||
}
|
||||
|
||||
function isMonorepoRoot(dir) {
|
||||
if (readWorkspacePatterns(dir).some((pattern) => !normalizeWorkspacePattern(pattern).startsWith('!'))) return true;
|
||||
if (readProjectPatterns(dir).some((pattern) => !normalizeWorkspacePattern(pattern).startsWith('!'))) return true;
|
||||
if (!MONOREPO_MARKER_FILES.some((file) => fs.existsSync(path.join(dir, file)))) return false;
|
||||
return hasFallbackWorkspaceChildren(dir);
|
||||
}
|
||||
@@ -267,10 +269,12 @@ function hasFallbackWorkspaceChildren(dir) {
|
||||
|
||||
function discoverTargetCandidates(repoRoot) {
|
||||
const roots = new Map();
|
||||
const patterns = readWorkspacePatterns(repoRoot);
|
||||
for (const pattern of patterns) {
|
||||
for (const root of discoverRootsForPattern(repoRoot, pattern)) {
|
||||
roots.set(path.relative(repoRoot, root).split(path.sep).join('/'), root);
|
||||
const patternGroups = readProjectPatternGroups(repoRoot);
|
||||
for (const patterns of patternGroups) {
|
||||
for (const pattern of patterns) {
|
||||
for (const root of discoverRootsForPattern(repoRoot, pattern)) {
|
||||
roots.set(path.relative(repoRoot, root).split(path.sep).join('/'), root);
|
||||
}
|
||||
}
|
||||
}
|
||||
if (MONOREPO_MARKER_FILES.some((file) => fs.existsSync(path.join(repoRoot, file)))) {
|
||||
@@ -291,10 +295,7 @@ function discoverTargetCandidates(repoRoot) {
|
||||
}
|
||||
return [...roots.entries()]
|
||||
.filter(([rel]) => rel && !rel.startsWith('..'))
|
||||
// Honor negated workspace patterns (e.g. "!packages/internal"). resolveWorkspaceProjectRoot
|
||||
// sends an excluded package back to the repo root, so an excluded folder must not appear as a
|
||||
// selectable target — choosing it would silently resolve to the root instead.
|
||||
.filter(([rel]) => !isExcludedByWorkspacePattern(rel.split('/').filter(Boolean), patterns))
|
||||
.filter(([rel]) => isSelectableCandidate(repoRoot, rel, patternGroups))
|
||||
.sort(([a], [b]) => a.localeCompare(b))
|
||||
.map(([rel, root]) => {
|
||||
const targetExample = findTargetExample(repoRoot, root);
|
||||
@@ -450,15 +451,13 @@ function resolveWorkspaceProjectRoot(repoRoot, targetDir) {
|
||||
const rel = path.relative(repoRoot, targetDir);
|
||||
if (!rel || rel.startsWith('..') || path.isAbsolute(rel)) return repoRoot;
|
||||
const relSegments = rel.split(path.sep).filter(Boolean);
|
||||
const patterns = readWorkspacePatterns(repoRoot);
|
||||
const excluded = isExcludedByWorkspacePattern(relSegments, patterns);
|
||||
if (!excluded) {
|
||||
for (const patterns of readProjectPatternGroups(repoRoot)) {
|
||||
if (isExcludedByWorkspacePattern(relSegments, patterns)) return repoRoot;
|
||||
for (const pattern of patterns) {
|
||||
const projectRoot = projectRootFromWorkspacePattern(repoRoot, relSegments, pattern);
|
||||
if (projectRoot) return projectRoot;
|
||||
}
|
||||
}
|
||||
if (excluded) return repoRoot;
|
||||
if (
|
||||
relSegments.length >= 2
|
||||
&& MONOREPO_FALLBACK_PROJECT_DIRS.includes(relSegments[0])
|
||||
@@ -470,6 +469,25 @@ function resolveWorkspaceProjectRoot(repoRoot, targetDir) {
|
||||
return repoRoot;
|
||||
}
|
||||
|
||||
// A discovered folder is only selectable when picking it would resolve back to
|
||||
// itself. Impeccable `projectRoots` patterns govern every path they match:
|
||||
// a negation drops the candidate (resolveWorkspaceProjectRoot would send it to
|
||||
// the repo root), and a positive match with a different boundary drops it too,
|
||||
// because the boundary root is already its own candidate and choosing the
|
||||
// deeper folder would silently resolve there. Paths the Impeccable group does
|
||||
// not match fall through to the package-manager negations, which is the
|
||||
// pre-existing behavior for package workspaces and marker-dir fallbacks.
|
||||
function isSelectableCandidate(repoRoot, rel, patternGroups) {
|
||||
const relSegments = rel.split('/').filter(Boolean);
|
||||
const [impeccablePatterns, packagePatterns] = patternGroups;
|
||||
if (isExcludedByWorkspacePattern(relSegments, impeccablePatterns)) return false;
|
||||
for (const pattern of impeccablePatterns) {
|
||||
const boundary = projectRootFromWorkspacePattern(repoRoot, relSegments, pattern);
|
||||
if (boundary) return path.resolve(boundary) === path.resolve(path.join(repoRoot, ...relSegments));
|
||||
}
|
||||
return !isExcludedByWorkspacePattern(relSegments, packagePatterns);
|
||||
}
|
||||
|
||||
function isExcludedByWorkspacePattern(relSegments, patterns) {
|
||||
return patterns.some((rawPattern) => {
|
||||
const pattern = normalizeWorkspacePattern(rawPattern);
|
||||
@@ -558,12 +576,37 @@ function workspacePatternMatchesRel(pattern, relSegments) {
|
||||
return true;
|
||||
}
|
||||
|
||||
function readWorkspacePatterns(repoRoot) {
|
||||
// Project boundaries come from two sources, in precedence order: explicit
|
||||
// `projectRoots` globs in .impeccable config, then package-manager workspace
|
||||
// declarations. A path matched by any Impeccable pattern — positive or
|
||||
// negated — is governed by the Impeccable group alone; package-manager
|
||||
// patterns only apply to paths the Impeccable group does not match. Within a
|
||||
// group, negations win over positives.
|
||||
function readProjectPatternGroups(repoRoot) {
|
||||
return [
|
||||
...readPackageWorkspaces(repoRoot),
|
||||
...readPnpmWorkspaces(repoRoot),
|
||||
...readLernaWorkspaces(repoRoot),
|
||||
].filter(Boolean);
|
||||
readImpeccableProjectRoots(repoRoot),
|
||||
[
|
||||
...readPackageWorkspaces(repoRoot),
|
||||
...readPnpmWorkspaces(repoRoot),
|
||||
...readLernaWorkspaces(repoRoot),
|
||||
].filter(Boolean),
|
||||
];
|
||||
}
|
||||
|
||||
function readProjectPatterns(repoRoot) {
|
||||
return readProjectPatternGroups(repoRoot).flat();
|
||||
}
|
||||
|
||||
function readImpeccableProjectRoots(repoRoot) {
|
||||
const patterns = [];
|
||||
for (const name of ['config.json', 'config.local.json']) {
|
||||
const cfg = readJson(path.join(repoRoot, '.impeccable', name));
|
||||
if (!Array.isArray(cfg?.projectRoots)) continue;
|
||||
for (const entry of cfg.projectRoots) {
|
||||
if (typeof entry === 'string' && entry.trim()) patterns.push(entry.trim());
|
||||
}
|
||||
}
|
||||
return patterns;
|
||||
}
|
||||
|
||||
function readPackageWorkspaces(repoRoot) {
|
||||
|
||||
@@ -769,6 +769,168 @@ describe('loadContext (monorepo project context)', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('loadContext (impeccable projectRoots config)', () => {
|
||||
function writeSkinsConfig(extra = {}) {
|
||||
write('.impeccable/config.json', JSON.stringify({ projectRoots: ['docs/design/skins/*'], ...extra }, null, 2));
|
||||
write('PRODUCT.md', '# Root product\n');
|
||||
write('DESIGN.md', '# Root design\n');
|
||||
}
|
||||
|
||||
it('treats a config-declared context root as a monorepo with no package-manager files', () => {
|
||||
writeSkinsConfig();
|
||||
write('docs/design/skins/neon-seoul/DESIGN.md', '# Neon Seoul design\n');
|
||||
|
||||
const ctx = loadContext(scratch, { targetPath: 'docs/design/skins/neon-seoul' });
|
||||
assert.equal(ctx.isMonorepo, true);
|
||||
assert.equal(ctx.projectRoot, path.join(scratch, 'docs', 'design', 'skins', 'neon-seoul'));
|
||||
assert.equal(ctx.repoRoot, scratch);
|
||||
// The skin uses its own DESIGN.md and inherits the root PRODUCT.md per file.
|
||||
assert.match(ctx.design, /Neon Seoul design/);
|
||||
assert.match(ctx.product, /Root product/);
|
||||
assert.equal(ctx.designPath, path.join('docs', 'design', 'skins', 'neon-seoul', 'DESIGN.md'));
|
||||
assert.equal(ctx.productPath, 'PRODUCT.md');
|
||||
});
|
||||
|
||||
it('resolves a config-declared child from cwd inside the folder', () => {
|
||||
writeSkinsConfig();
|
||||
write('docs/design/skins/marble/DESIGN.md', '# Marble design\n');
|
||||
|
||||
const skinDir = path.join(scratch, 'docs', 'design', 'skins', 'marble');
|
||||
const ctx = loadContext(skinDir);
|
||||
assert.equal(ctx.isMonorepo, true);
|
||||
assert.equal(ctx.projectRoot, skinDir);
|
||||
assert.match(ctx.design, /Marble design/);
|
||||
assert.match(ctx.product, /Root product/);
|
||||
});
|
||||
|
||||
it('extends shared projectRoots with config.local.json', () => {
|
||||
write('.impeccable/config.json', JSON.stringify({ projectRoots: ['docs/design/skins/*'] }, null, 2));
|
||||
write('.impeccable/config.local.json', JSON.stringify({ projectRoots: ['experiments/*'] }, null, 2));
|
||||
write('PRODUCT.md', '# Root product\n');
|
||||
write('DESIGN.md', '# Root design\n');
|
||||
write('experiments/wip/DESIGN.md', '# WIP design\n');
|
||||
|
||||
const ctx = loadContext(scratch, { targetPath: 'experiments/wip' });
|
||||
assert.equal(ctx.projectRoot, path.join(scratch, 'experiments', 'wip'));
|
||||
assert.match(ctx.design, /WIP design/);
|
||||
assert.match(ctx.product, /Root product/);
|
||||
});
|
||||
|
||||
it('does not treat an .impeccable config without projectRoots as a monorepo', () => {
|
||||
write('.impeccable/config.json', JSON.stringify({ hook: { consent: 'accepted' } }, null, 2));
|
||||
write('PRODUCT.md', '# Root product\n');
|
||||
write('docs/design/skins/marble/DESIGN.md', '# Marble design\n');
|
||||
|
||||
const ctx = loadContext(scratch, { targetPath: 'docs/design/skins/marble' });
|
||||
assert.equal(ctx.isMonorepo, false);
|
||||
});
|
||||
|
||||
it('asks for app selection from a config-declared monorepo root', () => {
|
||||
write('.impeccable/config.json', JSON.stringify({ projectRoots: ['docs/design/skins/*'] }, null, 2));
|
||||
write('PRODUCT.md', '# Root product\n');
|
||||
write('DESIGN.md', '# Root design\n');
|
||||
write('docs/design/skins/neon-seoul/DESIGN.md', '# Neon Seoul\n');
|
||||
write('docs/design/skins/marble/DESIGN.md', '# Marble\n');
|
||||
|
||||
const res = spawnSync(process.execPath, [SCRIPT_PATH], {
|
||||
cwd: scratch,
|
||||
encoding: 'utf8',
|
||||
env: { ...process.env, IMPECCABLE_NO_UPDATE_CHECK: '1' },
|
||||
});
|
||||
assert.equal(res.status, 0);
|
||||
const selection = parseTargetSelection(res.stdout);
|
||||
const paths = selection.targetCandidates.map((candidate) => candidate.path).sort();
|
||||
assert.deepEqual(paths, ['docs/design/skins/marble', 'docs/design/skins/neon-seoul']);
|
||||
});
|
||||
|
||||
// Composition with package-manager workspaces: a path matched by any
|
||||
// projectRoots pattern (positive or negated) is governed by the impeccable
|
||||
// config alone; package-manager patterns fill in the paths it does not match.
|
||||
describe('composition with package-manager workspaces', () => {
|
||||
function selectionPaths() {
|
||||
const res = spawnSync(process.execPath, [SCRIPT_PATH], {
|
||||
cwd: scratch,
|
||||
encoding: 'utf8',
|
||||
env: { ...process.env, IMPECCABLE_NO_UPDATE_CHECK: '1' },
|
||||
});
|
||||
assert.equal(res.status, 0, res.stderr);
|
||||
const selection = parseTargetSelection(res.stdout);
|
||||
return selection.targetCandidates.map((candidate) => candidate.path).sort();
|
||||
}
|
||||
|
||||
it('lets an impeccable negation exclude a package-manager workspace', () => {
|
||||
write('package.json', JSON.stringify({ private: true, workspaces: ['apps/*'] }, null, 2));
|
||||
write('.impeccable/config.json', JSON.stringify({ projectRoots: ['!apps/internal'] }, null, 2));
|
||||
write('PRODUCT.md', '# Root product\n');
|
||||
write('apps/dashboard/PRODUCT.md', '# Dashboard product\n');
|
||||
write('apps/internal/PRODUCT.md', '# Internal product\n');
|
||||
|
||||
const ctx = loadContext(scratch, { targetPath: 'apps/internal' });
|
||||
assert.equal(ctx.isMonorepo, true);
|
||||
assert.equal(ctx.projectRoot, scratch);
|
||||
assert.match(ctx.product, /Root product/);
|
||||
assert.deepEqual(selectionPaths(), ['apps/dashboard']);
|
||||
});
|
||||
|
||||
it('keeps a package-manager negation scoped to its own source', () => {
|
||||
write('package.json', JSON.stringify({
|
||||
private: true,
|
||||
workspaces: ['apps/*', '!docs/design/skins/marble'],
|
||||
}, null, 2));
|
||||
write('.impeccable/config.json', JSON.stringify({ projectRoots: ['docs/design/skins/*'] }, null, 2));
|
||||
write('PRODUCT.md', '# Root product\n');
|
||||
write('apps/dashboard/PRODUCT.md', '# Dashboard product\n');
|
||||
write('docs/design/skins/marble/DESIGN.md', '# Marble design\n');
|
||||
|
||||
const ctx = loadContext(scratch, { targetPath: 'docs/design/skins/marble' });
|
||||
assert.equal(ctx.projectRoot, path.join(scratch, 'docs', 'design', 'skins', 'marble'));
|
||||
assert.match(ctx.design, /Marble design/);
|
||||
assert.deepEqual(selectionPaths(), ['apps/dashboard', 'docs/design/skins/marble']);
|
||||
});
|
||||
|
||||
it('gives a broad impeccable pattern the boundary over a deeper package workspace', () => {
|
||||
write('package.json', JSON.stringify({ private: true, workspaces: ['apps/web/packages/ui'] }, null, 2));
|
||||
write('.impeccable/config.json', JSON.stringify({ projectRoots: ['apps/*'] }, null, 2));
|
||||
write('PRODUCT.md', '# Root product\n');
|
||||
write('apps/web/PRODUCT.md', '# Web product\n');
|
||||
write('apps/web/packages/ui/PRODUCT.md', '# UI product\n');
|
||||
write('apps/web/packages/ui/src/Button.jsx', 'export default null;\n');
|
||||
|
||||
const ctx = loadContext(scratch, { targetPath: 'apps/web/packages/ui/src/Button.jsx' });
|
||||
assert.equal(ctx.projectRoot, path.join(scratch, 'apps', 'web'));
|
||||
assert.match(ctx.product, /Web product/);
|
||||
// The subsumed package workspace must not appear as its own pick:
|
||||
// choosing it would silently resolve to apps/web.
|
||||
assert.deepEqual(selectionPaths(), ['apps/web']);
|
||||
});
|
||||
|
||||
it('falls through to package workspaces for paths impeccable does not match', () => {
|
||||
write('package.json', JSON.stringify({ private: true, workspaces: ['apps/*'] }, null, 2));
|
||||
write('.impeccable/config.json', JSON.stringify({ projectRoots: ['docs/design/skins/*'] }, null, 2));
|
||||
write('PRODUCT.md', '# Root product\n');
|
||||
write('apps/dashboard/PRODUCT.md', '# Dashboard product\n');
|
||||
write('docs/design/skins/marble/DESIGN.md', '# Marble design\n');
|
||||
|
||||
const ctx = loadContext(scratch, { targetPath: 'apps/dashboard' });
|
||||
assert.equal(ctx.projectRoot, path.join(scratch, 'apps', 'dashboard'));
|
||||
assert.match(ctx.product, /Dashboard product/);
|
||||
});
|
||||
|
||||
it('resolves other workspaces normally when impeccable config only negates', () => {
|
||||
write('package.json', JSON.stringify({ private: true, workspaces: ['apps/*'] }, null, 2));
|
||||
write('.impeccable/config.json', JSON.stringify({ projectRoots: ['!apps/internal'] }, null, 2));
|
||||
write('PRODUCT.md', '# Root product\n');
|
||||
write('apps/dashboard/PRODUCT.md', '# Dashboard product\n');
|
||||
write('apps/internal/PRODUCT.md', '# Internal product\n');
|
||||
|
||||
const ctx = loadContext(scratch, { targetPath: 'apps/dashboard' });
|
||||
assert.equal(ctx.isMonorepo, true);
|
||||
assert.equal(ctx.projectRoot, path.join(scratch, 'apps', 'dashboard'));
|
||||
assert.match(ctx.product, /Dashboard product/);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('loadContext (IMPECCABLE_CONTEXT_DIR escape hatch)', () => {
|
||||
it('reads from the override path when defaults are empty', () => {
|
||||
write('design/PRODUCT.md', '# overridden product\n');
|
||||
|
||||
Reference in New Issue
Block a user