From efd3ec9e260ef9ac7d9cbc134952590bedd33acb Mon Sep 17 00:00:00 2001 From: "n8n-cat-bot[bot]" <283985454+n8n-cat-bot[bot]@users.noreply.github.com> Date: Wed, 2 Sep 2026 17:55:08 +0000 Subject: [PATCH] =?UTF-8?q?chore:=20DEVP-1038=20=C2=B7=20c1:=20Fix=20worki?= =?UTF-8?q?ng-tree=20restore=20and=20stryker-setup=20file=20cleanup=20(#37?= =?UTF-8?q?672)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: n8n-cat-bot[bot] Co-authored-by: Claude Opus 5 --- scripts/mutation-health/README.md | 22 +- scripts/mutation-health/mutate.mjs | 250 +++++++++++++++++-- scripts/mutation-health/mutate.test.mjs | 307 +++++++++++++++++++++++- 3 files changed, 554 insertions(+), 25 deletions(-) diff --git a/scripts/mutation-health/README.md b/scripts/mutation-health/README.md index bf95b75ee62..f6a271a9106 100644 --- a/scripts/mutation-health/README.md +++ b/scripts/mutation-health/README.md @@ -109,10 +109,24 @@ Runs use Stryker's `--inPlace`. Its default sandbox copy breaks on any package w config resolves a workspace dependency through a path alias — the alias doesn't survive the copy, and `packages/cli` dies on `ERR_LOAD_URL … .stryker-tmp/@n8n/backend-test-utils`. -Stryker restores your files on a clean exit and on `Ctrl-C`. Because a hard crash would not, -and because in `--diff` mode those files hold *uncommitted work* (so `git checkout --` is not a -safe undo), `mutate.mjs` snapshots the exact bytes of every target before the run and writes -them back if they differ afterwards. +Stryker restores your files on a clean exit and on `Ctrl-C`, but not after a crash, a timeout or +a `SIGTERM` — and its preprocessing reaches past the mutate targets, so a target-only snapshot +left mutated files behind. + +`mutate.mjs` therefore snapshots the **whole working tree** before the run: + +- the exact bytes of every tracked file that is already dirty, plus every target. In `--diff` + mode those files hold *uncommitted work*, so `git checkout --` is not a safe undo. +- the dirty set itself. Anything dirty *after* the run that was clean before it was changed by + Stryker, and git holds its pre-run state, so `git checkout --` is the right undo there. + +One cleanup routine restores that snapshot and deletes every `stryker-setup-*.js` the vitest +runner left under the repo root. It is idempotent and registered on every exit path: the usual +one, an uncaught exception, `SIGINT` and `SIGTERM`. A cancelled run exits `130` (`143` for +`SIGTERM`) with a clean `git status`. + +One caveat: files you edit in another terminal *while a run is in flight* look like Stryker's +work and are reverted. Don't edit the repo during a run. ## Which packages can be scored diff --git a/scripts/mutation-health/mutate.mjs b/scripts/mutation-health/mutate.mjs index a4b9aefe273..23ebf7123b2 100644 --- a/scripts/mutation-health/mutate.mjs +++ b/scripts/mutation-health/mutate.mjs @@ -47,7 +47,7 @@ import { spawn, spawnSync } from 'node:child_process'; import { readFile, writeFile, mkdir, rm } from 'node:fs/promises'; -import { existsSync, readFileSync, writeFileSync } from 'node:fs'; +import { existsSync, readFileSync, readdirSync, unlinkSync, writeFileSync } from 'node:fs'; import { createRequire } from 'node:module'; import { fileURLToPath } from 'node:url'; import path from 'node:path'; @@ -452,14 +452,77 @@ function resolveStrykerBin(pkgRoot, packageDir) { ); } +// --- working-tree snapshot and cleanup --- +// // Runs use `--inPlace`, because the sandbox copy breaks each package whose // vitest config finds a workspace dependency through a path alias. The alias // does not stay correct in the copy. See the README for the failure. // // Stryker restores the files after a usual exit and after SIGINT, but not after -// a crash. In diff mode the files hold uncommitted work, thus `git checkout --` -// is not a safe undo. Keep a copy of the bytes and write them back instead. -function snapshotFiles(absPaths) { +// a crash, a timeout, or a SIGTERM. Its preprocessing also reaches past the +// mutate targets: it instruments the related test graph and drops a +// `stryker-setup-.js` in the run cwd. A target-only snapshot therefore +// left mutated files and setup files behind. Snapshot the whole working tree +// instead, and run one cleanup routine on every exit path. + +// Glob `stryker-setup-*.js`, anchored to one path segment. +const STRYKER_SETUP_FILE = /^stryker-setup-[^/\\]*\.js$/; + +// Directories the walk never enters. `node_modules` alone makes a full-repo +// walk take minutes, and none of these hold working-tree state to clean. +const UNWALKED_DIRS = new Set([ + '.git', + 'node_modules', + 'dist', + 'coverage', + '.turbo', + '.stryker-tmp', +]); + +/** + * Every `stryker-setup-*.js` under `root`. The vitest runner writes one for + * each worker into the run cwd and does not always remove it, thus the search + * covers the whole repo, not only the package dir Stryker ran in. + */ +export function findStrykerSetupFiles(root) { + const found = []; + const queue = [root]; + while (queue.length > 0) { + const dir = queue.pop(); + let entries; + try { + entries = readdirSync(dir, { withFileTypes: true }); + } catch { + continue; // unreadable directory — nothing to collect + } + for (const entry of entries) { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) { + if (!UNWALKED_DIRS.has(entry.name)) queue.push(full); + } else if (STRYKER_SETUP_FILE.test(entry.name)) { + found.push(full); + } + } + } + return found; +} + +/** Delete every `stryker-setup-*.js` under `root`. Returns the paths removed. */ +export function removeStrykerSetupFiles(root) { + const removed = []; + for (const file of findStrykerSetupFiles(root)) { + try { + unlinkSync(file); + removed.push(file); + } catch { + // Already gone. There is nothing to remove. + } + } + return removed; +} + +/** Keep the exact bytes of each path, so a restore never needs git. */ +export function snapshotFiles(absPaths) { const snap = new Map(); for (const p of absPaths) { try { @@ -471,13 +534,14 @@ function snapshotFiles(absPaths) { return snap; } -function restoreFiles(snap) { +/** Write the snapshot back where it differs. Returns the paths restored. */ +export function restoreFiles(snap) { const restored = []; for (const [p, original] of snap) { try { if (!readFileSync(p).equals(original)) { writeFileSync(p, original); - restored.push(path.relative(repoRoot, p)); + restored.push(p); } } catch { // The file is gone. There is nothing to restore. @@ -486,6 +550,130 @@ function restoreFiles(snap) { return restored; } +/** + * Tracked files that were clean before the run and are dirty after it. Stryker + * changed them, and git holds their pre-run state, thus `git checkout --` is + * the correct undo. A file that was already dirty is excluded: it carries the + * user's own uncommitted work, which the byte snapshot restores instead. + */ +export function filesToCheckout(dirtyBefore, dirtyAfter) { + const before = new Set(dirtyBefore); + return dirtyAfter.filter((f) => !before.has(f)); +} + +/** + * The one cleanup routine every exit path shares: restore the working tree, + * then delete the `stryker-setup-*.js` files. Idempotent — the normal path, a + * crash, SIGINT and SIGTERM all call it, and only the first call does the work. + * A failing restore must not keep the setup files on disk, so each half is + * guarded on its own. + */ +export function createCleanup({ restore, removeSetupFiles, report }) { + let result; + return function cleanup() { + if (result) return result; + result = { restored: [], removed: [] }; + try { + result.restored = restore() ?? []; + } catch { + // Report what the second half did even when the restore fails. + } + try { + result.removed = removeSetupFiles() ?? []; + } catch { + // Nothing more to clean. + } + report?.(result); + return result; + }; +} + +/** + * Wire `cleanup` onto every exit path: the usual one (`exit`), a crash + * (`uncaughtException`), and a cancellation (`SIGINT` / `SIGTERM`). + * + * `onSignal` gets first refusal on a signal. It returns true when it told a + * live Stryker to stop; the run then unwinds and cleans up once the child is + * gone, because a restore that races Stryker's own writes fixes nothing. + * Otherwise this handler cleans up and leaves at once. + * + * `proc`, `exit` and `write` are injected so the unit tests can drive the + * handlers without signalling or ending the test runner. + */ +export function registerCleanupHandlers({ + cleanup, + onSignal, + proc = process, + exit = (code) => process.exit(code), + write = (msg) => process.stderr.write(msg), +}) { + const handleExit = () => cleanup(); + const handleUncaught = (err) => { + cleanup(); + write(`\n✗ mutate.mjs crashed: ${err?.stack ?? err}\n`); + exit(3); + }; + const handleSignal = (signal) => { + if (onSignal?.(signal)) return; + cleanup(); + exit(signal === 'SIGTERM' ? 143 : 130); + }; + const handleSigint = () => handleSignal('SIGINT'); + const handleSigterm = () => handleSignal('SIGTERM'); + + proc.on('exit', handleExit); + proc.on('uncaughtException', handleUncaught); + proc.on('SIGINT', handleSigint); + proc.on('SIGTERM', handleSigterm); + + return { + dispose() { + proc.off('exit', handleExit); + proc.off('uncaughtException', handleUncaught); + proc.off('SIGINT', handleSigint); + proc.off('SIGTERM', handleSigterm); + }, + }; +} + +// Tracked files that differ from HEAD, staged or not. Untracked files are left +// out: they have no pre-run state to go back to. This never dies — it runs +// inside the cleanup handlers, where exiting would skip the rest of the work. +function listDirtyFiles() { + const res = spawnSync('git', ['diff', '--name-only', 'HEAD'], { + cwd: repoRoot, + encoding: 'utf8', + maxBuffer: 64 * 1024 * 1024, + }); + if (res.error || res.status !== 0) return []; + return res.stdout + .split('\n') + .map((s) => s.trim()) + .filter(Boolean); +} + +// Bytes for every file that is already dirty plus the ones the run will touch, +// and the dirty set itself so the restore can tell Stryker's edits from the +// user's. +function snapshotWorkingTree(extraAbsPaths = []) { + const dirtyBefore = listDirtyFiles(); + const paths = new Set([...dirtyBefore.map((f) => path.join(repoRoot, f)), ...extraAbsPaths]); + return { bytes: snapshotFiles([...paths]), dirtyBefore }; +} + +function restoreWorkingTree({ bytes, dirtyBefore }) { + const restored = new Set(restoreFiles(bytes).map((p) => path.relative(repoRoot, p))); + const checkout = filesToCheckout(dirtyBefore, listDirtyFiles()); + if (checkout.length > 0) { + const res = spawnSync('git', ['checkout', '--', ...checkout], { + cwd: repoRoot, + encoding: 'utf8', + }); + if (!res.error && res.status === 0) for (const f of checkout) restored.add(f); + } + return [...restored]; +} + async function runJob({ pkgRoot, packageDir, targets }, { configArg }) { const configPath = resolveConfig(pkgRoot, configArg); const strykerBin = resolveStrykerBin(pkgRoot, packageDir); @@ -507,17 +695,42 @@ async function runJob({ pkgRoot, packageDir, targets }, { configArg }) { `(config: ${path.relative(repoRoot, configPath)}, threshold: ${THRESHOLD}%)\n`, ); - const snap = snapshotFiles([ + const snapshot = snapshotWorkingTree([ ...new Set(targets.map((t) => path.join(pkgRoot, splitRange(t).file))), ]); const strykerOutputChunks = []; let child; - const onSignal = () => { - child?.kill('SIGINT'); - }; - process.on('SIGINT', onSignal); - process.on('SIGTERM', onSignal); + let signalled; + const cleanup = createCleanup({ + restore: () => restoreWorkingTree(snapshot), + removeSetupFiles: () => removeStrykerSetupFiles(repoRoot), + report: ({ restored, removed }) => { + if (restored.length > 0) { + process.stderr.write( + `⚠ Stryker left changes behind; restored ${restored.length} file(s): ` + + `${restored.join(', ')}\n`, + ); + } + if (removed.length > 0) { + process.stderr.write( + ` removed ${removed.length} stryker-setup file(s): ` + + `${removed.map((p) => path.relative(repoRoot, p)).join(', ')}\n`, + ); + } + }, + }); + const handlers = registerCleanupHandlers({ + cleanup, + onSignal: (signal) => { + // Pass the signal to Stryker so it unwinds its own state, then let the + // `finally` below clean up once the child is gone. + if (!child || child.exitCode !== null || child.signalCode !== null) return false; + signalled = signal; + child.kill('SIGINT'); + return true; + }, + }); let strykerExitCode; try { strykerExitCode = await new Promise((resolve) => { @@ -538,14 +751,11 @@ async function runJob({ pkgRoot, packageDir, targets }, { configArg }) { child.on('error', (err) => die(3, `Stryker failed to start: ${err.message}`)); }); } finally { - process.off('SIGINT', onSignal); - process.off('SIGTERM', onSignal); - const restored = restoreFiles(snap); - if (restored.length > 0) { - process.stderr.write( - `⚠ Stryker left mutants behind; restored ${restored.length} file(s): ${restored.join(', ')}\n`, - ); - } + handlers.dispose(); + cleanup(); + // The run was cancelled. The tree is clean again, so leave with the shell's + // code for the signal instead of reporting on a run that never finished. + if (signalled) process.exit(signalled === 'SIGTERM' ? 143 : 130); } const strykerOutput = Buffer.concat(strykerOutputChunks).toString('utf8'); const target = mutateArg; diff --git a/scripts/mutation-health/mutate.test.mjs b/scripts/mutation-health/mutate.test.mjs index 2020dffb16e..e7b5746bf75 100644 --- a/scripts/mutation-health/mutate.test.mjs +++ b/scripts/mutation-health/mutate.test.mjs @@ -1,16 +1,28 @@ -import { describe, it } from 'node:test'; +import { afterEach, beforeEach, describe, it } from 'node:test'; import assert from 'node:assert/strict'; +import { spawnSync } from 'node:child_process'; +import { EventEmitter } from 'node:events'; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; import { buildNoTestsSummary, buildSummary, classifyRun, coverageFromCounts, + createCleanup, + filesToCheckout, + findStrykerSetupFiles, formatMutateArg, isMutableSource, mergeRanges, parseHunkRanges, + registerCleanupHandlers, + removeStrykerSetupFiles, + restoreFiles, scoreFromCounts, + snapshotFiles, splitRange, } from './mutate.mjs'; @@ -348,3 +360,296 @@ describe('splitRange', () => { assert.deepEqual(splitRange('src/a:b/cron.ts'), { file: 'src/a:b/cron.ts', range: null }); }); }); + +describe('filesToCheckout', () => { + it('picks the files the run made dirty', () => { + assert.deepEqual(filesToCheckout(['a.ts'], ['a.ts', 'b.ts']), ['b.ts']); + }); + + // A file that was dirty before the run holds the user's uncommitted work. + // `git checkout --` would throw it away; the byte snapshot restores it. + it('leaves the already-dirty files to the byte snapshot', () => { + assert.deepEqual(filesToCheckout(['a.ts', 'b.ts'], ['a.ts', 'b.ts']), []); + }); + + it('returns nothing when the run left the tree as it found it', () => { + assert.deepEqual(filesToCheckout([], []), []); + }); +}); + +describe('createCleanup', () => { + it('restores the tree and then removes the setup files', () => { + const calls = []; + const cleanup = createCleanup({ + restore: () => { + calls.push('restore'); + return ['src/cron.ts']; + }, + removeSetupFiles: () => { + calls.push('remove'); + return ['stryker-setup-0.js']; + }, + }); + assert.deepEqual(cleanup(), { restored: ['src/cron.ts'], removed: ['stryker-setup-0.js'] }); + assert.deepEqual(calls, ['restore', 'remove']); + }); + + // Four exit paths are wired to the same routine, and more than one can fire + // (SIGINT, then `exit`). Doing the work twice would undo a restore the user + // made in between. + it('does the work once however many exit paths call it', () => { + let restores = 0; + let removals = 0; + const cleanup = createCleanup({ + restore: () => { + restores++; + return []; + }, + removeSetupFiles: () => { + removals++; + return []; + }, + }); + cleanup(); + cleanup(); + cleanup(); + assert.equal(restores, 1); + assert.equal(removals, 1); + }); + + it('still removes the setup files when the restore throws', () => { + let removals = 0; + const cleanup = createCleanup({ + restore: () => { + throw new Error('working tree is locked'); + }, + removeSetupFiles: () => { + removals++; + return ['stryker-setup-0.js']; + }, + }); + assert.deepEqual(cleanup(), { restored: [], removed: ['stryker-setup-0.js'] }); + assert.equal(removals, 1); + }); +}); + +describe('registerCleanupHandlers', () => { + // A stand-in process, so the tests can fire the handlers without signalling + // or ending the test runner. + function harness({ onSignal } = {}) { + const proc = new EventEmitter(); + const exits = []; + const writes = []; + let cleaned = 0; + const handlers = registerCleanupHandlers({ + cleanup: () => { + cleaned++; + return { restored: [], removed: [] }; + }, + onSignal, + proc, + exit: (code) => exits.push(code), + write: (msg) => writes.push(msg), + }); + return { proc, exits, writes, handlers, cleaned: () => cleaned }; + } + + it('cleans up on the usual exit path', () => { + const h = harness(); + h.proc.emit('exit', 0); + assert.equal(h.cleaned(), 1); + }); + + it('cleans up and exits 130 on SIGINT', () => { + const h = harness({ onSignal: () => false }); + h.proc.emit('SIGINT'); + assert.equal(h.cleaned(), 1); + assert.deepEqual(h.exits, [130]); + }); + + it('cleans up and exits 143 on SIGTERM', () => { + const h = harness({ onSignal: () => false }); + h.proc.emit('SIGTERM'); + assert.equal(h.cleaned(), 1); + assert.deepEqual(h.exits, [143]); + }); + + // While Stryker is alive the run has to unwind first: a restore that races + // Stryker's own writes fixes nothing. The run's `finally` then cleans up. + it('defers to a live run, which still reaches cleanup', () => { + const seen = []; + const h = harness({ + onSignal: (signal) => { + seen.push(signal); + return true; + }, + }); + h.proc.emit('SIGINT'); + h.proc.emit('SIGTERM'); + assert.deepEqual(seen, ['SIGINT', 'SIGTERM']); + assert.equal(h.cleaned(), 0); + assert.deepEqual(h.exits, []); + + h.proc.emit('exit', 130); + assert.equal(h.cleaned(), 1); + }); + + it('cleans up on an uncaught exception and exits 3', () => { + const h = harness(); + h.proc.emit('uncaughtException', new Error('boom')); + assert.equal(h.cleaned(), 1); + assert.deepEqual(h.exits, [3]); + assert.match(h.writes.join(''), /boom/); + }); + + it('stops listening after dispose, so the next job owns its own snapshot', () => { + const h = harness({ onSignal: () => false }); + h.handlers.dispose(); + h.proc.emit('exit', 0); + h.proc.emit('SIGINT'); + h.proc.emit('SIGTERM'); + h.proc.emit('uncaughtException', new Error('boom')); + assert.equal(h.cleaned(), 0); + assert.deepEqual(h.exits, []); + }); +}); + +describe('working-tree cleanup', () => { + let root; + + beforeEach(() => { + root = mkdtempSync(path.join(tmpdir(), 'mutate-cleanup-')); + }); + + afterEach(() => { + rmSync(root, { recursive: true, force: true }); + }); + + // A mutate target plus two files outside it. The old target-only snapshot + // restored the target and left the other two mutated. + function seedTree() { + mkdirSync(path.join(root, 'packages/pkg/src'), { recursive: true }); + mkdirSync(path.join(root, 'packages/other/src'), { recursive: true }); + const files = { + target: path.join(root, 'packages/pkg/src/cron.ts'), + sibling: path.join(root, 'packages/pkg/src/helper.ts'), + test: path.join(root, 'packages/other/src/cron.test.ts'), + }; + for (const [name, file] of Object.entries(files)) writeFileSync(file, `original ${name}\n`); + return files; + } + + // A stand-in for Stryker: a real child process that rewrites each file it is + // given and drops a setup file for each worker, the way the vitest runner + // does. `--inPlace` means those writes hit the working tree. + function runMockStryker({ mutates = [], setupFiles = [] }) { + const script = [ + "const { writeFileSync } = require('node:fs');", + `for (const f of ${JSON.stringify(mutates)}) writeFileSync(f, 'Stryker was here!\\n');`, + `for (const f of ${JSON.stringify(setupFiles)}) writeFileSync(f, '// stryker setup\\n');`, + ].join('\n'); + const res = spawnSync(process.execPath, ['-e', script], { encoding: 'utf8' }); + assert.equal(res.status, 0, res.stderr); + } + + it('restores every file the run modified, not only the mutate targets', () => { + const files = seedTree(); + const snap = snapshotFiles(Object.values(files)); + + runMockStryker({ mutates: Object.values(files) }); + for (const file of Object.values(files)) { + assert.equal(readFileSync(file, 'utf8'), 'Stryker was here!\n'); + } + + assert.deepEqual(new Set(restoreFiles(snap)), new Set(Object.values(files))); + for (const [name, file] of Object.entries(files)) { + assert.equal(readFileSync(file, 'utf8'), `original ${name}\n`); + } + }); + + it('reports only the files it had to write back', () => { + const files = seedTree(); + const snap = snapshotFiles(Object.values(files)); + + runMockStryker({ mutates: [files.sibling] }); + + assert.deepEqual(restoreFiles(snap), [files.sibling]); + }); + + it('skips a file the run deleted instead of throwing', () => { + const files = seedTree(); + const snap = snapshotFiles(Object.values(files)); + rmSync(files.test); + + assert.deepEqual(restoreFiles(snap), []); + }); + + it('deletes every stryker-setup-*.js under the repo root', () => { + const files = seedTree(); + const setupFiles = [ + path.join(root, 'stryker-setup-0.js'), + path.join(root, 'packages/pkg/stryker-setup-1.js'), + path.join(root, 'packages/other/src/stryker-setup-12.js'), + ]; + runMockStryker({ setupFiles }); + + assert.deepEqual(new Set(removeStrykerSetupFiles(root)), new Set(setupFiles)); + for (const file of setupFiles) assert.equal(existsSync(file), false); + assert.deepEqual(findStrykerSetupFiles(root), []); + // The sources stay where they are. + assert.ok(existsSync(files.target)); + }); + + it('leaves files that only look like a setup file alone', () => { + seedTree(); + const keep = [ + path.join(root, 'stryker-setup.js'), + path.join(root, 'stryker-setup-0.ts'), + path.join(root, 'my-stryker-setup-0.js'), + ]; + runMockStryker({ setupFiles: keep }); + + assert.deepEqual(removeStrykerSetupFiles(root), []); + for (const file of keep) assert.ok(existsSync(file)); + }); + + // A full-repo walk that enters node_modules takes minutes, and Stryker's own + // packaged `stryker-setup.js` lives there. + it('does not walk node_modules', () => { + mkdirSync(path.join(root, 'node_modules/@stryker-mutator'), { recursive: true }); + const inside = path.join(root, 'node_modules/@stryker-mutator/stryker-setup-0.js'); + runMockStryker({ setupFiles: [inside] }); + + assert.deepEqual(removeStrykerSetupFiles(root), []); + assert.ok(existsSync(inside)); + }); + + it('leaves neither mutants nor setup files behind when a run is interrupted', () => { + const files = seedTree(); + const setupFiles = [path.join(root, 'packages/pkg/stryker-setup-0.js')]; + const snap = snapshotFiles(Object.values(files)); + + const cleanup = createCleanup({ + restore: () => restoreFiles(snap), + removeSetupFiles: () => removeStrykerSetupFiles(root), + }); + const proc = new EventEmitter(); + const exits = []; + registerCleanupHandlers({ + cleanup, + onSignal: () => false, // Stryker is already gone + proc, + exit: (code) => exits.push(code), + write: () => {}, + }); + + runMockStryker({ mutates: Object.values(files), setupFiles }); + proc.emit('SIGINT'); // the user hits Ctrl-C mid-run + + for (const [name, file] of Object.entries(files)) { + assert.equal(readFileSync(file, 'utf8'), `original ${name}\n`); + } + assert.deepEqual(findStrykerSetupFiles(root), []); + assert.deepEqual(exits, [130]); + }); +});