From 898480070f22aa9abc63340c04cd4f8eeaea6ab3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milorad=20FIlipovi=C4=87?= Date: Fri, 18 Sep 2026 21:19:44 +0000 Subject: [PATCH] fix(core): Isolate instance settings writes (no-changelog) (#39118) Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> --- .agents/skills/conventions/SKILL.md | 3 +++ .agents/skills/reproduce-bug/SKILL.md | 6 +++++ AGENTS.md | 11 ++++++++++ ...process-process-run-execution-data.test.ts | 1 + .../__tests__/workflow-execute.test.ts | 1 + packages/core/test/setup-mocks.ts | 22 +++++++++++++++++++ 6 files changed, 44 insertions(+) diff --git a/.agents/skills/conventions/SKILL.md b/.agents/skills/conventions/SKILL.md index 641af0b1c74..7c877a58eb5 100644 --- a/.agents/skills/conventions/SKILL.md +++ b/.agents/skills/conventions/SKILL.md @@ -45,6 +45,9 @@ throw new UnexpectedError('message', { extra: { context } }); **Testing:** - Vitest (unit), Playwright (E2E) - Mock external dependencies +- Keep filesystem tests in a test-owned temporary directory +- Set `N8N_USER_FOLDER` before importing settings code +- Trace branches activated by mocks and isolate every reachable mutation - Work from package directory: `pushd packages/cli && pnpm test` **Database:** diff --git a/.agents/skills/reproduce-bug/SKILL.md b/.agents/skills/reproduce-bug/SKILL.md index 3a8ca1a927c..36b346006ed 100644 --- a/.agents/skills/reproduce-bug/SKILL.md +++ b/.agents/skills/reproduce-bug/SKILL.md @@ -73,6 +73,12 @@ Write a regression test that: - Asserts the CORRECT behavior (test will fail on current code) - Also includes a "happy path" test to prove the setup works +Complete the test safety gate in `/AGENTS.md` before you run the test: +- Trace side effects from imports, constructors, hooks, and mocked branches +- Keep filesystem access in a test-owned temporary directory +- Set `N8N_USER_FOLDER` before importing modules that resolve it +- When a mock changes a state check, inspect and isolate the branch that it activates + ## Step 8: Run and Score Run the test from the package directory (e.g., `cd packages/nodes-base && pnpm test `). diff --git a/AGENTS.md b/AGENTS.md index 376c610588b..6e8db4f6e42 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -342,6 +342,17 @@ Testing rules: - Run tests and `pnpm typecheck` from the owning package. - Confirm unit test cases with the user before you write them. - Mock external dependencies. Use `nock` for HTTP services. +- Trace side effects from imports, constructors, hooks, and mocked branches before + you run a new or changed test. +- Do not let tests read from or write to the developer's home directory, + `~/.n8n`, or other user-owned locations. +- Use a test-owned temporary directory for filesystem tests. Set ++ `N8N_USER_FOLDER` before you import modules that resolve it. n8n writes to `${N8N_USER_FOLDER}/.n8n`, so expect the `.n8n` subfolder there. +- When a mock changes a state check such as `existsSync()`, inspect the branch + that it activates. Mock every reachable filesystem mutation unless filesystem + behavior is under test. +- Run tests that can initialize n8n settings with an isolated + `N8N_USER_FOLDER` first. Clean up only paths that the test created. - Reuse immutable hoisted `mock(...)` fixtures. Do not replace typed entity mocks with `as unknown as T`. - Use `createVitestConfigWithDecorators` for Vitest packages that use `@n8n/di` decorators. - Check for unused computed properties after you change a Pinia store. diff --git a/packages/core/src/execution-engine/__tests__/workflow-execute-process-process-run-execution-data.test.ts b/packages/core/src/execution-engine/__tests__/workflow-execute-process-process-run-execution-data.test.ts index 8d77d61bad6..1412967ebe2 100644 --- a/packages/core/src/execution-engine/__tests__/workflow-execute-process-process-run-execution-data.test.ts +++ b/packages/core/src/execution-engine/__tests__/workflow-execute-process-process-run-execution-data.test.ts @@ -23,6 +23,7 @@ vi.mock('node:fs', async (importActual) => ({ ...(await importActual()), existsSync: vi.fn().mockReturnValue(false), renameSync: vi.fn(), + writeFileSync: vi.fn(), })); import { DirectedGraph } from '../partial-execution-utils'; diff --git a/packages/core/src/execution-engine/__tests__/workflow-execute.test.ts b/packages/core/src/execution-engine/__tests__/workflow-execute.test.ts index c7419a08e38..4a3a2e7ed37 100644 --- a/packages/core/src/execution-engine/__tests__/workflow-execute.test.ts +++ b/packages/core/src/execution-engine/__tests__/workflow-execute.test.ts @@ -64,6 +64,7 @@ vi.mock('node:fs', async (importActual) => ({ ...(await importActual()), existsSync: vi.fn().mockReturnValue(false), renameSync: vi.fn(), + writeFileSync: vi.fn(), })); const nodeTypes = Helpers.NodeTypes(); diff --git a/packages/core/test/setup-mocks.ts b/packages/core/test/setup-mocks.ts index d2c9bc6e645..6a8f674a127 100644 --- a/packages/core/test/setup-mocks.ts +++ b/packages/core/test/setup-mocks.ts @@ -1 +1,23 @@ +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + import 'reflect-metadata'; + +const originalUserFolder = process.env.N8N_USER_FOLDER; +const testUserFolder = mkdtempSync(join(tmpdir(), 'n8n-core-test-')); + +process.env.N8N_USER_FOLDER = testUserFolder; + +afterAll(() => { + if (originalUserFolder === undefined) { + delete process.env.N8N_USER_FOLDER; + } else { + process.env.N8N_USER_FOLDER = originalUserFolder; + } + + rmSync(testUserFolder, { recursive: true, force: true }); +}); + +// Best-effort cleanup for runs interrupted before afterAll executes (SIGINT/worker kill). +process.on('exit', () => rmSync(testUserFolder, { recursive: true, force: true }));