ci: Explain single-instance duplicate findings instead of counting them (#36566)

This commit is contained in:
Bernhard Wittmann
2026-08-20 06:54:29 +00:00
committed by GitHub
parent 86c53a28c9
commit f87b345e38
8 changed files with 931 additions and 57 deletions
+37 -1
View File
@@ -43,7 +43,8 @@ git commit -m "chore: update code-health baseline"
## Output
All output is JSON. Exit code 1 if new violations are found, 0 if clean.
Rule output is JSON (the single-instance subcommands below print plain text). Exit code 1 if new
violations are found, 0 if clean.
```json
{
@@ -55,6 +56,41 @@ All output is JSON. Exit code 1 if new violations are found, 0 if clean.
}
```
## Single-instance dependency checks
Separate subcommands (not rules) verify that the curated single-instance libraries in
`src/single-instance/libs.ts` resolve to exactly one physical copy — a second copy breaks
`instanceof`, module singletons and cross-package schema composition at runtime.
```bash
# Verify an already-installed closure (e.g. a pruned production tree)
pnpm --dir packages/testing/code-health exec tsx src/cli.ts verify-closure <dir>
# Reproduce the `npm install` graph of published tarballs and verify that
pnpm --dir packages/testing/code-health exec tsx src/cli.ts verify-npm-install <pkgName>...
# Scopes CI uses: packages changed since a ref, or every publishable package.
# `--report-only` downgrades a finding to a warning and exits 0 (what CI passes today, while the
# curated-lib backlog is worked down).
pnpm --dir packages/testing/code-health exec tsx src/cli.ts verify-npm-install --changed=origin/master
pnpm --dir packages/testing/code-health exec tsx src/cli.ts verify-npm-install --all --report-only
```
`verify-npm-install` packs each target with `pnpm pack` and installs it with npm, because root
`pnpm.overrides` — which hide duplication locally and in `pnpm deploy` — do not travel in a
published tarball.
On a finding it prints every physical copy with the package that pulled it in, then the fix options
that apply to those requirers: move the library to `peerDependencies` (`catalog:`) in one of our
packages, align a version in a package the peer rule exempts, bump/replace the third-party package
pinning an incompatible range, or, as a last resort, add a documented `EXPECTED_DUPLICATES` entry in
`src/single-instance/collect-copies.ts`. In CI the same findings also go to the job summary, so a
finding is visible without opening the log.
A blocking run keeps its scratch install so you can walk the full requirer chain with `npm ls`; a
`--report-only` run deletes it (it is a complete `node_modules`), so re-run the same targets locally
when you need that chain.
## Adding rules
Rules extend `BaseRule<CodeHealthContext>` from `@n8n/rules-engine`. See `src/rules/catalog-violations.rule.ts` for the pattern. Register new rules in `src/index.ts`.
@@ -2,12 +2,7 @@ import { BaseRule } from '@n8n/rules-engine';
import type { Violation } from '@n8n/rules-engine';
import type { CodeHealthContext } from '../context.js';
import {
CURATED_LIBS,
FRONTEND_PATH_PREFIXES,
HOST_PACKAGES,
PEER_LIBS,
} from '../single-instance/libs.js';
import { CURATED_LIBS, isPeerRuleExempt, PEER_LIBS } from '../single-instance/libs.js';
import { REQUIRED_CURATED_PEERS } from '../single-instance/required-peers.js';
import {
findPackageJsonFiles,
@@ -52,7 +47,7 @@ export class SingleInstanceLibsRule extends BaseRule<CodeHealthContext> {
),
});
if (this.isExempt(pkg.packageName, relativeDir(rootDir, file))) continue;
if (isPeerRuleExempt(pkg.packageName, relativeDir(rootDir, file))) continue;
for (const dep of pkg.deps) {
if (PEER_LIBS.includes(dep.name) && RUNTIME_SECTIONS.has(dep.section)) {
@@ -106,11 +101,4 @@ export class SingleInstanceLibsRule extends BaseRule<CodeHealthContext> {
return violations;
}
private isExempt(packageName: string, relDir: string): boolean {
return (
HOST_PACKAGES.includes(packageName) ||
FRONTEND_PATH_PREFIXES.some((prefix) => relDir.startsWith(prefix))
);
}
}
@@ -12,6 +12,8 @@ export interface DuplicateGroup {
name: string;
isCurated: boolean;
allowed: boolean;
/** Why the duplicate is tolerated, when it is allowlisted. */
reason?: string;
copies: Copy[];
}
@@ -155,6 +157,7 @@ export function analyze(
name,
isCurated: CURATED_LIBS.includes(name),
allowed: Object.hasOwn(allowlist, name),
reason: allowlist[name],
copies: distinct,
});
}
@@ -0,0 +1,341 @@
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
import { analyze, collectCopies } from './collect-copies.js';
import {
attributeCopy,
describeOrigin,
explainDuplicates,
formatCopyLines,
formatCuratedReport,
formatRemediation,
formatStepSummary,
} from './explain-duplicates.js';
// Planted on disk: nesting is how npm records "this package forced this copy", so attribution is
// exercised against a real tree rather than a hand-built path string.
let ROOT: string;
const WORKSPACE = new Set(['@n8n/api-types', 'n8n']);
const NO_EXEMPTIONS = new Set<string>();
function pkg(relDir: string, manifest: Record<string, unknown>): void {
const dir = join(ROOT, relDir);
mkdirSync(dir, { recursive: true });
writeFileSync(join(dir, 'package.json'), JSON.stringify(manifest));
}
beforeAll(() => {
ROOT = mkdtempSync(join(tmpdir(), 'explain-duplicates-'));
// hoisted copy npm picked for the root project
pkg('node_modules/zod', { name: 'zod', version: '4.4.3' });
// a workspace package of ours pinning its own copy -> fix is a manifest change in this repo
pkg('node_modules/@n8n/api-types', {
name: '@n8n/api-types',
version: '1.0.0',
dependencies: { zod: '^3.25.0' },
});
pkg('node_modules/@n8n/api-types/node_modules/zod', { name: 'zod', version: '3.25.76' });
// a third-party package pinning an incompatible range -> fix is upstream / the catalog
pkg('node_modules/third-party', {
name: 'third-party',
version: '2.0.0',
peerDependencies: { zod: '~3.20.0' },
});
pkg('node_modules/third-party/node_modules/zod', { name: 'zod', version: '3.20.1' });
// nested under a package that does not declare the lib itself
pkg('node_modules/silent', { name: 'silent', version: '1.0.0' });
pkg('node_modules/silent/node_modules/zod', { name: 'zod', version: '3.1.0' });
});
afterAll(() => rmSync(ROOT, { recursive: true, force: true }));
const attribute = (relPath: string, version: string, lib = 'zod') =>
attributeCopy(ROOT, lib, { realPath: join(ROOT, relPath), version }, WORKSPACE);
const explainAll = () => explainDuplicates(ROOT, analyze(collectCopies(ROOT)).failures, WORKSPACE);
describe('attributeCopy', () => {
it('marks a top-level copy as hoisted', () => {
const copy = attribute('node_modules/zod', '4.4.3');
expect(copy.requiredBy).toBeNull();
expect(describeOrigin(copy)).toBe('hoisted at the top level');
});
it('attributes a nested copy to the package that declares the range', () => {
const copy = attribute('node_modules/third-party/node_modules/zod', '3.20.1');
expect(copy).toMatchObject({ requiredBy: 'third-party', range: '~3.20.0', isWorkspace: false });
expect(describeOrigin(copy)).toBe('required by third-party (peerDependencies "~3.20.0")');
});
// Which section declared it decides the fix, so attribution has to keep it.
it('records the section the range came from', () => {
expect(attribute('node_modules/@n8n/api-types/node_modules/zod', '3.25.76').section).toBe(
'dependencies',
);
expect(attribute('node_modules/third-party/node_modules/zod', '3.20.1').section).toBe(
'peerDependencies',
);
expect(attribute('node_modules/silent/node_modules/zod', '3.1.0').section).toBeNull();
});
it('flags a requirer that is one of our own packages', () => {
const copy = attribute('node_modules/@n8n/api-types/node_modules/zod', '3.25.76');
expect(copy).toMatchObject({ requiredBy: '@n8n/api-types', isWorkspace: true });
expect(describeOrigin(copy)).toContain('[workspace package]');
});
it('falls back to the enclosing package when no ancestor declares the lib', () => {
const copy = attribute('node_modules/silent/node_modules/zod', '3.1.0');
expect(copy).toMatchObject({ requiredBy: 'silent', range: null });
expect(describeOrigin(copy)).toBe('nested under silent (no direct declaration)');
});
// A store key is not a requirer, so naming it as one would be confidently wrong.
it('reports a copy inside a pnpm virtual store as such, not as hoisted', () => {
// Its own root: a store entry in the shared tree would count as another copy of zod.
const storeRoot = join(ROOT, 'store');
const relPath = 'node_modules/.pnpm/zod@3.25.76/node_modules/zod';
pkg(join('store', relPath), { name: 'zod', version: '3.25.76' });
const copy = attributeCopy(
storeRoot,
'zod',
{ realPath: join(storeRoot, relPath), version: '3.25.76' },
WORKSPACE,
);
expect(copy.requiredBy).toBeNull();
expect(describeOrigin(copy)).toBe(
'in the pnpm virtual store (requirer not derivable from the path)',
);
});
// devDependencies are not installed for a consumer, so they cannot nest a copy — a dev-only
// declaration must not be reported as the requirer.
it('ignores a declaration that cannot nest a copy', () => {
const devRoot = join(ROOT, 'dev-only');
pkg('dev-only/node_modules/zod', { name: 'zod', version: '4.4.3' });
pkg('dev-only/node_modules/tool', {
name: 'tool',
version: '1.0.0',
devDependencies: { zod: '^3.0.0' },
});
pkg('dev-only/node_modules/tool/node_modules/zod', { name: 'zod', version: '3.25.76' });
const copy = attributeCopy(
devRoot,
'zod',
{ realPath: join(devRoot, 'node_modules/tool/node_modules/zod'), version: '3.25.76' },
WORKSPACE,
);
expect(copy).toMatchObject({ requiredBy: 'tool', range: null });
});
});
describe('explainDuplicates', () => {
it('attributes every copy of a real collected duplicate', () => {
const [zod] = explainAll();
expect(zod.name).toBe('zod');
expect(zod.copies.map((c) => c.requiredBy)).toEqual([
null,
'@n8n/api-types',
'silent',
'third-party',
]);
});
it('prints the version, the origin and the path of each copy', () => {
const output = formatCopyLines(explainAll()[0].copies).join('\n');
expect(output).toContain('v3.20.1');
expect(output).toContain('required by third-party (peerDependencies "~3.20.0")');
expect(output).toContain('node_modules/third-party/node_modules/zod');
});
});
describe('formatCuratedReport', () => {
it('reports every curated library, and the caller renders the failing copies', () => {
const found = collectCopies(ROOT);
const { duplicates } = analyze(found);
const output = formatCuratedReport(found, duplicates, () => [' <detail>']).join('\n');
expect(output).toContain('zod: FAIL — 4 copies, expected 1:');
expect(output).toContain('<detail>');
// A curated lib absent from the closure is still listed, so a run always states the verdict
// for the full enforced set rather than only for what happened to be installed.
expect(output).toContain('reflect-metadata: not present');
});
// Which copies an entry covers is what tells you whether it is still accurate, so an allowlisted
// duplicate lists them too rather than only its reason.
it('prints the copies of an allowlisted duplicate and why it is tolerated', () => {
const found = collectCopies(ROOT);
const { duplicates } = analyze(found, { allowlist: { zod: 'tolerated for <reason>' } });
const output = formatCuratedReport(found, duplicates, () => [' <copy>']).join('\n');
expect(output).toContain('zod: ALLOWED DUP');
expect(output).toContain('<copy>');
expect(output).toContain('allowlisted: tolerated for <reason>');
});
});
describe('formatRemediation', () => {
it('tells our own packages to move the library to peerDependencies', () => {
const output = formatRemediation(explainAll(), { exemptPackages: NO_EXEMPTIONS }).join('\n');
expect(output).toContain('- zod <- @n8n/api-types (dependencies "^3.25.0")');
expect(output).toContain('"peerDependencies"');
});
// The `single-instance-libs` rule exempts host packages, so telling their owner to move a
// curated lib to peerDependencies would send them into a change that rule rejects.
it('does not propose a peer move for a package the peer rule exempts', () => {
pkg('host/node_modules/zod', { name: 'zod', version: '4.4.3' });
pkg('host/node_modules/n8n', {
name: 'n8n',
version: '1.0.0',
dependencies: { zod: '^3.0.0' },
});
pkg('host/node_modules/n8n/node_modules/zod', { name: 'zod', version: '3.25.76' });
const hostRoot = join(ROOT, 'host');
const explained = explainDuplicates(
hostRoot,
analyze(collectCopies(hostRoot)).failures,
WORKSPACE,
);
const output = formatRemediation(explained, { exemptPackages: new Set(['n8n']) }).join('\n');
expect(output).toContain('legitimately own a copy');
expect(output).toContain('- zod <- n8n (dependencies "^3.0.0")');
expect(output).not.toContain('"peerDependencies"');
});
// The compliant shape: @n8n/api-types, n8n-core and n8n-workflow all declare zod only as a peer,
// so they are the likeliest requirers in the real closure — and there is nothing for them to move.
it('does not propose a peer move for a package that already declares the peer', () => {
const peerRoot = join(ROOT, 'peer');
pkg('peer/node_modules/zod', { name: 'zod', version: '4.4.3' });
pkg('peer/node_modules/@n8n/api-types', {
name: '@n8n/api-types',
version: '1.0.0',
peerDependencies: { zod: '3.25.76' },
});
pkg('peer/node_modules/@n8n/api-types/node_modules/zod', { name: 'zod', version: '3.25.76' });
const explained = explainDuplicates(
peerRoot,
analyze(collectCopies(peerRoot)).failures,
WORKSPACE,
);
const output = formatRemediation(explained, { exemptPackages: NO_EXEMPTIONS }).join('\n');
expect(output).toContain('already declare the library as a peer');
expect(output).toContain('- zod <- @n8n/api-types (peerDependencies "3.25.76")');
expect(output).not.toContain('Move it to "peerDependencies"');
});
// reflect-metadata is curated but pin-only, so the peer rule does not cover it either.
it('does not propose a peer move for a curated library outside the peer rule', () => {
const copy = attribute(
'node_modules/@n8n/api-types/node_modules/zod',
'3.25.76',
'reflect-metadata',
);
const explained = [
{
name: 'reflect-metadata',
copies: [{ ...copy, requiredBy: '@n8n/api-types', range: '^0.2.0', isWorkspace: true }],
},
];
const output = formatRemediation(explained, { exemptPackages: NO_EXEMPTIONS }).join('\n');
expect(output).toContain('legitimately own a copy');
expect(output).not.toContain('"peerDependencies"');
});
it('names the third-party requirer and its range', () => {
expect(formatRemediation(explainAll(), { exemptPackages: NO_EXEMPTIONS }).join('\n')).toContain(
'- zod <- third-party (peerDependencies "~3.20.0")',
);
});
it('numbers the steps consecutively whichever ones apply', () => {
const output = formatRemediation(explainAll(), {
exemptPackages: new Set(['@n8n/api-types']),
}).join('\n');
expect(output).toContain(' 1. Our own packages that legitimately own a copy');
expect(output).toContain(' 2. Third-party packages');
expect(output).toContain(' 3. Copies nested under a package that does not declare');
expect(output).toContain(' 4. If the split cannot be removed yet');
});
it('points at the allowlist as the last resort', () => {
expect(formatRemediation(explainAll(), { exemptPackages: NO_EXEMPTIONS }).join('\n')).toContain(
'EXPECTED_DUPLICATES',
);
});
it('mentions the kept install tree only when one is kept', () => {
const options = { exemptPackages: NO_EXEMPTIONS };
expect(
formatRemediation(explainAll(), { ...options, scratch: '/tmp/kept' }).join('\n'),
).toContain('npm ls --all --prefix /tmp/kept zod');
expect(formatRemediation(explainAll(), options).join('\n')).not.toContain('--prefix');
});
});
describe('formatStepSummary', () => {
it('renders a table row per copy and marks an advisory run as such', () => {
const summary = formatStepSummary(explainAll(), {
reportOnly: true,
exemptPackages: NO_EXEMPTIONS,
});
expect(summary).toContain('(advisory)');
expect(summary).toContain('| Library | Version | Pulled in by | Path |');
expect(summary.match(/^\| zod \|/gm)).toHaveLength(4);
});
it('does not mark a blocking run as advisory, and carries the kept tree', () => {
const summary = formatStepSummary(explainAll(), {
reportOnly: false,
exemptPackages: NO_EXEMPTIONS,
scratch: '/tmp/kept',
});
expect(summary).not.toContain('advisory');
expect(summary).toContain('npm ls --all --prefix /tmp/kept');
});
// A `|` ends a table cell even inside a code span, and `||` ranges are common on curated libs.
it('escapes dependency-controlled text so a range cannot break the table', () => {
const summary = formatStepSummary(
[
{
name: 'zod',
copies: [
{
version: '3.20.1',
path: 'node_modules/pkg/node_modules/zod',
requiredBy: 'pkg',
range: '^3.22.0 || ^4.0.0',
section: 'dependencies' as const,
isWorkspace: false,
inPnpmStore: false,
},
],
},
],
{ reportOnly: true, exemptPackages: NO_EXEMPTIONS },
);
const row = summary.split('\n').find((line) => line.startsWith('| zod |'));
expect(row).toContain('\\|\\|');
expect(row?.split(/(?<!\\)\|/)).toHaveLength(6); // 4 cells + the empty ends
});
});
@@ -0,0 +1,396 @@
import { readFileSync, realpathSync } from 'node:fs';
import { join, relative, sep } from 'node:path';
import type { Copy, DuplicateGroup } from './collect-copies.js';
import { distinctCopies } from './collect-copies.js';
import { CURATED_LIBS, PEER_LIBS, PUBLISHED_SECTIONS } from './libs.js';
/** A manifest section a copy can be declared in. */
type Section = (typeof PUBLISHED_SECTIONS)[number];
export interface AttributedCopy {
version: string;
/** Path within the install tree — the nesting chain itself reads as the cause. */
path: string;
/** Nearest enclosing package that declares the lib; `null` when the path does not name one. */
requiredBy: string | null;
/** Range `requiredBy` declares, when it declares one directly. */
range: string | null;
/**
* Section the range came from. Which one decides the fix: a package that already declares the
* library as a peer has nothing to move, so the split is a version conflict, not a wrong shape.
*/
section: Section | null;
/** `requiredBy` is a package in this repo, so the fix is a manifest change here. */
isWorkspace: boolean;
/** Copy lives in a pnpm virtual store, whose nesting names no requirer. */
inPnpmStore: boolean;
}
function toPosix(p: string): string {
return sep === '/' ? p : p.split(sep).join('/');
}
function resolveRealPath(p: string): string {
try {
return realpathSync(p);
} catch {
return p;
}
}
/**
* Split an install-tree-relative copy path into the chain of package names it is nested in,
* outermost first, with the copy itself last:
* `node_modules/a/node_modules/zod` -> `['a', 'zod']`.
*/
function packageChain(relPath: string): string[] {
return toPosix(relPath)
.replace(/^node_modules\//, '')
.split('/node_modules/');
}
type Manifest = Record<string, Record<string, string> | undefined>;
function isManifest(value: unknown): value is Manifest {
return typeof value === 'object' && value !== null;
}
interface Declaration {
section: Section;
range: string;
}
function readDeclaration(dir: string, lib: string): Declaration | null {
let parsed: unknown;
try {
parsed = JSON.parse(readFileSync(join(dir, 'package.json'), 'utf8'));
} catch {
return null;
}
if (!isManifest(parsed)) return null;
for (const section of PUBLISHED_SECTIONS) {
const range = parsed[section]?.[lib];
if (typeof range === 'string' && range) return { section, range };
}
return null;
}
/**
* Explain where a physical copy came from. npm nests a copy inside the package that forced it, so
* the nearest enclosing package that declares the lib is the one to fix — that attribution is the
* difference between "zod has 5 copies" and "this dependency pins an incompatible range".
*
* Nesting only carries that meaning in an `npm install` tree. A pnpm virtual store nests every
* package under a `<name>@<version>` store key instead, so the path names no requirer and this
* reports the copy as unattributed rather than presenting a store directory as one.
*/
export function attributeCopy(
root: string,
lib: string,
copy: Copy,
workspaceNames: Set<string>,
): AttributedCopy {
// `collectCopies` records realpaths, so the root has to be resolved too — otherwise a symlinked
// tmpdir (every macOS `/var` path) makes the install path relative to nothing useful.
const resolvedRoot = resolveRealPath(root);
const path = toPosix(relative(resolvedRoot, resolveRealPath(copy.realPath)));
const chain = packageChain(path);
const inPnpmStore = path.startsWith('node_modules/.pnpm/');
const ancestors = inPnpmStore ? [] : chain.slice(0, -1);
const base = { version: copy.version, path, inPnpmStore };
let dir = resolvedRoot;
const ancestorDirs = ancestors.map((name) => (dir = join(dir, 'node_modules', name)));
// Nearest ancestor first: the innermost declaration is the one that forced this copy.
for (let i = ancestorDirs.length - 1; i >= 0; i--) {
const declared = readDeclaration(ancestorDirs[i], lib);
if (declared) {
const requiredBy = ancestors[i];
return {
...base,
requiredBy,
...declared,
isWorkspace: workspaceNames.has(requiredBy),
};
}
}
// Nested with no declaring ancestor (npm nesting for a deeper conflict), the hoisted copy, or a
// tree shape whose nesting names no requirer.
const nearest = ancestors.at(-1) ?? null;
return {
...base,
requiredBy: nearest,
range: null,
section: null,
isWorkspace: nearest !== null && workspaceNames.has(nearest),
};
}
export interface ExplainedDuplicate {
name: string;
copies: AttributedCopy[];
}
export function explainDuplicates(
root: string,
groups: DuplicateGroup[],
workspaceNames: Set<string>,
): ExplainedDuplicate[] {
return groups.map((group) => ({
name: group.name,
copies: group.copies
.map((copy) => attributeCopy(root, group.name, copy, workspaceNames))
// Hoisted copy first — it is the version npm picked, so every nested copy below it reads
// as "and this package refused that one".
.sort(
(a, b) => Number(!isHoisted(a)) - Number(!isHoisted(b)) || a.path.localeCompare(b.path),
),
}));
}
function isHoisted(copy: AttributedCopy): boolean {
return copy.requiredBy === null && !copy.inPnpmStore;
}
/** One-line origin for a copy, e.g. `required by @langchain/community ("~0.1.29")`. */
export function describeOrigin(copy: AttributedCopy): string {
if (copy.inPnpmStore) return 'in the pnpm virtual store (requirer not derivable from the path)';
if (copy.requiredBy === null) return 'hoisted at the top level';
const marker = copy.isWorkspace ? ' [workspace package]' : '';
return copy.range === null
? `nested under ${copy.requiredBy} (no direct declaration)${marker}`
: `required by ${copy.requiredBy} (${copy.section} "${copy.range}")${marker}`;
}
/** One indented block per physical copy: version, what pulled it in, and where it landed. */
export function formatCopyLines(copies: AttributedCopy[]): string[] {
return copies.flatMap((copy) => [
` v${copy.version} ${describeOrigin(copy)}`,
` ${copy.path}`,
]);
}
/**
* Per-curated-library verdict, in `CURATED_LIBS` order: the enforced list every run prints,
* whatever closure it inspected. `renderCopies` is the caller's detail for a failing library — an
* npm-install run can name the requirer of each copy, a pnpm-shaped closure can only list paths.
*/
export function formatCuratedReport(
found: Map<string, Copy[]>,
duplicates: DuplicateGroup[],
renderCopies: (dup: DuplicateGroup) => string[],
): string[] {
const byName = new Map(duplicates.map((d) => [d.name, d]));
return CURATED_LIBS.flatMap((lib) => {
const dup = byName.get(lib);
if (!dup) {
const copies = distinctCopies(found.get(lib) ?? []);
return [
` ${lib}: ${copies.length === 0 ? 'not present' : `OK (1 copy, v${copies[0].version})`}`,
];
}
if (dup.allowed) {
return [
` ${lib}: ALLOWED DUP — ${dup.copies.length} physical copies:`,
...renderCopies(dup),
` allowlisted: ${dup.reason}`,
];
}
return [` ${lib}: FAIL — ${dup.copies.length} copies, expected 1:`, ...renderCopies(dup)];
});
}
/** The verdict sentence, shared so both commands phrase a finding identically. */
export function describeFailureCount(count: number): string {
return count === 1
? '1 curated library resolves to more than one physical copy'
: `${count} curated libraries each resolve to more than one physical copy`;
}
const LIBS_FILE = 'packages/testing/code-health/src/single-instance/libs.ts';
const ALLOWLIST_FILE = 'packages/testing/code-health/src/single-instance/collect-copies.ts';
interface Requirer {
lib: string;
requiredBy: string;
range: string | null;
section: Section | null;
isWorkspace: boolean;
}
function describeRequirer({ lib, requiredBy, range, section }: Requirer): string {
return ` - ${lib} <- ${requiredBy}${range === null ? '' : ` (${section} "${range}")`}`;
}
/** Which remediation a requirer needs. One bucket per fix, so every copy is accounted for. */
type Bucket = 'peerMove' | 'peerConflict' | 'workspaceExempt' | 'thirdParty' | 'indirect';
function bucketFor(requirer: Requirer, exemptPackages: Set<string>): Bucket {
// No declared range means the requirer of record does not ask for the lib itself — something
// deeper in its own graph does, and none of the manifest fixes below apply to it.
if (requirer.range === null) return 'indirect';
if (!requirer.isWorkspace) return 'thirdParty';
// Already a peer: the shape the peer rule asks for. Nothing to move — the copy exists because
// the graph resolved a version outside this range, which is a pin problem elsewhere.
if (requirer.section === 'peerDependencies') return 'peerConflict';
// A lib the peer rule does not cover (pin-only, e.g. reflect-metadata) or a package it exempts
// is legitimately allowed its own dependency; only a version split is left to fix.
return PEER_LIBS.includes(requirer.lib) && !exemptPackages.has(requirer.requiredBy)
? 'peerMove'
: 'workspaceExempt';
}
/**
* Remediation steps, ordered by what the findings actually show. `exemptPackages` are the workspace
* packages `isPeerRuleExempt` covers — telling their owner to move a curated lib to
* `peerDependencies` would send them into a change that rule then rejects.
*/
export function formatRemediation(
explained: ExplainedDuplicate[],
{ exemptPackages, scratch }: { exemptPackages: Set<string>; scratch?: string },
): string[] {
// Keyed, not per copy: npm installs the same requirer at several nesting depths, and the fix is
// one manifest edit however many copies it produced.
const requirers = new Map<string, Requirer>();
for (const dup of explained) {
for (const { requiredBy, range, section, isWorkspace } of dup.copies) {
if (requiredBy === null) continue;
requirers.set(`${dup.name}|${requiredBy}|${section}|${range}`, {
lib: dup.name,
requiredBy,
range,
section,
isWorkspace,
});
}
}
const inBucket = (bucket: Bucket): Requirer[] =>
[...requirers.values()].filter((r) => bucketFor(r, exemptPackages) === bucket);
const peerMove = inBucket('peerMove');
const peerConflict = inBucket('peerConflict');
const workspaceExempt = inBucket('workspaceExempt');
const thirdParty = inBucket('thirdParty');
const indirect = inBucket('indirect');
const lines = [
' A curated library must resolve to ONE physical copy per process — a second copy breaks',
` instanceof, module singletons and cross-package schema composition (${LIBS_FILE}).`,
'',
];
let step = 0;
if (peerMove.length > 0) {
lines.push(
` ${++step}. Our own packages declaring the library as a runtime dependency:`,
...peerMove.map(describeRequirer),
' Move it to "peerDependencies" with "catalog:" in that package.json, and keep it in',
' devDependencies (also "catalog:") so local builds still resolve it.',
'',
);
}
if (peerConflict.length > 0) {
lines.push(
` ${++step}. Our own packages that already declare the library as a peer:`,
...peerConflict.map(describeRequirer),
' Nothing to move — this copy exists because something else in the graph resolved a',
' version outside that range. Align the "catalog:" pin, or fix the requirer that forced',
' the other version (it is listed under one of the other steps).',
'',
);
}
if (workspaceExempt.length > 0) {
lines.push(
` ${++step}. Our own packages that legitimately own a copy (host package, or a library the`,
' peer rule does not cover):',
...workspaceExempt.map(describeRequirer),
' A peer move is not the fix here — align the declared version with "catalog:" so this',
' copy and the rest of the graph resolve to the same one.',
'',
);
}
if (thirdParty.length > 0) {
lines.push(
` ${++step}. Third-party packages pinning an incompatible range:`,
...thirdParty.map(describeRequirer),
' Bump or replace that dependency, or move the catalog version in pnpm-workspace.yaml to a',
' version its range accepts. Root "pnpm.overrides" will NOT help: they do not travel in a',
' published tarball, which is exactly what this check reproduces.',
'',
);
}
if (indirect.length > 0) {
lines.push(
` ${++step}. Copies nested under a package that does not declare the library itself:`,
...indirect.map(describeRequirer),
" The requirer is deeper in that package's own dependency graph — find it with",
' "npm ls --all <library>" in the install tree before picking a fix.',
'',
);
}
lines.push(
` ${++step}. If the split cannot be removed yet, add an EXPECTED_DUPLICATES entry in`,
` ${ALLOWLIST_FILE} documenting why it is tolerated and what removes it.`,
'',
' Reproduce locally (packs + installs the same closure, no CI needed):',
' pnpm --dir packages/testing/code-health exec tsx src/cli.ts verify-npm-install <pkgName>...',
' (or --changed=origin/master to reproduce the scope this job used)',
);
if (scratch) {
lines.push(
'',
` The install tree is kept at ${scratch} — for a full requirer chain, run:`,
` npm ls --all --prefix ${scratch} ${explained.map((d) => d.name).join(' ')}`,
);
}
return lines;
}
/**
* Table cells carry dependency-controlled text (names, semver ranges, paths). In GFM a `|` ends the
* cell even inside a code span — and `||` ranges are common on curated libs — so a raw range would
* shift every following column. Newlines would end the row outright.
*/
function cell(text: string): string {
return text.replace(/[\\|`]/g, '\\$&').replace(/\s*[\r\n]+\s*/g, ' ');
}
// One backtick more than any fence a dependency's own text could contain, so the fix block cannot
// be closed early and the rest of the summary reinterpreted as markdown.
const FENCE = '````';
/** GitHub job-summary markdown — the findings surface without opening a 5k-line log. */
export function formatStepSummary(
explained: ExplainedDuplicate[],
{
reportOnly,
exemptPackages,
scratch,
}: { reportOnly: boolean; exemptPackages: Set<string>; scratch?: string },
): string {
const rows = explained.flatMap((dup) =>
dup.copies.map(
(copy) =>
`| ${[dup.name, copy.version, describeOrigin(copy), copy.path].map(cell).join(' | ')} |`,
),
);
return [
`### Single-instance deps: duplicates found${reportOnly ? ' (advisory)' : ''}`,
'',
`${describeFailureCount(explained.length)} in the \`npm install\` graph.`,
'',
'| Library | Version | Pulled in by | Path |',
'| --- | --- | --- | --- |',
...rows,
'',
'<details><summary>How to fix</summary>',
'',
FENCE,
...formatRemediation(explained, { exemptPackages, scratch }),
FENCE,
'',
'</details>',
'',
].join('\n');
}
@@ -7,6 +7,16 @@
* `verify-npm-install` CLI subcommands.
*/
/**
* Manifest sections that follow the publish graph: what a consumer installs, and therefore what can
* nest a second physical copy. devDependencies don't ship, so they can't.
*/
export const PUBLISHED_SECTIONS = [
'dependencies',
'peerDependencies',
'optionalDependencies',
] as const;
/** Libraries a single process must resolve to exactly one physical copy of. */
export const CURATED_LIBS = ['zod', 'form-data', '@langchain/core', 'reflect-metadata'];
@@ -31,3 +41,15 @@ export const FRONTEND_PATH_PREFIXES = ['packages/frontend/'];
/** Curated libs subject to the peer rule (pin-only libs are exempt). */
export const PEER_LIBS = CURATED_LIBS.filter((lib) => !PIN_ONLY_LIBS.includes(lib));
/**
* Whether the peer rule exempts a package: it provides its own runtime instance, or it bundles.
* The `single-instance-libs` rule skips these, and the duplicate report must not tell their owner
* to make a change that rule then rejects — so both read the exemption from here.
*/
export function isPeerRuleExempt(packageName: string, relDir: string): boolean {
return (
HOST_PACKAGES.includes(packageName) ||
FRONTEND_PATH_PREFIXES.some((prefix) => relDir.startsWith(prefix))
);
}
@@ -1,5 +1,5 @@
import { analyze, collectCopies, distinctCopies, EXPECTED_DUPLICATES } from './collect-copies.js';
import { CURATED_LIBS } from './libs.js';
import { analyze, collectCopies } from './collect-copies.js';
import { describeFailureCount, formatCuratedReport } from './explain-duplicates.js';
/**
* Exit code for "the closure was checked and curated duplicates were found".
@@ -21,23 +21,13 @@ export function runVerifyClosure(dir: string): number {
const { duplicates, failures } = analyze(found);
console.log(`\nSingle-instance dependency verifier — root: ${dir}`);
const curatedDups = new Map(duplicates.filter((d) => d.isCurated).map((d) => [d.name, d]));
console.log('\nCurated single-instance libraries (enforced):');
for (const lib of CURATED_LIBS) {
const dup = curatedDups.get(lib);
if (!dup) {
const copies = distinctCopies(found.get(lib) ?? []);
console.log(
` ${lib}: ${copies.length === 0 ? 'not present' : `OK (1 copy, v${copies[0].version})`}`,
);
continue;
}
console.log(
` ${lib}: ${dup.allowed ? 'ALLOWED DUP' : 'FAIL'}${dup.copies.length} physical copies:`,
);
for (const c of dup.copies) console.log(` v${c.version} ${c.realPath}`);
if (dup.allowed) console.log(` allowlisted: ${EXPECTED_DUPLICATES[lib]}`);
}
// Paths, not requirers: this runs against a pruned `pnpm deploy` closure, whose nesting is a
// virtual store rather than "package X forced this copy".
const report = formatCuratedReport(found, duplicates, (dup) =>
dup.copies.map((c) => ` v${c.version} ${c.realPath}`),
);
for (const line of report) console.log(line);
const otherDups = duplicates.filter((d) => !d.isCurated);
if (otherDups.length > 0) {
@@ -51,8 +41,10 @@ export function runVerifyClosure(dir: string): number {
console.log('');
if (failures.length > 0) {
console.error(
`FAIL: curated ${failures.length === 1 ? 'library resolves' : 'libraries resolve'} to multiple physical copies: ${failures.map((f) => f.name).join(', ')}`,
// stdout, not stderr: the streams interleave in CI logs, and build-n8n.mjs pipes stdout — a
// verdict on stderr can land in the middle of the list it summarises, or out of view.
console.log(
`FAIL: ${describeFailureCount(failures.length)}: ${failures.map((f) => f.name).join(', ')}`,
);
return EXIT_DUPLICATES_FOUND;
}
@@ -1,10 +1,26 @@
import { execFileSync } from 'node:child_process';
import { mkdirSync, mkdtempSync, readdirSync, rmSync, writeFileSync } from 'node:fs';
import {
appendFileSync,
mkdirSync,
mkdtempSync,
readdirSync,
rmSync,
writeFileSync,
} from 'node:fs';
import { tmpdir } from 'node:os';
import { dirname, join } from 'node:path';
import { setTimeout } from 'node:timers/promises';
import { analyze, collectCopies } from './collect-copies.js';
import {
describeFailureCount,
explainDuplicates,
formatCopyLines,
formatCuratedReport,
formatRemediation,
formatStepSummary,
} from './explain-duplicates.js';
import { isPeerRuleExempt, PUBLISHED_SECTIONS } from './libs.js';
import type { PackageJsonInfo } from '../utils/package-json-scanner.js';
import {
findPackageJsonFiles,
@@ -16,8 +32,7 @@ import {
// root manifest, or the single-instance tooling itself) — a scoped diff would otherwise miss them.
const ROOT_TRIGGERS = ['pnpm-workspace.yaml', 'package.json', 'packages/testing/code-health/'];
// Sections that follow the publish graph — devDependencies don't ship, so they're not packed.
const CLOSURE_SECTIONS = new Set(['dependencies', 'peerDependencies', 'optionalDependencies']);
const CLOSURE_SECTIONS = new Set<string>(PUBLISHED_SECTIONS);
/** True when a changed file can shift dependency resolution repo-wide, forcing a full check. */
export function filesTriggerFullRun(files: string[]): boolean {
@@ -25,11 +40,41 @@ export function filesTriggerFullRun(files: string[]): boolean {
}
/**
* Surface a finding in the Actions UI. Report-only runs exit 0, and nobody opens the log of a
* green check — without an annotation the finding is indistinguishable from silence.
* Surface a finding in the Actions UI: `error` when the run blocks, `warning` when it is advisory.
* An advisory run exits 0, and nobody opens the log of a green check — without the annotation the
* finding is indistinguishable from silence.
*/
function annotate(message: string): void {
if (process.env.GITHUB_ACTIONS) console.log(`::warning::${message}`);
function annotate(message: string, level: 'warning' | 'error'): void {
if (process.env.GITHUB_ACTIONS) console.log(`::${level}::${message}`);
}
/**
* Fold a noisy section away — collapsed in Actions, a plain header locally. The npm-install log and
* the non-curated duplicate list are ~600 lines between them, and burying the verdict in them is
* what makes a finding read as "there was an error".
*/
function groupStart(title: string): void {
console.log(process.env.GITHUB_ACTIONS ? `::group::${title}` : `\n--- ${title}`);
}
function groupEnd(): void {
if (process.env.GITHUB_ACTIONS) console.log('::endgroup::');
}
/** Put the findings where they are seen without opening the log at all. */
function writeStepSummary(markdown: string): void {
const file = process.env.GITHUB_STEP_SUMMARY;
if (!file) return;
try {
appendFileSync(file, markdown);
} catch (error) {
const reason = error instanceof Error ? error.message : String(error);
console.log(`Could not write the job summary: ${reason}`);
}
}
function seconds(startedAt: number): string {
return `${Math.round((Date.now() - startedAt) / 1000)}s`;
}
/**
@@ -53,6 +98,12 @@ async function installWithRetry(scratch: string, attempts = 3): Promise<void> {
}
}
/** Workspace packages the peer model exempts — the repo paths `isPeerRuleExempt` needs live here. */
function exemptPackages(byName: Map<string, WorkspacePkg>): Set<string> {
const names = [...byName].filter(([name, { relDir }]) => isPeerRuleExempt(name, relDir));
return new Set(names.map(([name]) => name));
}
interface WorkspacePkg {
dir: string;
relDir: string;
@@ -158,7 +209,7 @@ export function resolveTargets(
console.log('No changed publishable packages; nothing to verify.');
return null;
}
console.log(`Changed publishable packages: ${targets.join(', ')}`);
console.log(`Changed publishable packages (${targets.length}): ${targets.join(', ')}`);
return targets;
}
return args.filter((a) => !a.startsWith('--'));
@@ -202,6 +253,7 @@ export async function runVerifyNpmInstall(args: string[], rootDir: string): Prom
// inspecting — either way, keeping it just leaks a full node_modules into tmpdir per run.
let keepScratch = false;
try {
const packStartedAt = Date.now();
console.log(`Packing ${toPack.length} workspace package(s) (targets: ${targets.length})...`);
const tarballByName: Record<string, string> = {};
for (const name of toPack) {
@@ -231,36 +283,80 @@ export async function runVerifyNpmInstall(args: string[], rootDir: string): Prom
),
);
console.log('Installing into scratch dir with npm...');
await installWithRetry(scratch);
console.log(`Packed in ${seconds(packStartedAt)}. Installing into scratch dir with npm...`);
const installStartedAt = Date.now();
// npm prints hundreds of ERESOLVE warnings here; they matter only when the install itself
// fails, so they are folded away rather than dropped. Actions cannot un-collapse a group
// after the fact, so a failure points back at it instead.
groupStart('npm install output');
try {
await installWithRetry(scratch);
} catch (error) {
groupEnd();
console.log('npm install failed — its output is in the "npm install output" group above.');
throw error;
}
groupEnd();
console.log(`Installed in ${seconds(installStartedAt)}.`);
const { duplicates, failures } = analyze(collectCopies(scratch));
const found = collectCopies(scratch);
const { duplicates, failures } = analyze(found);
const workspaceNames = new Set(byName.keys());
const curatedDuplicates = duplicates.filter((d) => d.isCurated);
const attributed = explainDuplicates(scratch, curatedDuplicates, workspaceNames);
const explained = attributed.filter((dup) => failures.some((f) => f.name === dup.name));
console.log(`\nnpm-install closure — scratch: ${scratch}\n`);
const curatedTag = reportOnly ? 'CURATED DUP (report)' : 'FAIL';
for (const d of duplicates) {
const tag = d.isCurated ? (d.allowed ? 'ALLOWED DUP' : curatedTag) : 'dup (report)';
console.log(
` ${d.name}: ${tag} ${d.copies.length} copies (${d.copies.map((c) => `v${c.version}`).join(', ')})`,
);
const nonCurated = duplicates.filter((d) => !d.isCurated);
if (nonCurated.length > 0) {
groupStart(`Other duplicated packages (report-only, NOT enforced — ${nonCurated.length})`);
for (const d of nonCurated) {
console.log(
` ${d.name}: ${d.copies.length} copies (${d.copies.map((c) => `v${c.version}`).join(', ')})`,
);
}
groupEnd();
}
const attributedCopies = new Map(attributed.map((dup) => [dup.name, dup.copies]));
console.log(
`\nCurated single-instance libraries — npm-install closure of ${targets.length} target(s):\n`,
);
// Unattributed copies fall back to raw paths rather than rendering a "N copies" header with
// nothing under it.
for (const line of formatCuratedReport(found, duplicates, (dup) => {
const copies = attributedCopies.get(dup.name);
return copies
? formatCopyLines(copies)
: dup.copies.map((c) => ` v${c.version} ${c.realPath}`);
})) {
console.log(line);
}
if (failures.length > 0) {
keepScratch = !reportOnly;
const scratchHint = keepScratch ? scratch : undefined;
const remediationOptions = { exemptPackages: exemptPackages(byName), scratch: scratchHint };
console.log('\nHow to fix\n');
for (const line of formatRemediation(explained, remediationOptions)) console.log(line);
writeStepSummary(formatStepSummary(explained, { reportOnly, ...remediationOptions }));
for (const f of failures) {
annotate(
`${f.name}: ${f.copies.length} copies in the npm-install graph (${f.copies.map((c) => `v${c.version}`).join(', ')})`,
reportOnly ? 'warning' : 'error',
);
}
console.error(
`\n${reportOnly ? 'REPORT' : 'FAIL'}: ${failures.length} curated library duplicate(s) in the npm-install graph.`,
// stdout, not stderr: the two streams interleave in the Actions log, which is how the
// verdict ended up printed in the middle of the duplicate list.
console.log(
`\n${reportOnly ? 'REPORT' : 'FAIL'}: ${describeFailureCount(failures.length)}: ${failures.map((f) => f.name).join(', ')}${reportOnly ? ' (advisory — this job does not block yet)' : ''}`,
);
return reportOnly ? 0 : 1;
}
console.log('\nOK: no curated duplicates in the npm-install graph.');
return 0;
} finally {
if (keepScratch) console.log(`\nScratch install kept for inspection: ${scratch}`);
else rmSync(work, { recursive: true, force: true });
// The kept tree is already pointed at from the remediation block, which is the only path
// that keeps one.
if (!keepScratch) rmSync(work, { recursive: true, force: true });
}
}