From 9e39914cf43cf4222371194982a66e97a4874996 Mon Sep 17 00:00:00 2001 From: "n8n-cat-bot[bot]" <283985454+n8n-cat-bot[bot]@users.noreply.github.com> Date: Sat, 30 May 2026 21:41:18 +0100 Subject: [PATCH] ci: Wire workflow/core through janitor + fix global-trigger scope gap (DEVP-195) (#31415) Co-authored-by: n8n-cat-bot[bot] Co-authored-by: Claude Opus 4.7 Co-authored-by: Declan Carroll --- packages/core/package.json | 2 + packages/testing/janitor/README.md | 22 ++++++++ .../core/affected-packages-analyzer.test.ts | 35 +++++++++--- .../src/core/affected-packages-analyzer.ts | 22 +------- .../janitor/src/core/global-triggers.ts | 41 ++++++++++++++ .../janitor/src/core/scope-analyzer.test.ts | 54 +++++++++++++++++++ .../janitor/src/core/scope-analyzer.ts | 11 ++++ packages/workflow/package.json | 2 + pnpm-lock.yaml | 6 +++ 9 files changed, 169 insertions(+), 26 deletions(-) create mode 100644 packages/testing/janitor/src/core/global-triggers.ts diff --git a/packages/core/package.json b/packages/core/package.json index ce5a68b6cf0..dfcaca46536 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -22,6 +22,7 @@ "watch": "tsc-watch -p tsconfig.build.json --onCompilationComplete \"tsc-alias -p tsconfig.build.json\"", "test": "vitest run", "test:unit": "vitest run", + "test:changed": "janitor test-scoped --runner=vitest", "test:dev": "vitest --silent=false" }, "files": [ @@ -30,6 +31,7 @@ ], "devDependencies": { "@n8n/errors": "workspace:*", + "@n8n/playwright-janitor": "workspace:*", "@n8n/typescript-config": "workspace:*", "@types/express": "catalog:", "@types/jsonwebtoken": "catalog:", diff --git a/packages/testing/janitor/README.md b/packages/testing/janitor/README.md index abc205de24d..e6f92b30c7e 100644 --- a/packages/testing/janitor/README.md +++ b/packages/testing/janitor/README.md @@ -243,6 +243,28 @@ plus setup files at `/jest.setup.*`, `/vitest.setup.*`, and `../cli/src/public-api/v1/**/*.yml` is honoured — a change to that yml marks nodes-base as affected. +**Global triggers force a full workspace run.** Some changes are invisible to +a per-package import-graph walk: a lockfile / root-manifest change, or an edit +to a universal sink (`packages/@n8n/db`, `packages/workflow`, `packages/core`) +whose runtime coupling to downstream packages isn't expressed as a static +import the test file can see. For these, scoping to "files in this package" +would find nothing and emit `SKIP` on every downstream — a silent false green. + +The trigger list lives in one place, `core/global-triggers.ts` +(`GLOBAL_TRIGGER_FILES` for exact filenames, `GLOBAL_TRIGGER_PREFIXES` for +directories), and is consulted at **both** layers of the pipeline: + +* `affectedPackages()` returns every package, so all jobs are listed as + affected; and +* `computeScope()` returns `RUN_FULL` for the package, so each job actually + runs its full suite instead of skipping. + +Both checks are required — `affectedPackages` alone only decides which jobs are +*listed*; without the `computeScope` check the job would still `SKIP`. The +trade-off is over-testing on the rare PRs that touch these paths (the failure +mode is "ran too much", never "ran nothing"). To add a new universal sink, add +its directory prefix to `GLOBAL_TRIGGER_PREFIXES`. + ## Rules ### Architecture Rules diff --git a/packages/testing/janitor/src/core/affected-packages-analyzer.test.ts b/packages/testing/janitor/src/core/affected-packages-analyzer.test.ts index 4bde96648cb..f3d96dac4b1 100644 --- a/packages/testing/janitor/src/core/affected-packages-analyzer.test.ts +++ b/packages/testing/janitor/src/core/affected-packages-analyzer.test.ts @@ -76,18 +76,41 @@ describe('affectedPackages', () => { }); it('includes transitive downstream packages', () => { + // Uses non-global-trigger package names so this exercises the dep-graph + // walk, not the workspace-wide bailout (workflow/core ARE global triggers). const rootDir = makeFixture({ patterns: ['packages/*'], packages: { - 'packages/workflow': { name: 'workflow' }, - 'packages/core': { name: 'core', deps: ['workflow'] }, - 'packages/cli': { name: 'cli', deps: ['core'] }, + 'packages/lib': { name: 'lib' }, + 'packages/mid': { name: 'mid', deps: ['lib'] }, + 'packages/app': { name: 'app', deps: ['mid'] }, 'packages/unrelated': { name: 'unrelated' }, }, }); - expect(affectedPackages({ rootDir, changedFiles: ['packages/workflow/src/index.ts'] })).toEqual( - ['cli', 'core', 'workflow'], - ); + expect(affectedPackages({ rootDir, changedFiles: ['packages/lib/src/index.ts'] })).toEqual([ + 'app', + 'lib', + 'mid', + ]); + }); + + it('expands all packages when a universal sink (workflow/core) changes', () => { + const rootDir = makeFixture({ + patterns: ['packages/*'], + packages: { + 'packages/workflow': { name: 'n8n-workflow' }, + 'packages/core': { name: 'n8n-core' }, + 'packages/unrelated': { name: 'unrelated' }, + }, + }); + expect( + affectedPackages({ rootDir, changedFiles: ['packages/workflow/src/Workflow.ts'] }), + ).toEqual(['n8n-core', 'n8n-workflow', 'unrelated']); + expect(affectedPackages({ rootDir, changedFiles: ['packages/core/src/x.ts'] })).toEqual([ + 'n8n-core', + 'n8n-workflow', + 'unrelated', + ]); }); it('expands all packages when pnpm-lock.yaml changes', () => { diff --git a/packages/testing/janitor/src/core/affected-packages-analyzer.ts b/packages/testing/janitor/src/core/affected-packages-analyzer.ts index dd17974ef91..fd550c1ac2b 100644 --- a/packages/testing/janitor/src/core/affected-packages-analyzer.ts +++ b/packages/testing/janitor/src/core/affected-packages-analyzer.ts @@ -10,6 +10,7 @@ import { existsSync, readFileSync } from 'node:fs'; import { join, relative } from 'node:path'; import { parse as parseYaml } from 'yaml'; +import { matchesGlobalTrigger } from './global-triggers.js'; import { findWorkspaceRoot, toPosix } from './path-utils.js'; function parseJsonFile(path: string): T { @@ -34,18 +35,6 @@ export interface AnalyzeOptions { changedFiles: string[] | null; } -const GLOBAL_TRIGGER_FILES = new Set(['pnpm-lock.yaml', 'package.json']); -// Directory prefixes whose contents force RUN_FULL across the workspace. Used -// when a package's contract is consumed at runtime by every other package and -// the workspace symlink/dep-graph alone can't catch the coupling: -// - packages/@n8n/db — schema + entities resolved by cli integration tests -// through the DI container at runtime; jest --findRelatedTests on the -// test file alone wouldn't see the migration/entity file in the import -// graph and would silently SKIP. Bailing the workspace is over-broad on -// rare db PRs but keeps the failure mode "ran too much" rather than -// "ran nothing". -const GLOBAL_TRIGGER_PREFIXES = ['packages/@n8n/db/']; - function loadWorkspacePackages(rootDir: string): WorkspacePackage[] { const wsFile = join(rootDir, 'pnpm-workspace.yaml'); if (!existsSync(wsFile)) throw new Error(`pnpm-workspace.yaml not found at ${wsFile}`); @@ -131,14 +120,7 @@ export function affectedPackages(options: AnalyzeOptions): string[] { // No signal (local dev, missing env) → safest default: everything. if (options.changedFiles === null) return allNames; - if ( - options.changedFiles.some( - (f) => - GLOBAL_TRIGGER_FILES.has(f) || - GLOBAL_TRIGGER_PREFIXES.some((prefix) => f.startsWith(prefix)), - ) - ) - return allNames; + if (options.changedFiles.some(matchesGlobalTrigger)) return allNames; const direct = new Set(); for (const file of options.changedFiles) { diff --git a/packages/testing/janitor/src/core/global-triggers.ts b/packages/testing/janitor/src/core/global-triggers.ts new file mode 100644 index 00000000000..be593576a5d --- /dev/null +++ b/packages/testing/janitor/src/core/global-triggers.ts @@ -0,0 +1,41 @@ +/** + * Files and directory prefixes whose changes force RUN_FULL across the whole + * workspace. + * + * These MUST be consulted at BOTH layers of the scoping pipeline: + * - `affectedPackages()` — returns every package so all test jobs are listed + * as affected and provision a runner. + * - `computeScope()` — returns RUN_FULL so each job actually executes its + * full suite instead of SKIPping on "no changed files in package". + * + * Both checks are required. `affectedPackages` alone only decides which jobs + * are *listed*; without the `computeScope` check a global-trigger change makes + * every scoped package SKIP — running zero tests and reporting a false green. + */ +export const GLOBAL_TRIGGER_FILES = new Set(['pnpm-lock.yaml', 'package.json']); + +/** + * Directory prefixes consumed at runtime by packages that don't import them in + * a way the test runner's import-graph walk can see. A change here is invisible + * to `jest --findRelatedTests` / `vitest related` on the test file, so we bail + * the workspace to full rather than silently skip: + * - `packages/@n8n/db/` — schema + entities resolved through the DI container + * by every consuming package's integration tests. + * - `packages/workflow/`, `packages/core/` — universal sinks imported by + * ~everything; a behaviour change that keeps the same type signature is not + * visible to a downstream import-graph walk (typecheck only catches the + * contract). DEVP-195. + * + * Over-broad on the rare PRs that touch these (keeps the failure mode "ran too + * much" rather than "ran nothing"), which is the intended trade-off. + */ +export const GLOBAL_TRIGGER_PREFIXES = [ + 'packages/@n8n/db/', + 'packages/workflow/', + 'packages/core/', +]; + +/** True when a repo-root-relative path forces a full workspace run. */ +export function matchesGlobalTrigger(file: string): boolean { + return GLOBAL_TRIGGER_FILES.has(file) || GLOBAL_TRIGGER_PREFIXES.some((p) => file.startsWith(p)); +} diff --git a/packages/testing/janitor/src/core/scope-analyzer.test.ts b/packages/testing/janitor/src/core/scope-analyzer.test.ts index b8dd709a2a9..1b8d109c0f3 100644 --- a/packages/testing/janitor/src/core/scope-analyzer.test.ts +++ b/packages/testing/janitor/src/core/scope-analyzer.test.ts @@ -252,4 +252,58 @@ describe('computeScope', () => { expect(result.kind).toBe('scoped'); }); }); + + describe('global triggers force RUN_FULL', () => { + it.each([ + ['pnpm-lock.yaml', 'pnpm-lock.yaml'], + ['root package.json', 'package.json'], + ['@n8n/db entity', 'packages/@n8n/db/src/entities/user.entity.ts'], + ['workflow source', 'packages/workflow/src/Workflow.ts'], + ['core source', 'packages/core/src/x.ts'], + ])('bails to full on %s even when nothing changed in-package', (_label, changed) => { + const rootDir = makePackageDir('packages/cli'); + const result = computeScope({ + runner: 'jest', + jestVariant: 'integration', + packageDir: 'packages/cli', + rootDir, + changedFiles: [changed], + }); + expect(result.kind).toBe('full'); + expect(formatScope(result)).toBe('RUN_FULL'); + }); + + it('a universal-sink change forces full for a vitest downstream too', () => { + const rootDir = makePackageDir('packages/nodes-base'); + const result = computeScope({ + runner: 'vitest', + packageDir: 'packages/nodes-base', + rootDir, + changedFiles: ['packages/workflow/src/Workflow.ts'], + }); + expect(result.kind).toBe('full'); + }); + + it('does NOT treat a per-package package.json as a global trigger', () => { + const rootDir = makePackageDir('packages/cli'); + const result = computeScope({ + runner: 'jest', + packageDir: 'packages/cli', + rootDir, + changedFiles: ['packages/other/package.json'], + }); + expect(result.kind).toBe('skip'); + }); + + it('still SKIPs an unrelated cross-package change', () => { + const rootDir = makePackageDir('packages/cli'); + const result = computeScope({ + runner: 'jest', + packageDir: 'packages/cli', + rootDir, + changedFiles: ['packages/nodes-base/nodes/Slack/Slack.node.ts'], + }); + expect(result.kind).toBe('skip'); + }); + }); }); diff --git a/packages/testing/janitor/src/core/scope-analyzer.ts b/packages/testing/janitor/src/core/scope-analyzer.ts index 3085dc24ece..ae3427538d2 100644 --- a/packages/testing/janitor/src/core/scope-analyzer.ts +++ b/packages/testing/janitor/src/core/scope-analyzer.ts @@ -8,6 +8,7 @@ import { existsSync } from 'node:fs'; import { isAbsolute, relative, resolve } from 'node:path'; +import { matchesGlobalTrigger } from './global-triggers.js'; import { toPosix } from './path-utils.js'; export type Runner = 'jest' | 'vitest'; @@ -95,6 +96,16 @@ export function computeScope(options: ComputeScopeOptions): ScopeResult { return { kind: 'full', reason: 'No CHANGED_FILES signal (local dev)' }; } + // Workspace-wide triggers (lockfile, root manifest, universal sinks like + // @n8n/db / workflow / core) force RUN_FULL regardless of which package we + // are scoping. The dep-graph in affected-packages lists the package as + // affected, but the in-package filter below would otherwise SKIP it because + // the trigger file lives outside the package — a silent false green. + const globalTrigger = options.changedFiles.find(matchesGlobalTrigger); + if (globalTrigger) { + return { kind: 'full', reason: 'Global trigger changed', trigger: globalTrigger }; + } + const absolute = isAbsolute(options.packageDir) ? options.packageDir : resolve(options.rootDir, options.packageDir); diff --git a/packages/workflow/package.json b/packages/workflow/package.json index c794c0748ae..4897dc04fa4 100644 --- a/packages/workflow/package.json +++ b/packages/workflow/package.json @@ -31,6 +31,7 @@ "watch": "tsc --build tsconfig.build.esm.json tsconfig.build.cjs.json --watch", "test": "vitest run", "test:unit": "vitest run", + "test:changed": "janitor test-scoped --runner=vitest", "test:dev": "vitest --watch", "mutate": "node scripts/mutate.mjs" }, @@ -40,6 +41,7 @@ "devDependencies": { "@langchain/core": "catalog:", "@n8n/config": "workspace:*", + "@n8n/playwright-janitor": "workspace:*", "@n8n/typescript-config": "workspace:*", "@n8n/vitest-config": "workspace:*", "@stryker-mutator/core": "catalog:", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 4299cf08553..68739ca2bed 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -3369,6 +3369,9 @@ importers: '@n8n/errors': specifier: workspace:* version: link:../@n8n/errors + '@n8n/playwright-janitor': + specifier: workspace:* + version: link:../testing/janitor '@n8n/typescript-config': specifier: workspace:* version: link:../@n8n/typescript-config @@ -5079,6 +5082,9 @@ importers: '@n8n/config': specifier: workspace:* version: link:../@n8n/config + '@n8n/playwright-janitor': + specifier: workspace:* + version: link:../testing/janitor '@n8n/typescript-config': specifier: workspace:* version: link:../@n8n/typescript-config