diff --git a/cli/__tests__/adapters.claude.environment.test.mjs b/cli/__tests__/adapters.claude.environment.test.mjs index 4e5ac54435..4dd809aea8 100644 --- a/cli/__tests__/adapters.claude.environment.test.mjs +++ b/cli/__tests__/adapters.claude.environment.test.mjs @@ -1806,10 +1806,14 @@ describe('claude adapter: the public seat Commonly MCP witness (TASK-256)', () = })); fs.writeFileSync(path.join(pkgDir, 'src', 'index.js'), ''); fs.writeFileSync(path.join(home, 'current'), '0.3.13\n'); - // A fresh registry answer naming the installed build, so no turn here starts - // a background warm: a detached `npm install` outlives the suite and writes - // into TMPDIR after it ends (the CLI temp-cleanup guard caught it). fs.writeFileSync(path.join(home, '.registry.json'), JSON.stringify({ version: '0.3.13', checkedAt: Date.now() })); + // A held warm lock, so no turn here starts a background warm. A fresh + // registry answer isn't enough: a seat running a hand-run copy resolves no + // version from this home, and a spawn with no version always wants a warm. + // That warm is a detached `npm install` from the real registry. It outlived + // the suite, refilled this home after the teardown below removed it, and + // raced the CLI temp-cleanup guard's own rm (ENOTEMPTY, two dequeued PRs). + fs.writeFileSync(path.join(home, '.warm.lock'), 'warming\n'); prior = process.env.COMMONLY_MCP_HOME; process.env.COMMONLY_MCP_HOME = home; Object.defineProperty(process, 'platform', { value: 'darwin' }); diff --git a/cli/scripts/verify-temp-cleanup.mjs b/cli/scripts/verify-temp-cleanup.mjs index f34521788a..5440973883 100644 --- a/cli/scripts/verify-temp-cleanup.mjs +++ b/cli/scripts/verify-temp-cleanup.mjs @@ -2,10 +2,17 @@ /** * Run the CLI Jest files that create temp homes twice in an isolated TMPDIR and - * fail if any test-created temp directory survives either run. The full CLI + * fail if anything the tests created survives either run. The full CLI * suite runs in CI separately; this is a cheap regression guard for the * teardown contract. The default list is the files that leaked ~40 GB * (TASK-436); pass file paths to check others. + * + * Everything left in the probe root counts except the tools' own caches. It + * used to count a fixed list of prefixes, and a home under a new prefix + * (`cli-claude-mcp-home-`) leaked 29 MB unseen: a detached MCP warm that a test + * started kept writing into it after Jest exited, and the only symptom was this + * script's final rm failing with ENOTEMPTY on whichever PR was in the queue. + * That rm still fails the run, now naming what was still being written. */ import { spawn } from 'node:child_process'; import { lstat, mkdtemp, readdir, rm } from 'node:fs/promises'; @@ -20,13 +27,11 @@ const targets = process.argv.length > 2 '__tests__/adapters.codex.test.mjs', '__tests__/adapters.pi.test.mjs', ]; -const prefixes = [ - 'opencode-adapter-test-', - 'cli-mcp-home-', - 'cli-mcp-pin-', - 'cli-mcp-home-old-', - 'commonly-mcp-home-jest-', -]; +// Caches Jest and Node write to TMPDIR for themselves, not for a test. +const toolCaches = [/^jest_/, /^node-compile-cache$/]; +// A detached child a test started gets this long to show itself in the probe +// root before the root is read. +const SETTLE_MS = 1500; const bytesUnder = async (path) => { const info = await lstat(path); @@ -38,7 +43,7 @@ const bytesUnder = async (path) => { }; const snapshot = async (root) => { - const entries = (await readdir(root)).filter((name) => prefixes.some((prefix) => name.startsWith(prefix))); + const entries = (await readdir(root)).filter((name) => !toolCaches.some((cache) => cache.test(name))); let bytes = 0; for (const entry of entries) bytes += await bytesUnder(join(root, entry)); return { count: entries.length, bytes, entries }; @@ -71,6 +76,7 @@ try { for (let pass = 1; pass <= 2; pass += 1) { const before = await snapshot(probeRoot); await run(probeRoot); + await new Promise((resolve) => { setTimeout(resolve, SETTLE_MS); }); const after = await snapshot(probeRoot); console.log(`CLI temp cleanup pass ${pass}: before=${before.count} dirs/${before.bytes} bytes, after=${after.count} dirs/${after.bytes} bytes`); if (after.count !== 0 || after.bytes !== 0) { @@ -78,5 +84,18 @@ try { } } } finally { - await rm(probeRoot, { recursive: true, force: true }); + try { + await rm(probeRoot, { recursive: true, force: true, maxRetries: 5, retryDelay: 200 }); + } catch (error) { + // A root that won't go is a leak the snapshots missed. ENOTEMPTY after the + // retries means something kept writing after the last snapshot, the + // detached-child case above. exitCode, not a throw, so a leak check's own + // error stays the one reported when both happen. + const left = await readdir(probeRoot).catch(() => []); + const hint = error.code === 'ENOTEMPTY' + ? ' Something kept writing there after the last snapshot, usually a process a test started that outlived Jest.' + : ''; + console.error(`CLI temp cleanup could not remove ${probeRoot} (${error.code || error.message}); left in it: ${left.join(', ') || 'nothing readable'}.${hint}`); + process.exitCode = 1; + } }