From dca8f1ca6f860784ca9da26e76ded0b827c67ed7 Mon Sep 17 00:00:00 2001 From: Abdul Wahab Date: Fri, 14 Aug 2026 23:41:29 +0500 Subject: [PATCH] Fix: inherit the monorepo root's DESIGN.md in detect design-system rules (#570) findDesignRoot stopped at the first package.json boundary, so every design-system rule silently abstained for files inside monorepo workspaces. The walk now continues past a workspace boundary to the monorepo root that owns it, recognized the same way context.mjs does (declared workspace globs, or a marker file beside apps/ or packages/ children). A nested repo with its own .git, a workspace-owned DESIGN.md, and non-monorepo projects keep their existing behavior. Written with AI assistance (Cursor); reviewed by maintainer. Co-authored-by: Cursor --- cli/engine/design-system.mjs | 72 ++++++++- scripts/test-suites.mjs | 1 + tests/detect-cli-design-monorepo.test.mjs | 181 ++++++++++++++++++++++ 3 files changed, 247 insertions(+), 7 deletions(-) create mode 100644 tests/detect-cli-design-monorepo.test.mjs diff --git a/cli/engine/design-system.mjs b/cli/engine/design-system.mjs index 5c9a949e6..1f07d5815 100644 --- a/cli/engine/design-system.mjs +++ b/cli/engine/design-system.mjs @@ -13,6 +13,11 @@ const FALLBACK_DIRS = ['.agents/context', 'docs']; // CLI can't import (separate tree). `.git` and `package.json` are the common // boundaries; `.impeccable` is our own project marker. const PROJECT_ROOT_MARKERS = ['.git', 'package.json', '.impeccable']; +// Monorepo-root recognition, mirroring context.mjs's isMonorepoRoot: declared +// workspace globs (package.json `workspaces`, pnpm-workspace.yaml `packages:`) +// or a marker file beside apps/ or packages/ children. +const MONOREPO_MARKER_FILES = ['pnpm-workspace.yaml', 'turbo.json', 'nx.json', 'lerna.json']; +const MONOREPO_FALLBACK_PROJECT_DIRS = ['apps', 'packages']; const COLOR_CHANNEL_TOLERANCE = 6; // Shadow blacks at different alphas are different tokens (0.28 vs 0.55 is the // difference between a documented shadow and drift), so shadow matching cannot @@ -581,24 +586,77 @@ function designSystemStartDir(targetPath, cwd = process.cwd()) { // - A directory carrying a DESIGN.md (directly or in a fallback dir) IS the // design root — that's where the rules live. // - A directory carrying a project marker (.git / package.json / .impeccable) -// but no DESIGN.md is a project BOUNDARY: the walk stops with no design -// system, so a sibling project never inherits a parent's or cwd's rules. +// but no DESIGN.md is a project BOUNDARY. When that boundary is a monorepo +// workspace (owned by a workspace-declaring root above it, recognized the +// same way context.mjs does), the workspace inherits the monorepo root's +// DESIGN.md; a nested separate repository (.git with no workspace +// declaration) still inherits nothing (issue #570). // - Reaching the home directory / filesystem root with neither means no // design system at all — never process.cwd()'s. // // Returns { dir, hasDesign } for the stopping directory, or null when the walk // runs out. This is the fix for cross-project contamination. +function readWorkspacePatterns(dir) { + const workspaces = safeReadJson(path.join(dir, 'package.json'))?.workspaces; + const patterns = Array.isArray(workspaces) ? [...workspaces] + : Array.isArray(workspaces?.packages) ? [...workspaces.packages] : []; + try { + let inPackages = false; + for (const line of fs.readFileSync(path.join(dir, 'pnpm-workspace.yaml'), 'utf-8').split(/\r?\n/)) { + const trimmed = line.trim(); + if (!trimmed || trimmed.startsWith('#')) continue; + const flow = trimmed.match(/^packages:\s*\[(.*)\]\s*$/); + if (flow) { + patterns.push(...flow[1].split(',').map(entry => entry.trim().replace(/^['"]|['"]$/g, '')).filter(Boolean)); + break; + } + if (/^packages:\s*$/.test(trimmed)) { inPackages = true; continue; } + if (!inPackages) continue; + const item = trimmed.match(/^-\s*(.+)$/); + if (item) patterns.push(item[1].trim().replace(/^['"]|['"]$/g, '')); + else if (/^[A-Za-z0-9_-]+:\s*/.test(trimmed)) break; + } + } catch { /* no pnpm-workspace.yaml */ } + return patterns; +} + +function isMonorepoRoot(dir) { + if (readWorkspacePatterns(dir).some(pattern => !String(pattern).trim().startsWith('!'))) return true; + if (!MONOREPO_MARKER_FILES.some(file => fs.existsSync(path.join(dir, file)))) return false; + return MONOREPO_FALLBACK_PROJECT_DIRS.some(name => { + try { + return fs.readdirSync(path.join(dir, name), { withFileTypes: true }).some(entry => entry.isDirectory()); + } catch { + return false; + } + }); +} + export function findDesignRoot(startDir) { let dir = path.resolve(startDir); const homeDir = path.resolve(os.homedir()); + let boundary = null; while (true) { - if (resolveDesignMdPath(dir)) return { dir, hasDesign: true }; - if (PROJECT_ROOT_MARKERS.some((marker) => fs.existsSync(path.join(dir, marker)))) { - return { dir, hasDesign: false }; + if (!boundary && resolveDesignMdPath(dir)) return { dir, hasDesign: true }; + if (boundary) { + // Past the boundary the walk only looks for the monorepo root that owns + // the workspace. Monorepo-root before .git, same order as context.mjs: + // a workspace root carrying its own .git is still recognized, while a + // .git that declares no workspaces is a separate repository and stops + // the walk with nothing inherited. The home directory is never an + // owning root, same as context.mjs's findMonorepoRoot, which stops at + // homeDir before its monorepo check. + if (dir !== homeDir && isMonorepoRoot(dir)) return { dir, hasDesign: !!resolveDesignMdPath(dir) }; + if (fs.existsSync(path.join(dir, '.git'))) return boundary; + } else if (PROJECT_ROOT_MARKERS.some((marker) => fs.existsSync(path.join(dir, marker)))) { + boundary = { dir, hasDesign: false }; + // A boundary that is itself a monorepo root, or a separate repository + // with its own .git, inherits nothing from above. + if (isMonorepoRoot(dir) || fs.existsSync(path.join(dir, '.git'))) return boundary; } - if (dir === homeDir) return null; + if (dir === homeDir) return boundary; const parent = path.dirname(dir); - if (parent === dir) return null; + if (parent === dir) return boundary; dir = parent; } } diff --git a/scripts/test-suites.mjs b/scripts/test-suites.mjs index 4fba4df6d..1c1f64c58 100644 --- a/scripts/test-suites.mjs +++ b/scripts/test-suites.mjs @@ -107,6 +107,7 @@ export const SUITES = { 'tests/detect-antipatterns-fixtures.test.mjs', 'tests/detect-antipatterns-browser.test.mjs', 'tests/detect-cli-design-contamination.test.mjs', + 'tests/detect-cli-design-monorepo.test.mjs', 'tests/detect-cli-stdin-dispatch.test.mjs', ], }, diff --git a/tests/detect-cli-design-monorepo.test.mjs b/tests/detect-cli-design-monorepo.test.mjs new file mode 100644 index 000000000..c7767b6b4 --- /dev/null +++ b/tests/detect-cli-design-monorepo.test.mjs @@ -0,0 +1,181 @@ +/** + * Regression for issue #570: design-system rules must reach a monorepo workspace + * by inheriting the repo root's DESIGN.md. + * + * Run with: node --test tests/detect-cli-design-monorepo.test.mjs + */ + +import { describe, it, after } from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { spawnSync } from 'node:child_process'; +import { fileURLToPath } from 'node:url'; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const CLI = path.resolve(__dirname, '../cli/bin/cli.js'); + +const PAGE_HTML = + '' + + '
Hi
'; + +const DESIGN_MD = `--- +typography: + body: + fontFamily: "Palatino, Georgia, serif" +--- +# Project A Design System +`; + +const tempRoots = []; + +function runDetect(cwd, targets, env = {}) { + const result = spawnSync(process.execPath, [CLI, 'detect', '--json', ...targets], { + cwd, + encoding: 'utf-8', + env: { ...process.env, ...env }, + }); + let findings = []; + try { + findings = JSON.parse(result.stdout || '[]'); + } catch { + throw new Error(`Non-JSON CLI output.\nstdout: ${result.stdout}\nstderr: ${result.stderr}`); + } + return findings; +} + +function fontFindingsFor(findings, file) { + return findings.filter( + (f) => f.antipattern === 'design-system-font' && (!file || f.file === file), + ); +} + +function mkPnpmMonorepo({ workspaceDesign = null } = {}) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'impeccable-detect-mono-pnpm-')); + tempRoots.push(dir); + fs.writeFileSync(path.join(dir, 'DESIGN.md'), DESIGN_MD); + fs.writeFileSync(path.join(dir, 'pnpm-workspace.yaml'), "packages:\n - 'apps/*'\n"); + fs.mkdirSync(path.join(dir, 'apps/web'), { recursive: true }); + fs.writeFileSync(path.join(dir, 'apps/web/package.json'), '{"name":"web"}'); + const page = path.join(dir, 'apps/web/page.html'); + fs.writeFileSync(page, PAGE_HTML); + if (workspaceDesign) { + fs.writeFileSync(path.join(dir, 'apps/web/DESIGN.md'), workspaceDesign); + } + return { dir, page, webDir: path.join(dir, 'apps/web') }; +} + +after(() => { + for (const dir of tempRoots) { + try { fs.rmSync(dir, { recursive: true, force: true }); } catch { /* best effort */ } + } +}); + +describe('detect CLI monorepo DESIGN.md inheritance', () => { + it('pnpm workspace root: workspace page inherits root DESIGN.md', () => { + const { dir, page } = mkPnpmMonorepo(); + const findings = runDetect(dir, [page]); + assert.ok( + fontFindingsFor(findings, page).some((f) => f.ignoreValue === 'verdana'), + 'Verdana must be flagged via inherited root DESIGN.md', + ); + }); + + it('npm/yarn workspaces root: workspace page inherits root DESIGN.md', () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'impeccable-detect-mono-npm-')); + tempRoots.push(dir); + fs.writeFileSync(path.join(dir, 'DESIGN.md'), DESIGN_MD); + fs.writeFileSync(path.join(dir, 'package.json'), '{"name":"mono","workspaces":["packages/*"]}'); + fs.mkdirSync(path.join(dir, 'packages/ui'), { recursive: true }); + fs.writeFileSync(path.join(dir, 'packages/ui/package.json'), '{"name":"ui"}'); + const page = path.join(dir, 'packages/ui/page.html'); + fs.writeFileSync(page, PAGE_HTML); + + const findings = runDetect(dir, [page]); + assert.ok( + fontFindingsFor(findings, page).some((f) => f.ignoreValue === 'verdana'), + 'Verdana must be flagged via inherited root DESIGN.md', + ); + }); + + it('turbo marker root: workspace page inherits root DESIGN.md', () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'impeccable-detect-mono-turbo-')); + tempRoots.push(dir); + fs.writeFileSync(path.join(dir, 'DESIGN.md'), DESIGN_MD); + fs.writeFileSync(path.join(dir, 'package.json'), '{"name":"mono"}'); + fs.writeFileSync(path.join(dir, 'turbo.json'), '{}'); + fs.mkdirSync(path.join(dir, 'apps/web'), { recursive: true }); + fs.writeFileSync(path.join(dir, 'apps/web/package.json'), '{"name":"web"}'); + const page = path.join(dir, 'apps/web/page.html'); + fs.writeFileSync(page, PAGE_HTML); + + const findings = runDetect(dir, [page]); + assert.ok( + fontFindingsFor(findings, page).some((f) => f.ignoreValue === 'verdana'), + 'Verdana must be flagged via inherited root DESIGN.md', + ); + }); + + it('directory target: scan apps/web dir inherits root DESIGN.md', () => { + const { dir, page, webDir } = mkPnpmMonorepo(); + const findings = runDetect(dir, [webDir]); + assert.ok( + fontFindingsFor(findings, page).some((f) => f.ignoreValue === 'verdana'), + 'Verdana must be flagged when scanning the workspace directory', + ); + }); + + it('workspace-owned DESIGN.md wins over monorepo root', () => { + const workspaceDesign = `--- +typography: + body: + fontFamily: "Verdana, sans-serif" +--- +# Workspace Design System +`; + const { dir, page } = mkPnpmMonorepo({ workspaceDesign }); + const findings = runDetect(dir, [page]); + assert.equal( + fontFindingsFor(findings, page).length, + 0, + 'workspace DESIGN.md allowing Verdana must suppress inherited root rules', + ); + }); + + it('nested separate repo inherits nothing from monorepo root', () => { + const { dir } = mkPnpmMonorepo(); + fs.mkdirSync(path.join(dir, 'vendor/other/.git'), { recursive: true }); + fs.writeFileSync(path.join(dir, 'vendor/other/package.json'), '{"name":"other"}'); + const page = path.join(dir, 'vendor/other/page.html'); + fs.writeFileSync(page, PAGE_HTML); + + const findings = runDetect(dir, [page]); + assert.equal( + fontFindingsFor(findings, page).length, + 0, + 'nested repo with no workspaces must not inherit monorepo root DESIGN.md', + ); + }); + + it('home directory is never an owning monorepo root', () => { + // context.mjs's findMonorepoRoot stops at homeDir before its monorepo + // check; the engine walk must match, or a workspace-declaring $HOME + // leaks its DESIGN.md into every git-less project beneath it. + const home = fs.mkdtempSync(path.join(os.tmpdir(), 'impeccable-detect-mono-home-')); + tempRoots.push(home); + fs.writeFileSync(path.join(home, 'DESIGN.md'), DESIGN_MD); + fs.writeFileSync(path.join(home, 'pnpm-workspace.yaml'), "packages:\n - 'apps/*'\n"); + fs.mkdirSync(path.join(home, 'project'), { recursive: true }); + fs.writeFileSync(path.join(home, 'project/package.json'), '{"name":"p"}'); + const page = path.join(home, 'project/page.html'); + fs.writeFileSync(page, PAGE_HTML); + + const findings = runDetect(home, [page], { HOME: home, USERPROFILE: home }); + assert.equal( + fontFindingsFor(findings, page).length, + 0, + 'a project under a workspace-declaring $HOME must not inherit its DESIGN.md', + ); + }); +});