diff --git a/.code-health-baseline.json b/.code-health-baseline.json index c84b70321ec..e28b7bb1ba5 100644 --- a/.code-health-baseline.json +++ b/.code-health-baseline.json @@ -1,7 +1,7 @@ { "version": 1, - "generated": "2026-06-04T06:55:45.363Z", - "totalViolations": 160, + "generated": "2026-07-22T13:28:07.821Z", + "totalViolations": 188, "violations": { "packages/@n8n/ai-workflow-builder.ee/package.json": [ { @@ -33,6 +33,18 @@ "line": 87, "message": "@types/turndown appears in 3 packages with 2 different versions — add to pnpm-workspace.yaml catalog", "hash": "407c8d1b3428" + }, + { + "rule": "single-instance-libs", + "line": 60, + "message": "\"@langchain/core\" is a runtime dependency of \"@n8n/ai-workflow-builder\"; it must be a peerDependency.", + "hash": "352bf126d45a" + }, + { + "rule": "single-instance-libs", + "line": 86, + "message": "\"zod\" is a runtime dependency of \"@n8n/ai-workflow-builder\"; it must be a peerDependency.", + "hash": "fd6171764eb4" } ], "packages/@n8n/json-schema-to-zod/package.json": [ @@ -91,6 +103,24 @@ "line": 298, "message": "mongodb appears in 2 packages with 2 different versions — add to pnpm-workspace.yaml catalog", "hash": "46cb48884e22" + }, + { + "rule": "single-instance-libs", + "line": 253, + "message": "\"@langchain/core\" is a runtime dependency of \"@n8n/n8n-nodes-langchain\"; it must be a peerDependency.", + "hash": "a3cb17459555" + }, + { + "rule": "single-instance-libs", + "line": 304, + "message": "\"form-data\" is a runtime dependency of \"@n8n/n8n-nodes-langchain\"; it must be a peerDependency.", + "hash": "d3d8fabe4a38" + }, + { + "rule": "single-instance-libs", + "line": 330, + "message": "\"zod\" is a runtime dependency of \"@n8n/n8n-nodes-langchain\"; it must be a peerDependency.", + "hash": "6dea0e57951a" } ], "packages/@n8n/tournament/package.json": [ @@ -267,6 +297,24 @@ "line": 901, "message": "eslint-plugin-n8n-nodes-base appears in 2 packages with 2 different versions — add to pnpm-workspace.yaml catalog", "hash": "ac254baa61f9" + }, + { + "rule": "single-instance-libs", + "line": 938, + "message": "\"@langchain/core\" is a runtime dependency of \"n8n-nodes-base\"; it must be a peerDependency.", + "hash": "9501f20519c4" + }, + { + "rule": "single-instance-libs", + "line": 961, + "message": "\"form-data\" is a runtime dependency of \"n8n-nodes-base\"; it must be a peerDependency.", + "hash": "3998c8aa25cb" + }, + { + "rule": "single-instance-libs", + "line": 1020, + "message": "\"zod\" is a runtime dependency of \"n8n-nodes-base\"; it must be a peerDependency.", + "hash": "de1e5b4624e0" } ], "packages/@n8n/node-cli/package.json": [ @@ -333,6 +381,18 @@ "line": 110, "message": "n8n-workflow appears in 2 packages with 2 different versions — add to pnpm-workspace.yaml catalog", "hash": "ea4fbfff30ba" + }, + { + "rule": "single-instance-libs", + "line": 120, + "message": "\"zod\" is a runtime dependency of \"@n8n/ai-utilities\"; it must be a peerDependency.", + "hash": "2ed5d6717e54" + }, + { + "rule": "single-instance-libs", + "line": 122, + "message": "\"@langchain/core\" is a runtime dependency of \"@n8n/ai-utilities\"; it must be a peerDependency.", + "hash": "5626f18eefaf" } ], "packages/@n8n/mcp-browser/package.json": [ @@ -371,6 +431,12 @@ "line": 27, "message": "@joplin/turndown-plugin-gfm appears in 2 packages with 2 different versions — add to pnpm-workspace.yaml catalog", "hash": "743e3a7dbb32" + }, + { + "rule": "single-instance-libs", + "line": 39, + "message": "\"zod\" is a runtime dependency of \"@n8n/mcp-browser\"; it must be a peerDependency.", + "hash": "5045d34d52ae" } ], "packages/@n8n/task-runner/package.json": [ @@ -411,6 +477,12 @@ "line": 55, "message": "@joplin/turndown-plugin-gfm appears in 2 packages with 2 different versions — add to pnpm-workspace.yaml catalog", "hash": "a3cf1504b5c2" + }, + { + "rule": "single-instance-libs", + "line": 86, + "message": "\"zod\" is a runtime dependency of \"@n8n/instance-ai\"; it must be a peerDependency.", + "hash": "4a6882cb8d62" } ], "packages/@n8n/benchmark/package.json": [ @@ -455,6 +527,12 @@ "line": 74, "message": "recast appears in 2 packages with 2 different versions — add to pnpm-workspace.yaml catalog", "hash": "b660317b5f6f" + }, + { + "rule": "single-instance-libs", + "line": 83, + "message": "\"form-data\" is a runtime dependency of \"n8n-workflow\"; it must be a peerDependency.", + "hash": "a5f5104e7293" } ], "packages/@n8n/eslint-plugin-community-nodes/package.json": [ @@ -1134,6 +1212,122 @@ "message": "pg@^8.17.0 should use \"catalog:\" (exists in pnpm-workspace.yaml)", "hash": "6cac348bdcc8" } + ], + "packages/core/package.json": [ + { + "rule": "single-instance-libs", + "line": 61, + "message": "\"@langchain/core\" is a runtime dependency of \"n8n-core\"; it must be a peerDependency.", + "hash": "602aad4587ee" + }, + { + "rule": "single-instance-libs", + "line": 81, + "message": "\"form-data\" is a runtime dependency of \"n8n-core\"; it must be a peerDependency.", + "hash": "643d5b8cd4be" + } + ], + "packages/@n8n/backend-network/package.json": [ + { + "rule": "single-instance-libs", + "line": 68, + "message": "\"form-data\" is a runtime dependency of \"@n8n/backend-network\"; it must be a peerDependency.", + "hash": "6b988cc15b6f" + }, + { + "rule": "single-instance-libs", + "line": 77, + "message": "\"zod\" is a runtime dependency of \"@n8n/backend-network\"; it must be a peerDependency.", + "hash": "3af671752aa6" + } + ], + "packages/@n8n/blob-storage/package.json": [ + { + "rule": "single-instance-libs", + "line": 52, + "message": "\"zod\" is a runtime dependency of \"@n8n/blob-storage\"; it must be a peerDependency.", + "hash": "2beb68b6b1eb" + } + ], + "packages/@n8n/config/package.json": [ + { + "rule": "single-instance-libs", + "line": 29, + "message": "\"zod\" is a runtime dependency of \"@n8n/config\"; it must be a peerDependency.", + "hash": "0acfb4c2c019" + } + ], + "packages/@n8n/db/package.json": [ + { + "rule": "single-instance-libs", + "line": 52, + "message": "\"zod\" is a runtime dependency of \"@n8n/db\"; it must be a peerDependency.", + "hash": "67751e0ed127" + } + ], + "packages/@n8n/extension-sdk/package.json": [ + { + "rule": "single-instance-libs", + "line": 66, + "message": "\"zod\" is a runtime dependency of \"@n8n/extension-sdk\"; it must be a peerDependency.", + "hash": "ab05f1864411" + } + ], + "packages/@n8n/expression-runtime/package.json": [ + { + "rule": "single-instance-libs", + "line": 48, + "message": "\"zod\" is a runtime dependency of \"@n8n/expression-runtime\"; it must be a peerDependency.", + "hash": "5669d2783982" + } + ], + "packages/@n8n/mcp-apps/package.json": [ + { + "rule": "single-instance-libs", + "line": 45, + "message": "\"zod\" is a runtime dependency of \"@n8n/mcp-apps\"; it must be a peerDependency.", + "hash": "5e1c42cefa8d" + } + ], + "packages/@n8n/permissions/package.json": [ + { + "rule": "single-instance-libs", + "line": 26, + "message": "\"zod\" is a runtime dependency of \"@n8n/permissions\"; it must be a peerDependency.", + "hash": "ead6e5815a0f" + } + ], + "packages/@n8n/syslog-client/package.json": [ + { + "rule": "single-instance-libs", + "line": 26, + "message": "\"zod\" is a runtime dependency of \"@n8n/syslog-client\"; it must be a peerDependency.", + "hash": "d1cff54dc25f" + } + ], + "packages/@n8n/workflow-sdk/package.json": [ + { + "rule": "single-instance-libs", + "line": 107, + "message": "\"zod\" is a runtime dependency of \"@n8n/workflow-sdk\"; it must be a peerDependency.", + "hash": "e8f5ce9766b4" + } + ], + "packages/@n8n/engine/package.json": [ + { + "rule": "single-instance-libs", + "line": 34, + "message": "\"zod\" is a runtime dependency of \"@n8n/engine\"; it must be a peerDependency.", + "hash": "a25ac9357fce" + } + ], + "packages/@n8n/telemetry/package.json": [ + { + "rule": "single-instance-libs", + "line": 26, + "message": "\"zod\" is a runtime dependency of \"@n8n/telemetry\"; it must be a peerDependency.", + "hash": "d61edcda4afe" + } ] } } diff --git a/.github/WORKFLOWS.md b/.github/WORKFLOWS.md index 667989ed334..534e116a00a 100644 --- a/.github/WORKFLOWS.md +++ b/.github/WORKFLOWS.md @@ -268,7 +268,8 @@ ci-pull-requests.yml ci-master.yml ├──────────────────────────▶ test-unit-reusable.yml - └──────────────────────────▶ test-linting-reusable.yml + ├──────────────────────────▶ test-linting-reusable.yml + └──────────────────────────▶ test-single-instance-npm.yml release-publish.yml ├──────────────────────────▶ docker-build-push.yml @@ -373,6 +374,7 @@ Push to master/1.x ├─ unit-test (matrix: Node 22.23.2, 24.18.1) │ └─ Coverage only on 24.18.1 ├─ lint +├─ verify-single-instance-npm (advisory; packages changed by this push) └─ notify-on-failure (Slack #alerts-build) ``` @@ -472,6 +474,7 @@ Workflows with `workflow_call` trigger: | `sec-poutine-reusable.yml` | `ref` | Poutine scanner | | `security-trivy-scan-callable.yml` | `image_ref` | Trivy scan | | `sbom-generation-callable.yml` | `n8n_version`, `release_tag_ref` | SBOM generation | +| `test-single-instance-npm.yml` | `scope`, `base-ref`, `base-branch`, `blocking`, `timeout-minutes` | Dependency duplication | --- diff --git a/.github/workflows/ci-master.yml b/.github/workflows/ci-master.yml index 75abe0e139d..50d9424f539 100644 --- a/.github/workflows/ci-master.yml +++ b/.github/workflows/ci-master.yml @@ -52,6 +52,16 @@ jobs: with: ref: ${{ github.sha }} + # Scoped to the packages this push touched, so it stays cheap enough to run on every commit. + verify-single-instance-npm: + name: Verify single-instance deps (npm-install) + uses: ./.github/workflows/test-single-instance-npm.yml + permissions: + contents: read + with: + scope: changed + base-ref: ${{ github.event.before }} + notify-on-failure: name: Notify Slack on failure runs-on: ubuntu-latest diff --git a/.github/workflows/release-create-pr.yml b/.github/workflows/release-create-pr.yml index 28460ae5209..539912dcebf 100644 --- a/.github/workflows/release-create-pr.yml +++ b/.github/workflows/release-create-pr.yml @@ -141,3 +141,15 @@ jobs: secrets: inherit with: pull-request-number: ${{ needs.create-release-pr.outputs.pull-request-number }} + + # Packs every publishable package, so it only runs at release prep, in parallel with the PR. + verify-single-instance-npm-full: + name: Verify single-instance deps (npm-install, full) + uses: ./.github/workflows/test-single-instance-npm.yml + permissions: + contents: read + with: + scope: all + # Profile the release being prepared, not this workflow's triggering revision. + base-branch: ${{ inputs.base-branch }} + timeout-minutes: 30 diff --git a/.github/workflows/test-single-instance-npm.yml b/.github/workflows/test-single-instance-npm.yml new file mode 100644 index 00000000000..1a0b66c3063 --- /dev/null +++ b/.github/workflows/test-single-instance-npm.yml @@ -0,0 +1,95 @@ +name: 'Test: Single-instance deps (npm-install)' + +# An npm-install graph exposes curated-lib duplicates that pnpm dev/deploy hides, because root +# `pnpm.overrides` don't travel in published tarballs. + +on: + workflow_call: + inputs: + scope: + description: '"changed" verifies only packages touched since base-ref; "all" packs every publishable package.' + required: true + type: string + base-ref: + description: 'Commit to diff against. Required when scope is "changed", rejected otherwise.' + required: false + type: string + default: '' + base-branch: + description: 'Branch to check out before verifying, so the run profiles that branch rather than the caller''s triggering revision. Only valid when scope is "all".' + required: false + type: string + default: '' + blocking: + description: 'Fail the calling workflow on a finding. Defaults to advisory while the curated-lib backlog is worked down.' + required: false + type: boolean + default: false + timeout-minutes: + description: 'Job timeout. The "all" scope packs every publishable package, so it needs longer than the scoped one.' + required: false + type: number + default: 20 + +jobs: + verify: + runs-on: ubuntu-latest + continue-on-error: ${{ !inputs.blocking }} + timeout-minutes: ${{ inputs.timeout-minutes }} + permissions: + contents: read + steps: + # Every rejected combination below otherwise degrades into a run that verifies nothing and + # still exits 0 — silently, since this job is advisory by default. + - name: Validate inputs + env: + SCOPE: ${{ inputs.scope }} + BASE_REF: ${{ inputs.base-ref }} + BASE_BRANCH: ${{ inputs.base-branch }} + run: | + case "$SCOPE" in + changed) + [ -n "$BASE_REF" ] || { echo "::error::scope=changed requires base-ref."; exit 1; } + # A base-branch checkout would move HEAD away from the pushed commit, so the diff + # would silently describe the wrong range. + [ -z "$BASE_BRANCH" ] || { echo "::error::scope=changed cannot be combined with base-branch."; exit 1; } + ;; + all) + [ -z "$BASE_REF" ] || { echo "::error::scope=all does not use base-ref."; exit 1; } + ;; + *) + echo "::error::Unknown scope \"$SCOPE\" (expected \"changed\" or \"all\")." + exit 1 + ;; + esac + + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + fetch-depth: 0 + persist-credentials: false + + - name: Switch to base branch + if: inputs.base-branch != '' + env: + BASE_BRANCH: ${{ inputs.base-branch }} + run: git checkout "$BASE_BRANCH" + + # Only the verifier's own closure needs compiling. The check reads package.json out of the + # npm-install graph, so the packed tarballs' contents don't matter, and the two packages with + # a prepack hook build themselves. + - name: Setup and Build + uses: ./.github/actions/setup-nodejs + with: + build-command: pnpm turbo run build --filter=@n8n/code-health... + + # `--dir` rather than `--filter`: a filter matching nothing exits 0, so a renamed or moved + # package would turn this into a green job that verified nothing. + - name: Verify npm-install closure (changed packages) + if: inputs.scope == 'changed' + env: + BASE_REF: ${{ inputs.base-ref }} + run: pnpm --dir packages/testing/code-health exec tsx src/cli.ts verify-npm-install --changed="$BASE_REF" + + - name: Verify npm-install closure (all packages) + if: inputs.scope == 'all' + run: pnpm --dir packages/testing/code-health exec tsx src/cli.ts verify-npm-install --all diff --git a/lefthook.yml b/lefthook.yml index a427ff331ab..0e9883a90dc 100644 --- a/lefthook.yml +++ b/lefthook.yml @@ -32,15 +32,16 @@ pre-commit: skip: - merge - rebase - workspace_private_deps_check: + # Supersedes the zod-only peer check: same guard, generalized to every curated library. + single_instance_libs_check: glob: '**/package.json' - run: node scripts/check-workspace-private-deps.mjs + run: pnpm --dir packages/testing/code-health exec tsx src/cli.ts --rule=single-instance-libs skip: - merge - rebase - zod_peer_deps_check: - glob: 'packages/{@n8n/api-types,workflow,core,@n8n/agents}/package.json' - run: node scripts/check-zod-peer-deps.mjs + workspace_private_deps_check: + glob: '**/package.json' + run: node scripts/check-workspace-private-deps.mjs skip: - merge - rebase diff --git a/package.json b/package.json index ac94c914363..0bbd088a6a4 100644 --- a/package.json +++ b/package.json @@ -51,7 +51,6 @@ "boundaries:baseline": "node scripts/check-boundaries.mjs --write", "sync:skill-links": "node scripts/sync-agent-skill-links.mjs", "check:skill-links": "node scripts/sync-agent-skill-links.mjs --check", - "check:zod-peer-deps": "node scripts/check-zod-peer-deps.mjs", "check:workspace-private-deps": "node scripts/check-workspace-private-deps.mjs", "optimize-svg": "find ./packages -name '*.svg' ! -name 'pipedrive.svg' -print0 | xargs -0 -P16 -L20 npx svgo", "setup-backend-module": "node scripts/ensure-zx.mjs && zx scripts/backend-module/setup.mjs", diff --git a/packages/@n8n/typeorm/package.json b/packages/@n8n/typeorm/package.json index 127145b9449..40b866bc150 100644 --- a/packages/@n8n/typeorm/package.json +++ b/packages/@n8n/typeorm/package.json @@ -115,7 +115,7 @@ "dotenv": "catalog:", "glob": "^10.0.0", "mkdirp": "^2.1.3", - "reflect-metadata": "^0.2.2", + "reflect-metadata": "catalog:", "sha.js": "^2.4.12", "tarn": "3.0.2", "tslib": "^2.5.0", diff --git a/packages/frontend/editor-ui/vite.config.mts b/packages/frontend/editor-ui/vite.config.mts index ae9663fe86b..208aa6e230b 100644 --- a/packages/frontend/editor-ui/vite.config.mts +++ b/packages/frontend/editor-ui/vite.config.mts @@ -23,6 +23,10 @@ const browsers = browserslist.loadConfig({ path: process.cwd() }); const packagesDir = resolve(__dirname, '..', '..'); +// zod is the only single-instance-sensitive library the frontend bundles; dedupe it so +// Vite resolves it to a single copy. The other curated libs are backend-only. +const singleInstanceDedupe = ['zod']; + const alias = [ { find: '@', replacement: resolve(__dirname, 'src') }, { find: 'stream', replacement: 'stream-browserify' }, @@ -234,7 +238,7 @@ export default mergeConfig( BASE_PATH: `'${publicPath}'`, }, plugins, - resolve: { alias }, + resolve: { alias, dedupe: singleInstanceDedupe }, base: publicPath, envPrefix: ['VUE', 'N8N_ENV_FEAT'], css: { diff --git a/packages/testing/code-health/package.json b/packages/testing/code-health/package.json index 9721000fe1d..623d64f485b 100644 --- a/packages/testing/code-health/package.json +++ b/packages/testing/code-health/package.json @@ -18,6 +18,7 @@ "build:unchecked": "tsc -p tsconfig.build.json --noCheck", "dev": "tsc --watch", "test": "vitest run", + "test:unit": "vitest run", "test:watch": "vitest", "test:coverage": "vitest run --coverage", "format": "biome format --write .", diff --git a/packages/testing/code-health/src/cli.ts b/packages/testing/code-health/src/cli.ts index ffb60071ec9..1adbc300236 100644 --- a/packages/testing/code-health/src/cli.ts +++ b/packages/testing/code-health/src/cli.ts @@ -13,6 +13,8 @@ import * as path from 'node:path'; import { parseArgs } from './cli/arg-parser.js'; import type { CodeHealthContext } from './context.js'; import { createDefaultRunner } from './index.js'; +import { runVerifyClosure } from './single-instance/verify-closure.js'; +import { runVerifyNpmInstall } from './single-instance/verify-npm-install.js'; const BASELINE_FILENAME = '.code-health-baseline.json'; @@ -28,6 +30,15 @@ async function main(): Promise { addedFiles: parseChangedFiles(process.env.CODE_HEALTH_ADDED_FILES), }; + if (options.command === 'verify-closure') { + const dir = options.args[0] ? path.resolve(process.cwd(), options.args[0]) : rootDir; + process.exit(runVerifyClosure(dir)); + } + + if (options.command === 'verify-npm-install') { + process.exit(await runVerifyNpmInstall(options.args, rootDir)); + } + const runner = createDefaultRunner(); if (options.command === 'rules') { @@ -49,7 +60,9 @@ async function main(): Promise { } if (options.command === 'baseline') { - const baseline = generateBaseline(report, rootDir); + // Pass the current file through so exceptions for rules this run didn't cover survive. + const previous = fs.existsSync(baselinePath) ? loadBaseline(baselinePath) : null; + const baseline = generateBaseline(report, rootDir, previous); saveBaseline(baseline, baselinePath); console.log( JSON.stringify( diff --git a/packages/testing/code-health/src/cli/arg-parser.test.ts b/packages/testing/code-health/src/cli/arg-parser.test.ts new file mode 100644 index 00000000000..0a2041bfb53 --- /dev/null +++ b/packages/testing/code-health/src/cli/arg-parser.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, it } from 'vitest'; + +import { parseArgs } from './arg-parser.js'; + +describe('parseArgs', () => { + it('defaults to analyze when given no positional', () => { + expect(parseArgs([]).command).toBe('analyze'); + expect(parseArgs(['--rule=single-instance-libs']).command).toBe('analyze'); + }); + + it('accepts each subcommand and passes the rest through', () => { + const options = parseArgs(['verify-closure', '/tmp/closure']); + + expect(options.command).toBe('verify-closure'); + expect(options.args).toEqual(['/tmp/closure']); + }); + + // A silent fallback to `analyze` would exit 0 having checked nothing the caller asked for, + // and every caller spells the subcommand out in a YAML or shell literal. + it.each(['verify-clsoure', 'verify_closure', 'Verify-Closure', 'check'])( + 'rejects the unknown command %j instead of falling back to analyze', + (command) => { + expect(() => parseArgs([command])).toThrow(/Unknown command/); + }, + ); + + it('names the accepted commands when rejecting', () => { + expect(() => parseArgs(['nope'])).toThrow(/verify-closure/); + }); +}); diff --git a/packages/testing/code-health/src/cli/arg-parser.ts b/packages/testing/code-health/src/cli/arg-parser.ts index 1b2ad9d0931..e88fd980082 100644 --- a/packages/testing/code-health/src/cli/arg-parser.ts +++ b/packages/testing/code-health/src/cli/arg-parser.ts @@ -1,26 +1,42 @@ +// The default command is `analyze`; these are the explicit subcommands accepted as argv[0]. +const SUBCOMMANDS = ['baseline', 'rules', 'verify-closure', 'verify-npm-install'] as const; +type Subcommand = (typeof SUBCOMMANDS)[number]; + export interface CliOptions { - command: 'analyze' | 'baseline' | 'rules'; + command: 'analyze' | Subcommand; rule?: string; file?: string; ignoreBaseline: boolean; + /** Remaining args after the command — interpreted by the `verify-*` subcommands. */ + args: string[]; +} + +function isSubcommand(value: string): value is Subcommand { + return (SUBCOMMANDS as readonly string[]).includes(value); } export function parseArgs(args: string[]): CliOptions { const options: CliOptions = { command: 'analyze', ignoreBaseline: false, + args: [], }; let i = 0; + // Every caller spells the subcommand out in a YAML or shell literal, so a typo must not fall + // back to `analyze` — that reports 0 new violations and exits 0, i.e. a pass that checked + // nothing the caller asked for. if (args.length > 0 && !args[0].startsWith('-')) { - const command = args[0]; - if (command === 'baseline' || command === 'rules') { - options.command = command; + if (!isSubcommand(args[0])) { + throw new Error(`Unknown command "${args[0]}". Expected one of: ${SUBCOMMANDS.join(', ')}.`); } + options.command = args[0]; i = 1; } + options.args = args.slice(i); + for (; i < args.length; i++) { const arg = args[i]; diff --git a/packages/testing/code-health/src/index.ts b/packages/testing/code-health/src/index.ts index 57135fc40a4..3b9f1c12ccb 100644 --- a/packages/testing/code-health/src/index.ts +++ b/packages/testing/code-health/src/index.ts @@ -5,6 +5,7 @@ import type { CodeHealthContext } from './context.js'; import { CatalogViolationsRule } from './rules/catalog-violations.rule.js'; import { EndpointScopeCoverageRule } from './rules/endpoint-scope-coverage.rule.js'; import { MigrationTimestampRule } from './rules/migration-timestamp.rule.js'; +import { SingleInstanceLibsRule } from './rules/single-instance-libs.rule.js'; import { StaleOverridesRule } from './rules/stale-overrides.rule.js'; import { SubpathPurityRule } from './rules/subpath-purity.rule.js'; import { WorkflowPrTargetSafetyRule } from './rules/workflow-pr-target-safety.rule.js'; @@ -13,6 +14,7 @@ export type { CodeHealthContext } from './context.js'; export { CatalogViolationsRule } from './rules/catalog-violations.rule.js'; export { EndpointScopeCoverageRule } from './rules/endpoint-scope-coverage.rule.js'; export { MigrationTimestampRule } from './rules/migration-timestamp.rule.js'; +export { SingleInstanceLibsRule } from './rules/single-instance-libs.rule.js'; export { StaleOverridesRule } from './rules/stale-overrides.rule.js'; export { SubpathPurityRule } from './rules/subpath-purity.rule.js'; export type { SubpathSpec } from './rules/subpath-purity.rule.js'; @@ -34,6 +36,11 @@ const defaultRuleSettings: RuleSettingsMap = { severity: 'error', options: {}, }, + 'single-instance-libs': { + enabled: true, + severity: 'error', + options: {}, + }, 'stale-overrides': { enabled: true, severity: 'warning', @@ -112,6 +119,7 @@ export function createDefaultRunner(settings?: RuleSettingsMap): RuleRunner): void { + const fullPath = path.join(dir, relDir, 'package.json'); + fs.mkdirSync(path.dirname(fullPath), { recursive: true }); + fs.writeFileSync(fullPath, JSON.stringify(pkg, null, 2)); +} + +describe('SingleInstanceLibsRule', () => { + let tmpDir: string; + let rule: SingleInstanceLibsRule; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'code-health-test-')); + rule = new SingleInstanceLibsRule(); + }); + + afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + const context = (): CodeHealthContext => ({ rootDir: tmpDir }); + + it('flags a curated lib declared as a runtime dependency', async () => { + writePackage(tmpDir, 'packages/@n8n/config', { + name: '@n8n/config', + dependencies: { zod: 'catalog:' }, + }); + const violations = await rule.analyze(context()); + expect(violations).toHaveLength(1); + expect(violations[0].message).toContain('zod'); + expect(violations[0].message).toContain('must be a peerDependency'); + }); + + it('flags a curated lib in optionalDependencies', async () => { + writePackage(tmpDir, 'packages/@n8n/config', { + name: '@n8n/config', + optionalDependencies: { 'form-data': 'catalog:' }, + }); + const violations = await rule.analyze(context()); + expect(violations.map((v) => v.message).join()).toContain('form-data'); + }); + + // `@n8n/config` is a non-host package not in REQUIRED_CURATED_PEERS, so these exercise the + // catalog-shape check in isolation from the dropped-peer guard below. + it('does not flag a curated lib declared as a catalog: peerDependency', async () => { + writePackage(tmpDir, 'packages/@n8n/config', { + name: '@n8n/config', + peerDependencies: { zod: 'catalog:' }, + devDependencies: { zod: 'catalog:' }, + }); + expect(await rule.analyze(context())).toHaveLength(0); + }); + + it('flags a curated peerDependency that is not catalog:', async () => { + writePackage(tmpDir, 'packages/@n8n/config', { + name: '@n8n/config', + peerDependencies: { zod: '^3.25.0' }, + }); + const violations = await rule.analyze(context()); + expect(violations).toHaveLength(1); + expect(violations[0].message).toContain('must use "catalog:"'); + }); + + // The catalog-shape check keys off the whole curated list, not just zod — a scoped curated + // lib and even the pin-only reflect-metadata must be pinned when declared as a peer. + it.each(['@langchain/core', 'reflect-metadata'])( + 'flags a non-zod curated peerDependency (%s) that is not catalog:', + async (lib) => { + writePackage(tmpDir, 'packages/@n8n/config', { + name: '@n8n/config', + peerDependencies: { [lib]: '^1.0.0' }, + }); + const violations = await rule.analyze(context()); + expect(violations).toHaveLength(1); + expect(violations[0].message).toContain(lib); + expect(violations[0].message).toContain('must use "catalog:"'); + }, + ); + + // Dropped-peer guard: `@n8n/api-types` is in REQUIRED_CURATED_PEERS.zod. + it('flags a required package that dropped its curated peerDependency', async () => { + writePackage(tmpDir, 'packages/@n8n/api-types', { + name: '@n8n/api-types', + dependencies: { lodash: '^4.0.0' }, + }); + const violations = await rule.analyze(context()); + expect(violations).toHaveLength(1); + expect(violations[0].message).toContain('zod'); + expect(violations[0].message).toContain('required single-instance peer'); + }); + + it('does not flag a required package that still declares its curated peer', async () => { + writePackage(tmpDir, 'packages/@n8n/api-types', { + name: '@n8n/api-types', + peerDependencies: { zod: 'catalog:' }, + }); + expect(await rule.analyze(context())).toHaveLength(0); + }); + + it('exempts host packages', async () => { + writePackage(tmpDir, 'packages/cli', { name: 'n8n', dependencies: { zod: 'catalog:' } }); + expect(await rule.analyze(context())).toHaveLength(0); + }); + + it('exempts frontend packages', async () => { + writePackage(tmpDir, 'packages/frontend/editor-ui', { + name: 'n8n-editor-ui', + dependencies: { zod: 'catalog:' }, + }); + expect(await rule.analyze(context())).toHaveLength(0); + }); + + it('exempts pin-only libs (reflect-metadata)', async () => { + writePackage(tmpDir, 'packages/@n8n/config', { + name: '@n8n/config', + dependencies: { 'reflect-metadata': 'catalog:' }, + }); + expect(await rule.analyze(context())).toHaveLength(0); + }); + + it('ignores curated libs in devDependencies (catalog-violations covers those)', async () => { + writePackage(tmpDir, 'packages/@n8n/config', { + name: '@n8n/config', + devDependencies: { zod: 'catalog:' }, + }); + expect(await rule.analyze(context())).toHaveLength(0); + }); +}); diff --git a/packages/testing/code-health/src/rules/single-instance-libs.rule.ts b/packages/testing/code-health/src/rules/single-instance-libs.rule.ts new file mode 100644 index 00000000000..4edcd7eabc7 --- /dev/null +++ b/packages/testing/code-health/src/rules/single-instance-libs.rule.ts @@ -0,0 +1,116 @@ +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 { REQUIRED_CURATED_PEERS } from '../single-instance/required-peers.js'; +import { + findPackageJsonFiles, + parsePackageJson, + relativeDir, +} from '../utils/package-json-scanner.js'; + +// Sections that get installed at runtime — a curated lib here can nest a second physical copy. +const RUNTIME_SECTIONS = new Set(['dependencies', 'optionalDependencies']); + +/** + * A single-instance-sensitive library must be a `peerDependency` in non-host, non-frontend + * packages — a plain `dependency` looks fine under the pnpm catalog but lets `npm install` nest a + * second physical copy, breaking cross-package composition / `instanceof` at runtime. + * + * `catalog-violations` forces curated libs to `catalog:` in every section but `peerDependencies`; + * this rule covers what it skips — the peer shape, plus pinning curated peers via `catalog:`. + * `reflect-metadata` is pin-only (excluded from the peer rule). Report-first via the baseline. + */ +export class SingleInstanceLibsRule extends BaseRule { + readonly id = 'single-instance-libs'; + readonly name = 'Single-instance Libraries'; + readonly description = + 'Single-instance-sensitive libraries must be peerDependencies (via catalog:), not plain/optional dependencies, in non-host packages'; + readonly severity = 'error' as const; + + async analyze(context: CodeHealthContext): Promise { + const { rootDir } = context; + const files = await findPackageJsonFiles(rootDir); + const violations: Violation[] = []; + // packageName -> { file, curated libs it declares as peers } — feeds the dropped-peer pass. + const declaredPeers = new Map }>(); + + for (const file of files) { + const pkg = parsePackageJson(file); + declaredPeers.set(pkg.packageName, { + file, + libs: new Set( + pkg.deps + .filter((d) => d.section === 'peerDependencies' && CURATED_LIBS.includes(d.name)) + .map((d) => d.name), + ), + }); + + if (this.isExempt(pkg.packageName, relativeDir(rootDir, file))) continue; + + for (const dep of pkg.deps) { + if (PEER_LIBS.includes(dep.name) && RUNTIME_SECTIONS.has(dep.section)) { + violations.push( + this.createViolation( + file, + dep.line, + 5, + `"${dep.name}" is a runtime dependency of "${pkg.packageName}"; it must be a peerDependency.`, + 'Move it to peerDependencies with "catalog:", and keep it in devDependencies with "catalog:" for local builds.', + ), + ); + } + + if ( + CURATED_LIBS.includes(dep.name) && + dep.section === 'peerDependencies' && + !dep.usesCatalog + ) { + violations.push( + this.createViolation( + file, + dep.line, + 5, + `"${dep.name}" peerDependency in "${pkg.packageName}" must use "catalog:".`, + 'Change the peerDependencies entry to "catalog:" so the published range is a single pinned version.', + ), + ); + } + } + } + + // Dropped-peer regression guard: a package that a prior snapshot recorded as declaring a + // curated peer must keep declaring it. Only checked for packages present in this scan — an + // absent package imports nothing here, so it carries no single-instance risk. + for (const [lib, requiredPackages] of Object.entries(REQUIRED_CURATED_PEERS)) { + for (const packageName of requiredPackages) { + const declared = declaredPeers.get(packageName); + if (!declared || declared.libs.has(lib)) continue; + violations.push( + this.createViolation( + declared.file, + 1, + 1, + `"${packageName}" no longer declares "${lib}" as a peerDependency; it is a required single-instance peer, and dropping it lets consumers resolve a second physical copy at runtime.`, + `Restore the "${lib}" peerDependency (with "catalog:"), or — if the removal is intentional — update REQUIRED_CURATED_PEERS in packages/testing/code-health/src/single-instance/required-peers.ts.`, + ), + ); + } + } + + return violations; + } + + private isExempt(packageName: string, relDir: string): boolean { + return ( + HOST_PACKAGES.includes(packageName) || + FRONTEND_PATH_PREFIXES.some((prefix) => relDir.startsWith(prefix)) + ); + } +} diff --git a/packages/testing/code-health/src/single-instance/collect-copies.test.ts b/packages/testing/code-health/src/single-instance/collect-copies.test.ts new file mode 100644 index 00000000000..5f95e6adc7d --- /dev/null +++ b/packages/testing/code-health/src/single-instance/collect-copies.test.ts @@ -0,0 +1,156 @@ +import { mkdirSync, mkdtempSync, rmSync, symlinkSync, 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, distinctCopies } from './collect-copies.js'; + +// Build a planted install tree on disk so the walk (collectCopies) is exercised, not just the +// pure analyze() core. `node_modules` dirs are gitignored, so we construct the tree in a temp dir. +let ROOT: string; + +function pkg(relDir: string, name: string, version: string): void { + const dir = join(ROOT, relDir); + mkdirSync(dir, { recursive: true }); + writeFileSync(join(dir, 'package.json'), JSON.stringify({ name, version })); +} + +let planted: ReturnType; +let clean: ReturnType; + +beforeAll(() => { + ROOT = mkdtempSync(join(tmpdir(), 'collect-copies-fixture-')); + + // two copies of zod (curated, NOT allowlisted) -> FAIL + pkg('planted/node_modules/zod', 'zod', '1.0.0'); + pkg('planted/node_modules/a', 'a', '1.0.0'); + pkg('planted/node_modules/a/node_modules/zod', 'zod', '2.0.0'); + // two copies of @langchain/core (curated) -> used for the allowlist case + pkg('planted/node_modules/@langchain/core', '@langchain/core', '1.0.0'); + pkg('planted/node_modules/b', 'b', '1.0.0'); + pkg('planted/node_modules/b/node_modules/@langchain/core', '@langchain/core', '2.0.0'); + // two copies of lodash (non-curated) -> report only, never fails + pkg('planted/node_modules/lodash', 'lodash', '1.0.0'); + pkg('planted/node_modules/c', 'c', '1.0.0'); + pkg('planted/node_modules/c/node_modules/lodash', 'lodash', '2.0.0'); + // single copy of each curated lib -> passes + pkg('clean/node_modules/zod', 'zod', '1.0.0'); + pkg('clean/node_modules/@langchain/core', '@langchain/core', '1.0.0'); + + planted = collectCopies(join(ROOT, 'planted')); + clean = collectCopies(join(ROOT, 'clean')); +}); + +afterAll(() => rmSync(ROOT, { recursive: true, force: true })); + +describe('collectCopies + analyze', () => { + it('fails on planted duplicates of non-allowlisted curated libs', () => { + const { failures } = analyze(planted); + expect(failures.map((f) => f.name).sort()).toEqual(['@langchain/core', 'zod']); + }); + + it('reports but does not fail on an allowlisted curated duplicate', () => { + const { duplicates, failures } = analyze(planted, { + allowlist: { '@langchain/core': 'test-only allowlist entry' }, + }); + expect(duplicates.find((d) => d.name === '@langchain/core')?.allowed).toBe(true); + expect(failures.some((f) => f.name === '@langchain/core')).toBe(false); + expect(failures.some((f) => f.name === 'zod')).toBe(true); + }); + + it('reports non-curated duplicates without failing on them', () => { + const { duplicates, failures } = analyze(planted); + const lodash = duplicates.find((d) => d.name === 'lodash'); + expect(lodash && !lodash.isCurated).toBe(true); + expect(failures.some((f) => f.name === 'lodash')).toBe(false); + }); + + it('passes a clean tree with a single copy of each curated lib', () => { + const { duplicates, failures } = analyze(clean); + expect(duplicates).toHaveLength(0); + expect(failures).toHaveLength(0); + }); +}); + +// `"zod-v3": "npm:zod@^3"` installs a real second copy of zod under a directory named `zod-v3`. +// Identifying copies by directory would leave it out of duplicate detection. +describe('npm rename aliases', () => { + it('counts an aliased copy under the name in its manifest', () => { + pkg('aliased/node_modules/zod', 'zod', '4.0.0'); + pkg('aliased/node_modules/zod-v3', 'zod', '3.0.0'); + + const found = collectCopies(join(ROOT, 'aliased')); + + expect(found.get('zod-v3')).toBeUndefined(); + expect( + distinctCopies(found.get('zod') ?? []) + .map((c) => c.version) + .sort(), + ).toEqual(['3.0.0', '4.0.0']); + expect(analyze(found).failures.map((f) => f.name)).toEqual(['zod']); + }); + + it('ignores a directory whose manifest has no name', () => { + mkdirSync(join(ROOT, 'nameless/node_modules/mystery'), { recursive: true }); + writeFileSync(join(ROOT, 'nameless/node_modules/mystery/package.json'), '{"version":"1.0.0"}'); + + expect(collectCopies(join(ROOT, 'nameless')).size).toBe(0); + }); +}); + +describe('unusable closure', () => { + // Reporting "no duplicates" for a tree that was never read is the worst outcome for this check, + // so callers get a throw (read as "did not run") rather than a clean verdict. + it('throws instead of reporting a clean result when the root has no node_modules', () => { + mkdirSync(join(ROOT, 'empty'), { recursive: true }); + + expect(() => collectCopies(join(ROOT, 'empty'))).toThrow(/No node_modules/); + expect(() => collectCopies(join(ROOT, 'does-not-exist'))).toThrow(/nothing was verified/); + }); + + it('throws when node_modules exists but is not a directory', () => { + mkdirSync(join(ROOT, 'nm-is-a-file'), { recursive: true }); + writeFileSync(join(ROOT, 'nm-is-a-file/node_modules'), 'not a directory'); + + expect(() => collectCopies(join(ROOT, 'nm-is-a-file'))).toThrow(/No node_modules directory/); + }); +}); + +// Exercises the pnpm-shaped layout the tool actually runs against: the physical copy lives under +// the `.pnpm` virtual store and is symlinked to `node_modules/`. This is what the `.pnpm` walk +// (walkPnpmStore) and the realpath dedup exist for — a single physical copy reached via a symlink +// alias must count once, not as a second copy. +describe('collectCopies on a pnpm-style store', () => { + let root: string; + + function storePkg(nm: string, key: string, name: string, version: string): string { + const dir = join(nm, '.pnpm', key, 'node_modules', name); + mkdirSync(dir, { recursive: true }); + writeFileSync(join(dir, 'package.json'), JSON.stringify({ name, version })); + return dir; + } + + afterAll(() => rmSync(root, { recursive: true, force: true })); + + beforeAll(() => { + root = mkdtempSync(join(tmpdir(), 'collect-copies-pnpm-')); + const nm = join(root, 'node_modules'); + // one physical zod in the store, symlinked to the top level (the normal single-copy case) + const zodReal = storePkg(nm, 'zod@1.0.0', 'zod', '1.0.0'); + symlinkSync(zodReal, join(nm, 'zod')); + // two physical copies of form-data in the store (distinct realpaths → a real duplicate) + storePkg(nm, 'form-data@1.0.0', 'form-data', '1.0.0'); + const fdReal = storePkg(nm, 'form-data@2.0.0', 'form-data', '2.0.0'); + symlinkSync(fdReal, join(nm, 'form-data')); + }); + + it('counts a store copy reached via a top-level symlink alias only once', () => { + const found = collectCopies(root); + expect(distinctCopies(found.get('zod') ?? [])).toHaveLength(1); + expect(analyze(found).failures.some((f) => f.name === 'zod')).toBe(false); + }); + + it('fails on two distinct physical copies in the store despite a symlink alias', () => { + expect(analyze(collectCopies(root)).failures.map((f) => f.name)).toEqual(['form-data']); + }); +}); diff --git a/packages/testing/code-health/src/single-instance/collect-copies.ts b/packages/testing/code-health/src/single-instance/collect-copies.ts new file mode 100644 index 00000000000..7751f8f5891 --- /dev/null +++ b/packages/testing/code-health/src/single-instance/collect-copies.ts @@ -0,0 +1,163 @@ +import { readdirSync, readFileSync, realpathSync, statSync } from 'node:fs'; +import { join } from 'node:path'; + +import { CURATED_LIBS } from './libs.js'; + +export interface Copy { + realPath: string; + version: string; +} + +export interface DuplicateGroup { + name: string; + isCurated: boolean; + allowed: boolean; + copies: Copy[]; +} + +/** + * Deliberately-tolerated duplicates (migration window). Each entry MUST document why it is + * tolerated and what removes it; remove an entry once remediated so a regression re-fails. + * Empty means every curated library must resolve to a single physical copy. + * + * Kept empty on purpose while the npm-install job is advisory. That job currently reports splits + * in `zod`, `@langchain/core` and `form-data`, all caused by third-party version pins we cannot + * override from a published tarball. Silencing them here is not the fix: entries are keyed by + * library name alone, so allowlisting `zod` would also stop the check ever failing on a zod split + * of our own making — the regression it exists to catch. Teaching the allowlist about the copies + * an entry covers is the prerequisite for both allowlisting these and making the job blocking. + */ +export const EXPECTED_DUPLICATES: Record = {}; + +/** + * Walk `/node_modules` (incl. nested installs and the pnpm `.pnpm` virtual store) and + * return a map of packageName -> every physical copy found. A distinct realpath is a distinct + * Node runtime module identity — the thing that breaks `instanceof`/singletons — so realpath, + * not version or inode, is the ground truth (pnpm hardlinks from the store, so distinct copies + * can share inodes yet stay distinct identities). + * + * Run against the PRUNED production closure (`compiled/`) or an `npm install` scratch tree — NOT + * the dev `.pnpm` store, which over-reports latent peer-context entries that are never co-loaded. + */ +export function collectCopies(root: string): Map { + const found = new Map(); + const walkedRealDirs = new Set(); // guard against symlink cycles / re-walks + const recordedReals = new Set(); // a physical copy is read+parsed once, not per alias + + const readEntries = (dir: string) => { + try { + return readdirSync(dir, { withFileTypes: true }); + } catch { + return []; + } + }; + + const record = (dir: string) => { + let real: string; + try { + real = realpathSync(dir); + } catch { + return; // not a real dir + } + if (recordedReals.has(real)) return; // same physical copy reached via another alias + let pj: { name?: string; version?: string }; + try { + pj = JSON.parse(readFileSync(join(real, 'package.json'), 'utf8')) as { + name?: string; + version?: string; + }; + } catch { + return; // not a real package dir (e.g. a decoy source folder) + } + // Key on the manifest name, not the directory: an npm rename (`"zod-v3": "npm:zod@^3"`) + // installs a real second copy of zod under `node_modules/zod-v3`, and keying on the + // directory would leave it out of duplicate detection entirely. + if (!pj.name) return; // no name means it is not a package we can identify + recordedReals.add(real); + const copies = found.get(pj.name) ?? []; + copies.push({ realPath: real, version: pj.version ?? '' }); + found.set(pj.name, copies); + }; + + // pnpm virtual store: each `@` entry holds the real package under its own node_modules. + const walkPnpmStore = (storeDir: string) => { + for (const entry of readEntries(storeDir)) walk(join(storeDir, entry.name, 'node_modules')); + }; + + const walk = (nmDir: string) => { + for (const e of readEntries(nmDir)) { + const name = e.name; + if (name === '.bin') continue; + const full = join(nmDir, name); + if (name === '.pnpm') { + walkPnpmStore(full); + } else if (name.startsWith('.')) { + continue; + } else if (name.startsWith('@')) { + for (const s of readEntries(full)) recordAndRecurse(join(full, s.name)); + } else { + recordAndRecurse(full); + } + } + }; + + const recordAndRecurse = (pkgDir: string) => { + record(pkgDir); + const nested = join(pkgDir, 'node_modules'); + try { + const real = realpathSync(nested); + if (statSync(real).isDirectory() && !walkedRealDirs.has(real)) { + walkedRealDirs.add(real); + walk(nested); + } + } catch { + /* no nested node_modules */ + } + }; + + // Absent nested `node_modules` are normal, so the walk tolerates them — but an absent root means + // there is no closure to inspect, and staying quiet about it reports "no duplicates found" for a + // closure that was never read. Callers treat a throw as "the check did not run". + // Anything that is not a directory fails the same way: `readdirSync` raises, the walk tolerates + // it, and the run reports a clean closure. `throwIfNoEntry: false` folds missing and non-directory + // into one check. + const rootNodeModules = join(root, 'node_modules'); + if (!statSync(rootNodeModules, { throwIfNoEntry: false })?.isDirectory()) { + throw new Error( + `No node_modules directory at ${rootNodeModules} — expected an installed closure, so nothing was verified.`, + ); + } + + walk(rootNodeModules); + return found; +} + +/** Reduce collected copies to distinct physical copies (dedup by realpath). */ +export function distinctCopies(copies: Copy[]): Copy[] { + const byReal = new Map(); + for (const c of copies) if (!byReal.has(c.realPath)) byReal.set(c.realPath, c); + return [...byReal.values()]; +} + +/** + * Given collected copies, return every package with >1 physical copy (`duplicates`, a discovery + * aid) and the curated, non-allowlisted subset that must hard-fail (`failures`). + */ +export function analyze( + found: Map, + { allowlist = EXPECTED_DUPLICATES }: { allowlist?: Record } = {}, +): { duplicates: DuplicateGroup[]; failures: DuplicateGroup[] } { + const duplicates: DuplicateGroup[] = []; + for (const [name, copies] of found) { + const distinct = distinctCopies(copies); + if (distinct.length <= 1) continue; + duplicates.push({ + name, + isCurated: CURATED_LIBS.includes(name), + allowed: Object.hasOwn(allowlist, name), + copies: distinct, + }); + } + const failures = duplicates.filter((d) => d.isCurated && !d.allowed); + return { duplicates, failures }; +} diff --git a/packages/testing/code-health/src/single-instance/libs.ts b/packages/testing/code-health/src/single-instance/libs.ts new file mode 100644 index 00000000000..754d531dd02 --- /dev/null +++ b/packages/testing/code-health/src/single-instance/libs.ts @@ -0,0 +1,33 @@ +/** + * Single source of truth for the "single-instance-sensitive" libraries whose runtime + * identity must not be duplicated across packages. A second physical copy (a distinct + * Node module identity) silently breaks `instanceof`, module singletons, and cross-package + * schema composition — and only surfaces in production / `npm install` graphs, not local + * pnpm dev. Consumed by the `single-instance-libs` rule and the `verify-closure` / + * `verify-npm-install` CLI subcommands. + */ + +/** Libraries a single process must resolve to exactly one physical copy of. */ +export const CURATED_LIBS = ['zod', 'form-data', '@langchain/core', 'reflect-metadata']; + +/** + * Curated libs enforced pin-only (must use `catalog:`) but NOT subject to the + * peerDependency rule this iteration. + */ +const PIN_ONLY_LIBS = ['reflect-metadata']; + +/** + * Host / standalone packages that provide their own runtime instance — the end-user CLI + * (`n8n`), the task runner, and standalone tools like `@n8n/computer-use`. They keep curated + * libs as real `dependencies` and are exempt from the peerDependency rule. + */ +export const HOST_PACKAGES = ['n8n', '@n8n/task-runner', '@n8n/computer-use']; + +/** + * Frontend packages that bundle their dependencies (Vite), so runtime-identity duplication + * does not apply. Repo-relative path prefixes, matched against package locations. + */ +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)); diff --git a/packages/testing/code-health/src/single-instance/required-peers.ts b/packages/testing/code-health/src/single-instance/required-peers.ts new file mode 100644 index 00000000000..2ec72e1378e --- /dev/null +++ b/packages/testing/code-health/src/single-instance/required-peers.ts @@ -0,0 +1,16 @@ +/** + * Snapshot of packages that declare a curated single-instance lib as a `peerDependency`. + * + * The `single-instance-libs` rule is otherwise presence-based (it flags a curated lib that appears + * in the wrong section) and so cannot see a curated peer that is *removed* from every section — yet + * a package that still imports the lib at runtime but no longer declares the peer lets consumers + * resolve a second physical copy, the exact single-instance regression the peer model prevents. + * This snapshot lets the rule fail when a listed package drops a required curated peer. + * + * Keyed by curated lib -> package names that must keep declaring it as a peerDependency. Update an + * entry only when a package intentionally adds or removes a curated peer (the rule's error names the + * file to edit). + */ +export const REQUIRED_CURATED_PEERS: Record = { + zod: ['@n8n/agents', '@n8n/api-types', '@n8n/json-schema-to-zod', 'n8n-core', 'n8n-workflow'], +}; diff --git a/packages/testing/code-health/src/single-instance/verify-closure.test.ts b/packages/testing/code-health/src/single-instance/verify-closure.test.ts new file mode 100644 index 00000000000..805d9b7577e --- /dev/null +++ b/packages/testing/code-health/src/single-instance/verify-closure.test.ts @@ -0,0 +1,51 @@ +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; + +import { EXIT_DUPLICATES_FOUND, runVerifyClosure } from './verify-closure.js'; + +let ROOT: string; + +function pkg(relDir: string, name: string, version: string): void { + const dir = join(ROOT, relDir); + mkdirSync(dir, { recursive: true }); + writeFileSync(join(dir, 'package.json'), JSON.stringify({ name, version })); +} + +beforeAll(() => { + ROOT = mkdtempSync(join(tmpdir(), 'verify-closure-fixture-')); + + pkg('clean/node_modules/zod', 'zod', '4.0.0'); + + // zod is curated and not allowlisted, so a second physical copy is a failure. + pkg('dirty/node_modules/zod', 'zod', '4.0.0'); + pkg('dirty/node_modules/a', 'a', '1.0.0'); + pkg('dirty/node_modules/a/node_modules/zod', 'zod', '3.0.0'); + + vi.spyOn(console, 'log').mockImplementation(() => {}); + vi.spyOn(console, 'error').mockImplementation(() => {}); +}); + +afterAll(() => { + vi.restoreAllMocks(); + rmSync(ROOT, { recursive: true, force: true }); +}); + +describe('runVerifyClosure', () => { + it('exits 0 when every curated library resolves to one copy', () => { + expect(runVerifyClosure(join(ROOT, 'clean'))).toBe(0); + }); + + it('exits EXIT_DUPLICATES_FOUND when a curated library is duplicated', () => { + expect(runVerifyClosure(join(ROOT, 'dirty'))).toBe(EXIT_DUPLICATES_FOUND); + }); + + // build-n8n.mjs reads the exit code to tell "found duplicates" apart from "never ran". A + // missing package or an unresolvable import also exits 1, so the finding must not use 1. + it('does not signal a finding with an exit code a failed toolchain also produces', () => { + expect(EXIT_DUPLICATES_FOUND).not.toBe(0); + expect(EXIT_DUPLICATES_FOUND).not.toBe(1); + expect(EXIT_DUPLICATES_FOUND).not.toBe(2); + }); +}); diff --git a/packages/testing/code-health/src/single-instance/verify-closure.ts b/packages/testing/code-health/src/single-instance/verify-closure.ts new file mode 100644 index 00000000000..278c30efea5 --- /dev/null +++ b/packages/testing/code-health/src/single-instance/verify-closure.ts @@ -0,0 +1,61 @@ +import { analyze, collectCopies, distinctCopies, EXPECTED_DUPLICATES } from './collect-copies.js'; +import { CURATED_LIBS } from './libs.js'; + +/** + * Exit code for "the closure was checked and curated duplicates were found". + * + * Deliberately not 1: a missing package, an unresolvable import or a crashed toolchain also exit + * 1, and a caller that reads 1 as a finding would report "duplication found, build continues" for + * a run that never checked anything. 0 and 3 are the only codes this function produces, so any + * other code means the check did not complete. + */ +export const EXIT_DUPLICATES_FOUND = 3; + +/** + * Verify a built closure at `dir`: print the curated verdict + report-only duplicates and return + * an exit code (`EXIT_DUPLICATES_FOUND` if a curated library resolves to multiple un-allowlisted + * physical copies). + */ +export function runVerifyClosure(dir: string): number { + const found = collectCopies(dir); + 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]}`); + } + + const otherDups = duplicates.filter((d) => !d.isCurated); + if (otherDups.length > 0) { + console.log(`\nOther duplicated packages (report-only, NOT enforced — ${otherDups.length}):`); + for (const d of otherDups) { + console.log( + ` ${d.name}: ${d.copies.length} copies (${d.copies.map((c) => `v${c.version}`).join(', ')})`, + ); + } + } + + 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(', ')}`, + ); + return EXIT_DUPLICATES_FOUND; + } + console.log('OK: no un-allowlisted curated duplicates.'); + return 0; +} diff --git a/packages/testing/code-health/src/single-instance/verify-npm-install.test.ts b/packages/testing/code-health/src/single-instance/verify-npm-install.test.ts new file mode 100644 index 00000000000..0e4a51852fb --- /dev/null +++ b/packages/testing/code-health/src/single-instance/verify-npm-install.test.ts @@ -0,0 +1,96 @@ +import { describe, expect, it } from 'vitest'; + +import { closureOf, filesTriggerFullRun, matchChangedFiles } from './verify-npm-install.js'; + +/** Build a WorkspacePkg fixture from `[depName, section]` pairs (only names + sections matter). */ +function ws(name: string, deps: Array<[string, string]>) { + return { + dir: `/x/${name}`, + relDir: `packages/${name}`, + info: { + filePath: `/x/${name}/package.json`, + packageName: name, + private: false, + deps: deps.map(([depName, section]) => ({ + name: depName, + version: '', + line: 1, + usesCatalog: false, + section, + })), + }, + }; +} + +const byName = new Map([ + [ + 'a', + ws('a', [ + ['b', 'dependencies'], + ['ext', 'dependencies'], + ]), + ], + ['b', ws('b', [['c', 'peerDependencies']])], + ['c', ws('c', [['d', 'optionalDependencies']])], + ['d', ws('d', [])], + ['solo', ws('solo', [['ext', 'dependencies']])], + ['dev-only', ws('dev-only', [['a', 'devDependencies']])], +]) as unknown as Parameters[1]; + +describe('closureOf', () => { + it('follows dependencies, peerDependencies and optionalDependencies transitively', () => { + expect(closureOf(['a'], byName).sort()).toEqual(['a', 'b', 'c', 'd']); + }); + + it('ignores non-workspace (external) deps', () => { + expect(closureOf(['solo'], byName).sort()).toEqual(['solo']); + }); + + it('does not follow devDependencies (they are not published)', () => { + expect(closureOf(['dev-only'], byName).sort()).toEqual(['dev-only']); + }); + + it('terminates on a dependency cycle', () => { + const cyclic = new Map([ + ['a', ws('a', [['b', 'dependencies']])], + ['b', ws('b', [['a', 'dependencies']])], + ]) as unknown as Parameters[1]; + expect(closureOf(['a'], cyclic).sort()).toEqual(['a', 'b']); + }); +}); + +describe('matchChangedFiles', () => { + const dirs: Array<[string, string]> = [ + ['pkg', 'packages/@n8n/pkg/'], + ['pkg-sub', 'packages/@n8n/pkg/sub/'], + ]; + + it('maps a file to its owning package', () => { + expect(matchChangedFiles(['packages/@n8n/pkg/src/x.ts'], dirs)).toEqual(['pkg']); + }); + + it('prefers the longest matching prefix when packages nest', () => { + expect(matchChangedFiles(['packages/@n8n/pkg/sub/x.ts'], dirs)).toEqual(['pkg-sub']); + }); + + it('returns nothing for files outside any package', () => { + expect(matchChangedFiles(['README.md'], dirs)).toEqual([]); + }); +}); + +describe('filesTriggerFullRun', () => { + it('escalates on the catalog / root manifest', () => { + expect(filesTriggerFullRun(['pnpm-workspace.yaml'])).toBe(true); + expect(filesTriggerFullRun(['package.json'])).toBe(true); + }); + + it('escalates on a change to the single-instance tool itself', () => { + expect(filesTriggerFullRun(['packages/testing/code-health/src/single-instance/libs.ts'])).toBe( + true, + ); + }); + + it('does not escalate on an ordinary package source change', () => { + expect(filesTriggerFullRun(['packages/core/src/index.ts'])).toBe(false); + }); +}); diff --git a/packages/testing/code-health/src/single-instance/verify-npm-install.ts b/packages/testing/code-health/src/single-instance/verify-npm-install.ts new file mode 100644 index 00000000000..d978f61e046 --- /dev/null +++ b/packages/testing/code-health/src/single-instance/verify-npm-install.ts @@ -0,0 +1,226 @@ +import { execFileSync } from 'node:child_process'; +import { mkdirSync, mkdtempSync, readdirSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { dirname, join } from 'node:path'; + +import { analyze, collectCopies } from './collect-copies.js'; +import type { PackageJsonInfo } from '../utils/package-json-scanner.js'; +import { + findPackageJsonFiles, + parsePackageJson, + relativeDir, +} from '../utils/package-json-scanner.js'; + +// Repo-relative paths whose change can shift dependency resolution repo-wide (the catalog, the +// 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']); + +/** True when a changed file can shift dependency resolution repo-wide, forcing a full check. */ +export function filesTriggerFullRun(files: string[]): boolean { + return files.some((f) => ROOT_TRIGGERS.some((t) => f === t || f.startsWith(t))); +} + +interface WorkspacePkg { + dir: string; + relDir: string; + info: PackageJsonInfo; +} + +/** Every non-private workspace package: name -> { dir, relDir, info }. */ +async function loadWorkspace(rootDir: string): Promise> { + const byName = new Map(); + for (const file of await findPackageJsonFiles(rootDir)) { + const info = parsePackageJson(file); + if (info.private) continue; + byName.set(info.packageName, { dir: dirname(file), relDir: relativeDir(rootDir, file), info }); + } + return byName; +} + +/** Package dirs as forward-slash, trailing-slash prefixes (matches git's path output on any OS). */ +function packageDirPrefixes(byName: Map): Array<[string, string]> { + return [...byName.entries()].map(([name, { relDir }]) => [name, `${relDir}/`]); +} + +/** + * Pure core: map changed file paths to the owning workspace package by longest matching dir + * prefix. `dirs` is `[name, 'rel/dir/']` pairs; `files` are git's forward-slash paths. + */ +export function matchChangedFiles(files: string[], dirs: Array<[string, string]>): string[] { + const hit = new Set(); + for (const file of files) { + let best: { name: string; prefix: string } | null = null; + for (const [name, prefix] of dirs) { + if (file.startsWith(prefix) && (!best || prefix.length > best.prefix.length)) { + best = { name, prefix }; + } + } + if (best) hit.add(best.name); + } + return [...hit]; +} + +/** Map changed files (vs a git ref) to the publishable workspace packages they belong to. */ +function changedPackages( + baseRef: string, + byName: Map, + rootDir: string, +): string[] { + if (!baseRef || /^0+$/.test(baseRef)) { + console.error('No base commit (first push / force-push); skipping scoped check.'); + return []; + } + if (baseRef.startsWith('-')) { + console.error(`Refusing suspicious --changed ref "${baseRef}".`); + return []; + } + let out: string; + try { + out = execFileSync('git', ['diff', '--name-only', baseRef, 'HEAD'], { + cwd: rootDir, + encoding: 'utf8', + }); + } catch { + console.error(`Could not diff against "${baseRef}"; skipping scoped check.`); + return []; + } + const files = out.split('\n').filter(Boolean); + if (filesTriggerFullRun(files)) { + console.error('Catalog/tooling change detected; verifying all publishable packages.'); + return [...byName.keys()]; + } + return matchChangedFiles(files, packageDirPrefixes(byName)); +} + +/** BFS the workspace-internal dependency closure of the given target names. */ +export function closureOf(targets: string[], byName: Map): string[] { + const seen = new Set(); + const queue = [...targets]; + while (queue.length > 0) { + const name = queue.shift(); + if (name === undefined || seen.has(name)) continue; + const entry = byName.get(name); + if (!entry) continue; + seen.add(name); + for (const dep of entry.info.deps) { + if (CLOSURE_SECTIONS.has(dep.section) && byName.has(dep.name) && !seen.has(dep.name)) { + queue.push(dep.name); + } + } + } + return [...seen]; +} + +function resolveTargets( + args: string[], + byName: Map, + rootDir: string, +): string[] | null { + const changedArg = args.find((a) => a.startsWith('--changed=')); + if (args.includes('--all')) return [...byName.keys()]; + if (changedArg) { + const targets = changedPackages(changedArg.slice('--changed='.length), byName, rootDir); + if (targets.length === 0) { + console.log('No changed publishable packages; nothing to verify.'); + return null; + } + console.log(`Changed publishable packages: ${targets.join(', ')}`); + return targets; + } + return args.filter((a) => !a.startsWith('--')); +} + +/** + * Reproduce the `npm install` graph for the targeted publishable packages and run the closure + * verifier against it. Local pnpm dev and the `pnpm deploy` closure both apply root + * `pnpm.overrides`, which hide duplication; those don't travel in published tarballs, so + * `npm install` can resolve a second copy. Packs with `pnpm pack` (resolving `catalog:`/ + * `workspace:` like publishing does), installs into a scratch dir with npm, then verifies. + */ +export async function runVerifyNpmInstall(args: string[], rootDir: string): Promise { + const byName = await loadWorkspace(rootDir); + const targets = resolveTargets(args, byName, rootDir); + if (targets === null) return 0; + if (targets.length === 0) { + console.error('Usage: verify-npm-install (--all | --changed= | ...)'); + return 2; + } + const unknown = targets.filter((t) => !byName.has(t)); + if (unknown.length > 0) { + console.error(`Unknown publishable packages: ${unknown.join(', ')}`); + return 2; + } + + // Pack the targets plus every workspace package they depend on (their versions aren't on npm + // yet, so they must resolve to local tarballs via npm `overrides`). + const toPack = closureOf(targets, byName); + const work = mkdtempSync(join(tmpdir(), 'single-instance-npm-verify-')); + const tarballs = join(work, 'tarballs'); + const scratch = join(work, 'scratch'); + mkdirSync(tarballs, { recursive: true }); + mkdirSync(scratch, { recursive: true }); + + console.log(`Packing ${toPack.length} workspace package(s) (targets: ${targets.length})...`); + const tarballByName: Record = {}; + for (const name of toPack) { + const entry = byName.get(name); + if (!entry) continue; + const before = new Set(readdirSync(tarballs)); + execFileSync('pnpm', ['pack', '--pack-destination', tarballs], { + cwd: entry.dir, + stdio: ['ignore', 'ignore', 'inherit'], + }); + const produced = readdirSync(tarballs).find((f) => !before.has(f) && f.endsWith('.tgz')); + if (!produced) throw new Error(`pnpm pack produced no tarball for ${name}`); + tarballByName[name] = join(tarballs, produced); + } + + // Scratch project: install targets as file: deps, force ALL packed workspace deps to their + // local tarballs. Third-party deps resolve from the real npm registry. + const fileDep = (n: string): [string, string] => [n, `file:${tarballByName[n]}`]; + const overrides = Object.fromEntries(toPack.map(fileDep)); + const deps = Object.fromEntries(targets.map(fileDep)); + writeFileSync( + join(scratch, 'package.json'), + JSON.stringify( + { name: 'single-instance-scratch', private: true, dependencies: deps, overrides }, + null, + 2, + ), + ); + + console.log('Installing into scratch dir with npm...'); + execFileSync( + 'npm', + ['install', '--no-audit', '--no-fund', '--ignore-scripts', '--no-package-lock'], + { + cwd: scratch, + stdio: ['ignore', 'inherit', 'inherit'], + }, + ); + + const { duplicates, failures } = analyze(collectCopies(scratch)); + + console.log(`\nnpm-install closure — scratch: ${scratch}\n`); + for (const d of duplicates) { + const tag = d.isCurated ? (d.allowed ? 'ALLOWED DUP' : 'FAIL') : 'dup (report)'; + console.log( + ` ${d.name}: ${tag} — ${d.copies.length} copies (${d.copies.map((c) => `v${c.version}`).join(', ')})`, + ); + } + + if (failures.length === 0) rmSync(work, { recursive: true, force: true }); + else console.log(`\nScratch install kept for inspection: ${scratch}`); + + if (failures.length > 0) { + console.error( + `\nFAIL: ${failures.length} curated library duplicate(s) in the npm-install graph.`, + ); + return 1; + } + console.log('\nOK: no curated duplicates in the npm-install graph.'); + return 0; +} diff --git a/packages/testing/code-health/src/utils/package-json-scanner.test.ts b/packages/testing/code-health/src/utils/package-json-scanner.test.ts new file mode 100644 index 00000000000..9087721f4fc --- /dev/null +++ b/packages/testing/code-health/src/utils/package-json-scanner.test.ts @@ -0,0 +1,44 @@ +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +import { parsePackageJson, relativeDir } from './package-json-scanner.js'; + +describe('parsePackageJson', () => { + let dir: string; + + beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), 'pkg-scanner-')); + }); + afterEach(() => rmSync(dir, { recursive: true, force: true })); + + function write(pkg: Record): string { + const file = join(dir, 'package.json'); + writeFileSync(file, JSON.stringify(pkg, null, 2)); + return file; + } + + it('marks a package with "private": true as private', () => { + expect(parsePackageJson(write({ name: 'p', private: true })).private).toBe(true); + }); + + it('treats an omitted or false private field as not private', () => { + expect(parsePackageJson(write({ name: 'p' })).private).toBe(false); + expect(parsePackageJson(write({ name: 'p', private: false })).private).toBe(false); + }); + + it('does not treat a non-boolean private value as private', () => { + // only a strict `true` counts — a stray "false"/0 must not silently exclude a real package + expect(parsePackageJson(write({ name: 'p', private: 'false' })).private).toBe(false); + }); +}); + +describe('relativeDir', () => { + it('returns the package dir relative to root with forward slashes', () => { + const root = join('/repo'); + expect(relativeDir(root, join(root, 'packages', '@n8n', 'core', 'package.json'))).toBe( + 'packages/@n8n/core', + ); + }); +}); diff --git a/packages/testing/code-health/src/utils/package-json-scanner.ts b/packages/testing/code-health/src/utils/package-json-scanner.ts index def3da74f7b..ed852fe25b4 100644 --- a/packages/testing/code-health/src/utils/package-json-scanner.ts +++ b/packages/testing/code-health/src/utils/package-json-scanner.ts @@ -13,6 +13,7 @@ export interface PackageJsonDep { export interface PackageJsonInfo { filePath: string; packageName: string; + private: boolean; deps: PackageJsonDep[]; } @@ -24,6 +25,11 @@ export async function findPackageJsonFiles(rootDir: string): Promise { }); } +/** A package's directory relative to the repo root, normalized to forward slashes. */ +export function relativeDir(rootDir: string, filePath: string): string { + return path.relative(rootDir, path.dirname(filePath)).split(path.sep).join('/'); +} + export function parsePackageJson(filePath: string): PackageJsonInfo { const content = fs.readFileSync(filePath, 'utf-8'); const lines = content.split('\n'); @@ -31,7 +37,12 @@ export function parsePackageJson(filePath: string): PackageJsonInfo { try { pkg = JSON.parse(content) as Record; } catch { - return { filePath, packageName: path.basename(path.dirname(filePath)), deps: [] }; + return { + filePath, + packageName: path.basename(path.dirname(filePath)), + private: false, + deps: [], + }; } const packageName = (pkg.name as string) ?? path.basename(path.dirname(filePath)); @@ -53,7 +64,7 @@ export function parsePackageJson(filePath: string): PackageJsonInfo { } } - return { filePath, packageName, deps }; + return { filePath, packageName, private: pkg.private === true, deps }; } function findLineNumber(lines: string[], depName: string, section: string): number { diff --git a/packages/testing/rules-engine/src/baseline.test.ts b/packages/testing/rules-engine/src/baseline.test.ts index 62b6b2e94ef..f9ed83810df 100644 --- a/packages/testing/rules-engine/src/baseline.test.ts +++ b/packages/testing/rules-engine/src/baseline.test.ts @@ -119,3 +119,85 @@ describe('baseline', () => { expect(filtered.results[0].violations[0].file).toBe('/root/src/b.ts'); }); }); + +describe('generateBaseline carrying over uncovered rules', () => { + function reportFor(rule: string, messages: string[]): Report { + return { + timestamp: new Date().toISOString(), + projectRoot: '/root', + rules: { enabled: [rule], disabled: [] }, + results: [ + { + rule, + violations: messages.map((message) => ({ + file: '/root/src/a.ts', + line: 1, + column: 1, + rule, + message, + severity: 'error' as const, + })), + filesAnalyzed: 1, + executionTimeMs: 0, + }, + ], + summary: { + totalViolations: messages.length, + byRule: { [rule]: messages.length }, + bySeverity: { error: messages.length, warning: 0, info: 0 }, + filesAnalyzed: 1, + }, + }; + } + + it('keeps exceptions for a rule the report does not cover', () => { + const previous = generateBaseline(reportFor('disabled-rule', ['pre-existing']), '/root'); + + const regenerated = generateBaseline(reportFor('other-rule', ['new']), '/root', previous); + + const rules = regenerated.violations['src/a.ts'].map((e) => e.rule); + expect(rules).toContain('disabled-rule'); + expect(rules).toContain('other-rule'); + expect(regenerated.totalViolations).toBe(2); + }); + + // A rule scoped to changed files reports nothing when none of its files changed, which is + // indistinguishable from every violation having been fixed. + it('keeps entries a rule that did run no longer reports', () => { + const previous = generateBaseline( + reportFor('a-rule', ['unchanged file', 'still there']), + '/root', + ); + + const regenerated = generateBaseline(reportFor('a-rule', ['still there']), '/root', previous); + + expect(regenerated.violations['src/a.ts'].map((e) => e.message)).toEqual([ + 'unchanged file', + 'still there', + ]); + expect(regenerated.totalViolations).toBe(2); + }); + + it('does not duplicate a violation present in both the previous baseline and the report', () => { + const previous = generateBaseline(reportFor('a-rule', ['same']), '/root'); + + const regenerated = generateBaseline(reportFor('a-rule', ['same']), '/root', previous); + + expect(regenerated.violations['src/a.ts']).toHaveLength(1); + expect(regenerated.totalViolations).toBe(1); + }); + + it('counts the merged set, not just what the report saw', () => { + const previous = generateBaseline(reportFor('disabled-rule', ['one', 'two']), '/root'); + + const regenerated = generateBaseline(reportFor('other-rule', ['three']), '/root', previous); + + expect(regenerated.totalViolations).toBe(3); + }); + + it('builds from the report alone when given no previous baseline', () => { + const baseline = generateBaseline(reportFor('a-rule', ['only']), '/root'); + + expect(baseline.totalViolations).toBe(1); + }); +}); diff --git a/packages/testing/rules-engine/src/baseline.ts b/packages/testing/rules-engine/src/baseline.ts index 3cdd0ddb98e..98fb406f8eb 100644 --- a/packages/testing/rules-engine/src/baseline.ts +++ b/packages/testing/rules-engine/src/baseline.ts @@ -45,15 +45,40 @@ export function loadBaseline(filePath: string): BaselineFile | null { } } -export function generateBaseline(report: Report, rootDir: string): BaselineFile { +/** + * Merge `report`'s violations into `previous` and return the combined baseline. + * + * Additive on purpose. A violation absent from the report may have been fixed, but it may equally + * never have been looked at — the rule was disabled, or scoped to changed files and none of its + * files changed. Those two cases are indistinguishable from a report, so dropping entries would + * let a regeneration meant to record one new exception silently un-baseline unrelated rules. + * Matching is by hash, so an entry for a violation that no longer exists is inert. + * + * To shrink the baseline, delete the file and regenerate it in a run that covers every rule. + */ +export function generateBaseline( + report: Report, + rootDir: string, + previous?: BaselineFile | null, +): BaselineFile { const violations: Record = {}; + const seen = new Set(); + + // The hash covers path, rule and message, so it dedupes across previous and report alike. + const add = (relativePath: string, entry: BaselineEntry): void => { + if (seen.has(entry.hash)) return; + seen.add(entry.hash); + violations[relativePath] ??= []; + violations[relativePath].push(entry); + }; + + for (const [relativePath, entries] of Object.entries(previous?.violations ?? {})) { + for (const entry of entries) add(relativePath, entry); + } for (const result of report.results) { for (const violation of result.violations) { - const relativePath = path.relative(rootDir, violation.file); - if (!violations[relativePath]) violations[relativePath] = []; - - violations[relativePath].push({ + add(path.relative(rootDir, violation.file), { rule: violation.rule, line: violation.line, message: violation.message, @@ -65,7 +90,7 @@ export function generateBaseline(report: Report, rootDir: string): BaselineFile return { version: BASELINE_VERSION, generated: new Date().toISOString(), - totalViolations: report.summary.totalViolations, + totalViolations: seen.size, violations, }; } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index cdfc1478ec3..9826063b1f8 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -3878,7 +3878,7 @@ importers: specifier: ^3.5.2 version: 3.8.0 reflect-metadata: - specifier: ^0.2.2 + specifier: 'catalog:' version: 0.2.2 sha.js: specifier: ^2.4.12 diff --git a/scripts/build-n8n.mjs b/scripts/build-n8n.mjs index e21d731582d..5d7634e5840 100755 --- a/scripts/build-n8n.mjs +++ b/scripts/build-n8n.mjs @@ -266,6 +266,44 @@ echo( await $`cd ${config.rootDir} && NODE_ENV=production DOCKER_BUILD=true pnpm --filter=@n8n/task-runner --prod --legacy deploy --no-optional ${config.compiledTaskRunnerDir}`; +// Check the production closure for single-instance dependency duplication. A curated +// library resolving to more than one physical copy silently breaks instanceof / +// singletons at runtime. Report-first: a duplicate is surfaced loudly but does NOT fail +// the build — matching the continue-on-error npm-install CI jobs, so a transitive +// third-party re-split can't hard-break every nightly/release with no config escape. +// Promote to a hard gate once it has proven stable across releases. +// Both closures this build produces are checked. The task runner is deployed independently and +// ships as its own image, and `@n8n/task-runner` is a host package — it carries the curated libs as +// real dependencies rather than peers, so it is if anything the likelier place for a second copy. +const verifySingleInstance = async (label, dir) => { + echo(chalk.yellow(`INFO: Verifying single-instance dependency integrity in ${label}...`)); + // `--dir` rather than `--filter`: a filter that matches nothing exits 0, so a renamed or moved + // package would report a passing check having run no verifier at all. + const verifyProcess = $`cd ${config.rootDir} && pnpm --dir packages/testing/code-health exec tsx src/cli.ts verify-closure ${dir}`.nothrow(); + verifyProcess.pipe(process.stdout); + const { exitCode } = await verifyProcess; + // 0 and 3 are the only codes the verifier itself produces; everything else (tsx failing to load, + // a missing package or closure, a crash) means the closure was never checked. + if (exitCode === 0) { + echo(chalk.green(`✅ Single-instance dependency check passed for ${label}`)); + } else if (exitCode === 3) { + echo( + chalk.red( + `⚠️ Single-instance dependency duplication reported in ${label} (see above) — not failing the build (report-first).`, + ), + ); + } else { + echo( + chalk.red( + `⚠️ Single-instance verifier failed to run for ${label} (exit ${exitCode}); that closure was NOT checked. This is a tooling error, not a duplication report.`, + ), + ); + } +}; + +await verifySingleInstance('the production closure', config.compiledAppDir); +await verifySingleInstance('the JavaScript task runner closure', config.compiledTaskRunnerDir); + const packageDeployTime = getElapsedTime('package_deploy'); // Generate SBOM + render THIRD_PARTY_LICENSES.md from the deployed runtime closure. diff --git a/scripts/check-zod-peer-deps.mjs b/scripts/check-zod-peer-deps.mjs deleted file mode 100644 index a52eb9770c4..00000000000 --- a/scripts/check-zod-peer-deps.mjs +++ /dev/null @@ -1,97 +0,0 @@ -#!/usr/bin/env node - -/** - * Guards against an npm-only startup break: workspace packages that compose zod - * schemas across package boundaries must declare `zod` as a **peerDependency**, - * not a regular dependency. - * - * `@n8n/api-types` builds a `z.discriminatedUnion` over schemas created in - * `n8n-workflow`. That only works when both packages resolve the *same* physical - * zod instance. In the pnpm monorepo the catalog forces one instance, so it always - * holds locally. But on `npm install n8n`, if these packages each list `zod` in - * `dependencies`, npm can install a separate nested copy per package (langchain - * pins `zod@4` at the root, pushing our `zod@3` down), so the cross-package schema - * composition resolves different instances and n8n crashes at boot. Declaring zod - * as a peerDependency makes the consuming app provide a single shared instance. - * - * This exact regression shipped in 2.28.0 (#32386 moved zod peer→dependencies), - * reverting the fix from #28604. This check keeps zod a peer so it can't recur. - */ - -import { existsSync, readFileSync } from 'node:fs'; -import { dirname, join, resolve } from 'node:path'; -import { fileURLToPath } from 'node:url'; - -const root = resolve(dirname(fileURLToPath(import.meta.url)), '..'); - -// Packages that share zod schema objects across the workspace and are runtime -// dependencies of the `n8n` cli. zod must be a peer here, never a plain dependency. -export const PACKAGES_REQUIRING_ZOD_PEER = [ - 'packages/@n8n/api-types', - 'packages/workflow', - 'packages/core', - 'packages/@n8n/agents', - 'packages/@n8n/json-schema-to-zod', -]; - -/** Pure, testable core: return a problem string for a manifest, or null if OK. */ -export function checkManifest(name, pkg) { - const inDeps = pkg.dependencies?.zod; - const inPeer = pkg.peerDependencies?.zod; - if (inDeps) { - return `${name}: "zod" is in "dependencies" (${inDeps}); it must be a "peerDependency" instead.`; - } - if (!inPeer) { - return `${name}: "zod" is missing from "peerDependencies".`; - } - return null; -} - -function main() { - const problems = []; - for (const rel of PACKAGES_REQUIRING_ZOD_PEER) { - const file = join(root, rel, 'package.json'); - if (!existsSync(file)) continue; // package moved/renamed — don't hard-fail here - const pkg = JSON.parse(readFileSync(file, 'utf8')); - const problem = checkManifest(pkg.name ?? rel, pkg); - if (problem) problems.push(problem); - } - - if (problems.length > 0) { - console.error(''); - console.error('ERROR: zod must stay a peerDependency in schema-composing packages.'); - console.error(''); - console.error('Moving zod into "dependencies" lets npm install duplicate zod copies, which'); - console.error('breaks cross-package schema composition and crashes n8n at startup on'); - console.error('`npm install n8n` (regressed once in 2.28.0 via #32386).'); - console.error(''); - for (const p of problems) console.error(` - ${p}`); - console.error(''); - console.error( - 'Move "zod" to "peerDependencies" (keep it in "devDependencies" for local builds).', - ); - console.error(''); - process.exit(1); - } - - console.log( - `OK: zod is a peerDependency in all ${PACKAGES_REQUIRING_ZOD_PEER.length} schema-composing packages.`, - ); -} - -// `--self-test` exercises the core logic without touching the workspace. -if (process.argv.includes('--self-test')) { - const good = checkManifest('good', { peerDependencies: { zod: 'catalog:' } }) === null; - const badDep = checkManifest('bad', { dependencies: { zod: 'catalog:' } }) !== null; - const badMissing = checkManifest('missing', {}) !== null; - if (good && badDep && badMissing) { - console.log('self-test passed'); - process.exit(0); - } - console.error(`self-test FAILED (good=${good}, badDep=${badDep}, badMissing=${badMissing})`); - process.exit(1); -} - -if (import.meta.url === `file://${process.argv[1]}`) { - main(); -}