From eadd10cad6ac9ec92d4d3187c7e94b42567a729d Mon Sep 17 00:00:00 2001 From: "clagentic-builder[bot]" Date: Fri, 23 Oct 2026 14:05:47 -0400 Subject: [PATCH 1/5] test(helpers): add a temp-home removal that waits for children and in-flight writes (lr-c3e80a) Co-Authored-By: Claude Sonnet 5.5 --- test/helpers/remove-temp-home.js | 161 +++++++++++++++++++++++++++++++ test/remove-temp-home.test.js | 137 ++++++++++++++++++++++++++ 2 files changed, 298 insertions(+) create mode 100644 test/helpers/remove-temp-home.js create mode 100644 test/remove-temp-home.test.js diff --git a/test/helpers/remove-temp-home.js b/test/helpers/remove-temp-home.js new file mode 100644 index 00000000..355ad6bc --- /dev/null +++ b/test/helpers/remove-temp-home.js @@ -0,0 +1,161 @@ +"use strict"; + +// The one way a test removes a temp home that a process or an in-process async +// writer may still be writing into. A bare synchronous rm races those writers: +// a file or directory created while the removal runs leaves the parent +// non-empty (ENOTEMPTY), and a write queued behind the removal recreates the +// tree afterwards. +// +// Order matters and is the fix, not a retry: (1) wait, bounded, for every +// child the test was told about to exit, failing with its pid when one does +// not; (2) let in-process promise-based fs work finish, so the event loop is +// free to run the chain that is still writing; (3) remove; (4) confirm the +// home stayed gone. The retry options only cover a transient directory +// listing race after (1) and (2). + +var fs = require("fs"); +var path = require("path"); +var { TOKEN_ENV, verifyOwnedPid } = require("../fixtures/process-ownership"); + +var REMOVE_OPTIONS = { recursive: true, force: true, maxRetries: 10, retryDelay: 100 }; +var DEFAULT_WAIT_MS = 10000; +var POLL_MS = 25; + +function sleep(ms) { + return new Promise(function (resolve) { setTimeout(resolve, ms); }); +} + +// A zombie has exited and only waits for its parent to reap it; it writes nothing. +function pidAlive(pid) { + try { process.kill(pid, 0); } catch (e) { return e.code === "EPERM"; } + try { + var stat = fs.readFileSync("/proc/" + pid + "/stat", "utf8"); + return stat.slice(stat.lastIndexOf(") ") + 2, stat.lastIndexOf(") ") + 3) !== "Z"; + } catch (e) { + return true; + } +} + +// Every live process that carries the harness token: the daemon's own children +// and grandchildren inherit its environment, and one outlives the daemon when +// the daemon exits first (it is re-parented, so no handle to it exists). +function ownedPids(token) { + var out = []; + var entries; + try { entries = fs.readdirSync("/proc"); } catch (e) { return out; } + var want = TOKEN_ENV + "=" + token; + entries.forEach(function (name) { + if (!/^\d+$/.test(name)) return; + var pid = Number(name); + if (pid === process.pid || !pidAlive(pid)) return; + if (verifyOwnedPid(pid, token).ok) out.push(pid); + }); + return out; +} + +function signalAll(pids, signal) { + pids.forEach(function (pid) { + try { process.kill(pid, signal); } catch (e) { if (e.code !== "ESRCH") throw e; } + }); +} + +// Stops what the harness started and the test holds no handle to. Only a +// process that carries this instance's token is signalled. Returns the pids +// that survived SIGKILL, so the caller fails loudly rather than skipping them. +async function stopOwned(token, ms) { + var pids = ownedPids(token); + if (pids.length === 0) return []; + signalAll(pids, "SIGTERM"); + var end = Date.now() + ms; + var killedAt = Date.now() + ms / 2; + var killed = false; + while (Date.now() < end) { + pids = pids.filter(pidAlive); + if (pids.length === 0) return []; + if (!killed && Date.now() >= killedAt) { signalAll(pids, "SIGKILL"); killed = true; } + await sleep(POLL_MS); + } + return pids.filter(pidAlive); +} + +// A child is a ChildProcess the test spawned, or a bare pid recorded elsewhere +// (a self-spawned successor). Resolves null once it has exited, or a message +// naming it when the bound expires. +function waitForChild(child, ms) { + if (typeof child === "number") { + return (async function () { + var end = Date.now() + ms; + while (Date.now() < end) { + if (!pidAlive(child)) return null; + await sleep(POLL_MS); + } + return pidAlive(child) ? "pid " + child + " was still running after " + ms + " ms" : null; + })(); + } + if (child.exitCode !== null || child.signalCode !== null) return Promise.resolve(null); + return new Promise(function (resolve) { + var timer = setTimeout(function () { + resolve("pid " + child.pid + " was still running after " + ms + " ms"); + }, ms); + child.once("exit", function () { clearTimeout(timer); resolve(null); }); + }); +} + +function fsPromiseRequestsInFlight() { + return process.getActiveResourcesInfo().indexOf("FSReqPromise") !== -1; +} + +// Every step of a promise-based fs chain (mkdir, then writeFile, then rename) +// is queued from a microtask after the previous step settles, so a check that +// runs after a macrotask turn sees the chain's next request if there is one. +async function drainPromiseFs(ms) { + var end = Date.now() + ms; + do { + await new Promise(function (resolve) { setImmediate(resolve); }); + if (!fsPromiseRequestsInFlight()) return null; + await sleep(POLL_MS); + } while (Date.now() < end); + return "promise-based fs requests were still in flight after " + ms + " ms"; +} + +/** + * @param {string} home the directory to remove + * @param {object} [o] + * @param {Array} [o.children] processes that may write into home + * @param {string} [o.token] the harness token the spawned processes carry; every live process with it is stopped (an orphaned grandchild included) + * @param {number} [o.waitMs] bound for each wait (default 10000) + * @throws when a child outlives the bound or the home comes back; the message names each pid + */ +async function removeTempHome(home, o) { + var opts = o || {}; + var waitMs = opts.waitMs || DEFAULT_WAIT_MS; + var problems = []; + + var verdicts = await Promise.all((opts.children || []).map(function (c) { return waitForChild(c, waitMs); })); + verdicts.forEach(function (v) { if (v) problems.push(v + " (it may still write into " + home + ")"); }); + + if (opts.token) { + var survivors = await stopOwned(opts.token, waitMs); + survivors.forEach(function (pid) { + problems.push("pid " + pid + " carrying the harness token survived SIGKILL (it may still write into " + home + ")"); + }); + } + + var drained = await drainPromiseFs(waitMs); + if (drained) problems.push(drained); + + // Bounded passes: a chain that began before the first pass and wrote after it + // shows up as the home existing again on the confirming check. + for (var pass = 0; pass < 3; pass++) { + await fs.promises.rm(home, REMOVE_OPTIONS); + await new Promise(function (resolve) { setImmediate(resolve); }); + if (!fsPromiseRequestsInFlight() && !fs.existsSync(home)) break; + var again = await drainPromiseFs(waitMs); + if (again) { problems.push(again); break; } + if (pass === 2 && fs.existsSync(home)) problems.push(path.basename(home) + " was recreated after removal"); + } + + if (problems.length > 0) throw new Error(problems.join("; ")); +} + +module.exports = { removeTempHome, pidAlive, REMOVE_OPTIONS }; diff --git a/test/remove-temp-home.test.js b/test/remove-temp-home.test.js new file mode 100644 index 00000000..f9c809dd --- /dev/null +++ b/test/remove-temp-home.test.js @@ -0,0 +1,137 @@ +"use strict"; +// The shared temp-home removal: a writer that is still running when a test +// thinks it is done must be waited for, never raced with, and a writer that +// does not stop must fail the test naming its pid. + +var test = require("node:test"); +var assert = require("node:assert/strict"); +var fs = require("fs"); +var os = require("os"); +var path = require("path"); +var { spawn } = require("child_process"); + +var { removeTempHome, pidAlive } = require("./helpers/remove-temp-home"); +var { TOKEN_ENV, newHarnessToken } = require("./fixtures/process-ownership"); + +// lib/store loads lib/config, which reads and may migrate files under the +// console home; keep that off the operator's real one. +var STORE_HOME = fs.mkdtempSync(path.join(os.tmpdir(), "console-remove-home-cfg-")); +process.env.CLAGENTIC_CONSOLE_HOME = STORE_HOME; +delete process.env.CLAGENTIC_HOME; +var { writeJsonAt } = require("../lib/store"); + +test.after(function () { fs.rmSync(STORE_HOME, { recursive: true, force: true }); }); + +var LATE_WRITER = [ + "var fs = require('fs'); var home = process.argv[1];", + "setTimeout(function () {", + " fs.mkdirSync(home + '/console/late', { recursive: true });", + " fs.writeFileSync(home + '/console/late/state.json', '{}');", + "}, 300);", +].join("\n"); + +function makeHome() { + var home = fs.mkdtempSync(path.join(os.tmpdir(), "console-remove-home-")); + fs.mkdirSync(path.join(home, "console"), { recursive: true }); + return home; +} + +function lateWriter(home) { + return spawn(process.execPath, ["-e", LATE_WRITER, home], { stdio: "ignore" }); +} + +test("a bare removal races a writer that finishes after it: the home comes back", async function () { + var home = makeHome(); + var child = lateWriter(home); + try { + fs.rmSync(home, { recursive: true, force: true, maxRetries: 10, retryDelay: 100 }); + assert.equal(fs.existsSync(home), false, "the bare removal itself succeeded"); + await new Promise(function (resolve) { child.once("exit", resolve); }); + assert.equal(fs.existsSync(home), true, "the late write recreated the removed home"); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } +}); + +test("removeTempHome waits for a child that writes late, then leaves the home gone", async function () { + var home = makeHome(); + var child = lateWriter(home); + await removeTempHome(home, { children: [child] }); + assert.equal(child.exitCode, 0, "the child finished its write before the removal"); + assert.equal(fs.existsSync(home), false); +}); + +test("removeTempHome waits for a bare pid to exit", async function () { + var home = makeHome(); + var child = lateWriter(home); + await removeTempHome(home, { children: [child.pid] }); + assert.equal(fs.existsSync(home), false); +}); + +test("removeTempHome fails naming the pid of a child that outlives the bound", async function () { + var home = makeHome(); + var child = spawn(process.execPath, ["-e", "setInterval(function () {}, 1000)"], { stdio: "ignore" }); + try { + await assert.rejects(removeTempHome(home, { children: [child], waitMs: 200 }), function (err) { + assert.ok(err.message.indexOf("pid " + child.pid) !== -1, "names the pid: " + err.message); + return true; + }); + } finally { + child.kill("SIGKILL"); + await new Promise(function (resolve) { child.once("exit", resolve); }); + fs.rmSync(home, { recursive: true, force: true }); + } +}); + +// A launcher that starts a detached writer and exits at once: the writer is +// re-parented, as the claude process a daemon started is when the daemon exits +// first, so the test has no handle to it. Only its environment ties it back. +var ORPHAN_LAUNCHER = [ + "var cp = require('child_process');", + "var writer = [", + " \"var fs = require('fs'); var home = process.argv[1];\",", + " \"setInterval(function () {\",", + " \" fs.mkdirSync(home + '/console', { recursive: true });\",", + " \" fs.writeFileSync(home + '/console/state.json', String(Date.now()));\",", + " \"}, 10);\",", + "].join('\\n');", + "var w = cp.spawn(process.execPath, ['-e', writer, process.argv[1]], { detached: true, stdio: 'ignore' });", + "w.unref();", + "process.stdout.write(String(w.pid));", +].join("\n"); + +test("removeTempHome stops an orphaned writer that carries the harness token", async function () { + var home = makeHome(); + var token = newHarnessToken(); + var env = Object.assign({}, process.env); + env[TOKEN_ENV] = token; + var launcher = spawn(process.execPath, ["-e", ORPHAN_LAUNCHER, home], { env: env, stdio: ["ignore", "pipe", "ignore"] }); + var out = ""; + launcher.stdout.on("data", function (c) { out += c; }); + await new Promise(function (resolve) { launcher.once("exit", resolve); }); + var orphan = Number(out); + assert.ok(orphan > 1, "the launcher reported the writer's pid: " + out); + try { + await new Promise(function (resolve) { setTimeout(resolve, 100); }); + fs.rmSync(home, { recursive: true, force: true, maxRetries: 10, retryDelay: 100 }); + } catch (e) { + assert.equal(e.code, "ENOTEMPTY"); + } + await new Promise(function (resolve) { setTimeout(resolve, 100); }); + assert.equal(fs.existsSync(home), true, "a bare removal loses to the live writer"); + + await removeTempHome(home, { token: token }); + assert.equal(pidAlive(orphan), false, "the orphaned writer was stopped"); + await new Promise(function (resolve) { setTimeout(resolve, 100); }); + assert.equal(fs.existsSync(home), false, "and nothing recreated the home"); +}); + +test("removeTempHome lets an in-flight store write finish instead of racing it", async function () { + var home = makeHome(); + var write = writeJsonAt(path.join(home, "console", "loops", "state.json"), { a: 1 }); + await removeTempHome(home); + await write; + // Give any chain step that escaped the wait a turn to recreate the home. + await new Promise(function (resolve) { setTimeout(resolve, 100); }); + assert.equal(fs.existsSync(home), false); +}); From 9a9cb9c8d373fb701804326c79a4cb9916b336c9 Mon Sep 17 00:00:00 2001 From: "clagentic-builder[bot]" Date: Fri, 23 Oct 2026 14:05:51 -0400 Subject: [PATCH 2/5] fix(test): route temp-home teardown through the shared helper to end ENOTEMPTY flakes (lr-c3e80a) Co-Authored-By: Claude Sonnet 5.5 --- test/access-harness.js | 20 +++++++----------- test/automation-sender.test.js | 7 +++++-- test/fixtures/daemon-harness.js | 14 +++++++++++-- test/message-sender-record.test.js | 12 ++++++----- test/shutdown-entry-points-lr-8f12ca.test.js | 22 ++++++++++++++++---- 5 files changed, 49 insertions(+), 26 deletions(-) diff --git a/test/access-harness.js b/test/access-harness.js index 448c9d67..bb40eb52 100644 --- a/test/access-harness.js +++ b/test/access-harness.js @@ -15,6 +15,7 @@ var path = require("path"); var crypto = require("crypto"); var http = require("http"); var WebSocket = require("ws"); +var { removeTempHome } = require("./helpers/remove-temp-home"); var COOKIE_NAME = "relay_auth_user"; var TOKEN_TTL_MS = 30 * 24 * 60 * 60 * 1000; @@ -202,19 +203,12 @@ async function start(spec) { } await new Promise(function (resolve) { relay.server.close(function () { resolve(); }); }); if (typeof relay.server.closeAllConnections === "function") relay.server.closeAllConnections(); - // An adapter may still be writing under HOME as the server shuts down: - // give those writes time to land, and retry the removal while one races it. - await settle(300); - var removed = false; - for (var attempt = 0; attempt < 5 && !removed; attempt++) { - try { - fs.rmSync(home, { recursive: true, force: true, maxRetries: 10, retryDelay: 100 }); - removed = true; - } catch (e) { - if (e.code !== "ENOTEMPTY" || attempt === 4) failures.push(e); - if (e.code !== "ENOTEMPTY") break; - await settle(300); - } + // An adapter may still be writing under HOME as the server shuts down: the + // helper lets its in-flight writes finish before it removes the home. + try { + await removeTempHome(home); + } catch (e) { + failures.push(e); } if (failures.length) throw failures[0]; } diff --git a/test/automation-sender.test.js b/test/automation-sender.test.js index bfdf9a1c..d4248b85 100644 --- a/test/automation-sender.test.js +++ b/test/automation-sender.test.js @@ -11,6 +11,7 @@ var os = require("os"); var path = require("path"); var execFileSync = require("child_process").execFileSync; +var { removeTempHome } = require("./helpers/remove-temp-home"); var turnInitiator = require("../lib/turn-initiator"); var sender = require("../lib/injected-sender"); var { RATE_LIMIT_RESUME_NOTE } = require("../lib/scheduled-input/rate-limit-resume"); @@ -308,8 +309,10 @@ test("a loop iteration is recorded as the loop, with its name and iteration, and armed.forEach(function (t) { clearTimeout(t); }); if (origHome === undefined) delete process.env.CLAGENTIC_CONSOLE_HOME; else process.env.CLAGENTIC_CONSOLE_HOME = origHome; - fs.rmSync(cwd, { recursive: true, force: true }); - fs.rmSync(home, { recursive: true, force: true }); + // The loop persists through lib/store in several async steps; the helper + // lets them finish before it removes either directory. + await removeTempHome(cwd); + await removeTempHome(home); } }); diff --git a/test/fixtures/daemon-harness.js b/test/fixtures/daemon-harness.js index f58e3c76..ade13e18 100644 --- a/test/fixtures/daemon-harness.js +++ b/test/fixtures/daemon-harness.js @@ -13,6 +13,7 @@ var http = require("http"); var { spawn } = require("child_process"); var WebSocket = require("ws"); var { TOKEN_ENV, newHarnessToken, verifyOwnedPid } = require("./process-ownership"); +var { removeTempHome } = require("../helpers/remove-temp-home"); var DAEMON_SCRIPT = path.resolve(__dirname, "..", "..", "lib", "daemon.js"); var SETUP_CODE = "HARNESSCODE"; @@ -61,7 +62,7 @@ async function waitForServer(port, ms) { function waitForExit(proc, ms) { return new Promise(function (resolve, reject) { - if (proc.exitCode !== null) return resolve(proc.exitCode); + if (proc.exitCode !== null || proc.signalCode !== null) return resolve(proc.exitCode); var timer = setTimeout(function () { reject(new Error("daemon did not exit within " + ms + " ms")); }, ms); proc.once("exit", function (code) { clearTimeout(timer); resolve(code); }); }); @@ -217,12 +218,21 @@ async function bootDaemon(o) { successor = null; } } + var children = [proc]; if (successor) { + children.push(successor); signalPid(successor, "SIGTERM"); await new Promise(function (r) { setTimeout(r, 1500); }); signalPid(successor, "SIGKILL"); } - fs.rmSync(home, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + // A claude process the daemon started can outlive it and keep writing + // under the home; the helper stops what carries the token and waits for + // every child before removing. + try { + await removeTempHome(home, { children: children, token: token }); + } catch (e) { + problems.push(e.message); + } if (problems.length > 0) throw new Error(problems.join("; ")); }, }; diff --git a/test/message-sender-record.test.js b/test/message-sender-record.test.js index a0a78ae1..67510b0e 100644 --- a/test/message-sender-record.test.js +++ b/test/message-sender-record.test.js @@ -14,6 +14,7 @@ var os = require("os"); var path = require("path"); var execFileSync = require("child_process").execFileSync; +var { removeTempHome } = require("./helpers/remove-temp-home"); var { INITIATORS } = require("../lib/turn-initiator"); var { RATE_LIMIT_RESUME_NOTE } = require("../lib/scheduled-input/rate-limit-resume"); var { createScheduledInputService } = require("../lib/scheduled-input/service"); @@ -110,13 +111,14 @@ function makeWorld(vendor, opts) { agentCatalog: function () { return []; }, }, extra || {})); }, - cleanup: function () { + cleanup: async function () { if (origHome === undefined) delete process.env.CLAGENTIC_CONSOLE_HOME; else process.env.CLAGENTIC_CONSOLE_HOME = origHome; if (origLegacyHome === undefined) delete process.env.CLAGENTIC_HOME; else process.env.CLAGENTIC_HOME = origLegacyHome; ["../lib/config", "../lib/sessions", "../lib/utils"].forEach(function (m) { delete require.cache[require.resolve(m)]; }); - // A loop saves its state asynchronously into this directory, so removal - // can meet a file written meanwhile: retry. - fs.rmSync(home, { recursive: true, force: true, maxRetries: 10, retryDelay: 50 }); + // A loop saves its state asynchronously into this directory (lib/store + // writes in several fs.promises steps); the helper lets those finish + // before removing, which a synchronous removal cannot. + await removeTempHome(home); }, }; return world; @@ -124,7 +126,7 @@ function makeWorld(vendor, opts) { async function inWorld(vendor, fn, opts) { var w = makeWorld(vendor, opts); - try { await fn(w); } finally { w.cleanup(); } + try { await fn(w); } finally { await w.cleanup(); } } // The browser composer's message handler over the real session manager. diff --git a/test/shutdown-entry-points-lr-8f12ca.test.js b/test/shutdown-entry-points-lr-8f12ca.test.js index c3419c27..6d8566dc 100644 --- a/test/shutdown-entry-points-lr-8f12ca.test.js +++ b/test/shutdown-entry-points-lr-8f12ca.test.js @@ -35,6 +35,7 @@ var { unsupervisedEnv } = require("../lib/supervisor"); var { bootDaemon: bootHarness } = require("./fixtures/daemon-harness"); var { TOKEN_ENV, newHarnessToken, verifyOwnedPid } = require("./fixtures/process-ownership"); var { makeInstall } = require("./fixtures/install-fixture"); +var { removeTempHome } = require("./helpers/remove-temp-home"); var DAEMON_SCRIPT = path.resolve(__dirname, "..", "lib", "daemon.js"); var SETUP_CODE = "SHUTDOWNENTRY"; @@ -84,7 +85,9 @@ async function waitForServer(port, ms) { function waitForExit(proc, ms) { return new Promise(function (resolve, reject) { - if (proc.exitCode !== null) return resolve(proc.exitCode); + // A process killed by a signal has exitCode null and signalCode set, and its + // "exit" event has already fired by the time a cleanup that awaited it returns. + if (proc.exitCode !== null || proc.signalCode !== null) return resolve(proc.exitCode); var timer = setTimeout(function () { reject(new Error("daemon did not exit within " + ms + " ms")); }, ms); proc.once("exit", function (code) { clearTimeout(timer); resolve(code); }); }); @@ -181,9 +184,11 @@ async function reapDaemon(proc, home, configPath, token) { // daemon.json is data the test does not control: signal only a pid that is // verifiably this test's daemon. var unverified = null; + var children = [proc]; if (successor) { var verdict = verifyOwnedPid(successor, token); if (verdict.ok) { + children.push(successor); signalPid(successor, "SIGTERM"); await new Promise(function (r) { setTimeout(r, 1500); }); signalPid(successor, "SIGKILL"); @@ -191,12 +196,21 @@ async function reapDaemon(proc, home, configPath, token) { unverified = "pid " + successor + " recorded in " + configPath + " was not signalled: " + verdict.reason; } } - fs.rmSync(home, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); - if (unverified) throw new Error(unverified); + // The daemon starts a claude process that can outlive it (re-parented, so + // proc has no handle to it) and keeps writing ~/.claude.json under the home; + // the helper stops what carries the token and waits for it before removing. + var problems = []; + try { + await removeTempHome(home, { children: children, token: token }); + } catch (e) { + problems.push(e.message); + } + if (unverified) problems.push(unverified); if (unreadable) { - throw new Error("could not establish the restart successor from " + configPath + problems.push("could not establish the restart successor from " + configPath + " (a detached daemon may still be running): " + unreadable.message); } + if (problems.length > 0) throw new Error(problems.join("; ")); } function sendWs(d, cookie, message) { From 40c64995c9c73eba69be46c17a7f210e13ecfefd Mon Sep 17 00:00:00 2001 From: "clagentic-builder[bot]" Date: Fri, 23 Oct 2026 14:31:28 -0400 Subject: [PATCH 3/5] fix(test): add a quiesce step, pid re-verification and a no-proc path to the temp-home helper (lr-c3e80a) Co-Authored-By: Claude Sonnet 5.5 --- test/helpers/remove-temp-home.js | 94 ++++++++++++++++++++++++-------- test/remove-temp-home.test.js | 93 ++++++++++++++++++++++++++++++- 2 files changed, 162 insertions(+), 25 deletions(-) diff --git a/test/helpers/remove-temp-home.js b/test/helpers/remove-temp-home.js index 355ad6bc..e8dbb135 100644 --- a/test/helpers/remove-temp-home.js +++ b/test/helpers/remove-temp-home.js @@ -6,20 +6,28 @@ // non-empty (ENOTEMPTY), and a write queued behind the removal recreates the // tree afterwards. // -// Order matters and is the fix, not a retry: (1) wait, bounded, for every -// child the test was told about to exit, failing with its pid when one does -// not; (2) let in-process promise-based fs work finish, so the event loop is -// free to run the chain that is still writing; (3) remove; (4) confirm the -// home stayed gone. The retry options only cover a transient directory -// listing race after (1) and (2). +// Order matters and is the fix, not a retry: (0) run the caller's quiesce step, +// which stops in-process writers that schedule work on a timer; (1) wait, +// bounded, for every child the test was told about to exit, failing with its +// pid when one does not; (2) let in-process promise-based fs work finish, so +// the event loop is free to run the chain that is still writing; (3) remove; +// (4) confirm the home stayed gone. The retry options only cover a transient +// directory listing race after (1) and (2). +// +// Limit: the drain in (2) sees fs requests that already exist. It cannot see a +// timer or debounce that will start a write later, so a writer that schedules +// on a timer must be stopped by the caller through its own close, flush or stop +// API, passed as `quiesce`. var fs = require("fs"); var path = require("path"); -var { TOKEN_ENV, verifyOwnedPid } = require("../fixtures/process-ownership"); +var util = require("util"); +var { verifyOwnedPid } = require("../fixtures/process-ownership"); var REMOVE_OPTIONS = { recursive: true, force: true, maxRetries: 10, retryDelay: 100 }; var DEFAULT_WAIT_MS = 10000; var POLL_MS = 25; +var debugLog = util.debuglog("remove-temp-home"); function sleep(ms) { return new Promise(function (resolve) { setTimeout(resolve, ms); }); @@ -39,18 +47,37 @@ function pidAlive(pid) { // Every live process that carries the harness token: the daemon's own children // and grandchildren inherit its environment, and one outlives the daemon when // the daemon exits first (it is re-parented, so no handle to it exists). -function ownedPids(token) { +// scanned:false means /proc cannot be listed (macOS), so nothing was checked. +function ownedProcesses(token) { var out = []; var entries; - try { entries = fs.readdirSync("/proc"); } catch (e) { return out; } - var want = TOKEN_ENV + "=" + token; + try { entries = fs.readdirSync("/proc"); } catch (e) { return { scanned: false, procs: out }; } entries.forEach(function (name) { if (!/^\d+$/.test(name)) return; var pid = Number(name); if (pid === process.pid || !pidAlive(pid)) return; - if (verifyOwnedPid(pid, token).ok) out.push(pid); + if (!verifyOwnedPid(pid, token).ok) return; + out.push({ pid: pid, start: startTimeOf(pid) }); }); - return out; + return { scanned: true, procs: out }; +} + +// Field 22 of /proc//stat, the process start time in clock ticks since +// boot: with the pid it identifies one process, so a recycled pid differs. +function startTimeOf(pid) { + try { + var stat = fs.readFileSync("/proc/" + pid + "/stat", "utf8"); + return stat.slice(stat.lastIndexOf(") ") + 2).split(" ")[19] || null; + } catch (e) { + return null; + } +} + +// Still the process that was found: alive, started at the same time, and still +// carrying the token. A pid recycled for an unrelated process fails this. +function stillOwned(proc, token) { + return pidAlive(proc.pid) && proc.start !== null && + startTimeOf(proc.pid) === proc.start && verifyOwnedPid(proc.pid, token).ok; } function signalAll(pids, signal) { @@ -60,22 +87,30 @@ function signalAll(pids, signal) { } // Stops what the harness started and the test holds no handle to. Only a -// process that carries this instance's token is signalled. Returns the pids -// that survived SIGKILL, so the caller fails loudly rather than skipping them. -async function stopOwned(token, ms) { - var pids = ownedPids(token); - if (pids.length === 0) return []; - signalAll(pids, "SIGTERM"); +// process that carries this instance's token is signalled, and it is checked +// again right before SIGKILL. Returns the pids that survived, so the caller +// fails loudly rather than skipping them. +async function stopOwned(token, ms, log) { + var found = ownedProcesses(token); + if (!found.scanned) { + log("token scan skipped: /proc cannot be listed on this platform"); + return []; + } + var procs = found.procs; + if (procs.length === 0) return []; + function owned(p) { return stillOwned(p, token); } + function pidOf(p) { return p.pid; } + signalAll(procs.map(pidOf), "SIGTERM"); var end = Date.now() + ms; var killedAt = Date.now() + ms / 2; var killed = false; while (Date.now() < end) { - pids = pids.filter(pidAlive); - if (pids.length === 0) return []; - if (!killed && Date.now() >= killedAt) { signalAll(pids, "SIGKILL"); killed = true; } + procs = procs.filter(owned); + if (procs.length === 0) return []; + if (!killed && Date.now() >= killedAt) { signalAll(procs.map(pidOf), "SIGKILL"); killed = true; } await sleep(POLL_MS); } - return pids.filter(pidAlive); + return procs.filter(owned).map(pidOf); } // A child is a ChildProcess the test spawned, or a bare pid recorded elsewhere @@ -124,18 +159,29 @@ async function drainPromiseFs(ms) { * @param {Array} [o.children] processes that may write into home * @param {string} [o.token] the harness token the spawned processes carry; every live process with it is stopped (an orphaned grandchild included) * @param {number} [o.waitMs] bound for each wait (default 10000) + * @param {function(): (void|Promise)} [o.quiesce] stops in-process writers + * that schedule a write on a timer or debounce, through the component's own + * close, flush or stop API. Runs first. The helper cannot detect such a + * writer: a timer that has not fired yet has no fs request to wait for. + * @param {function(string): void} [o.log] receives the one-line notice that the + * token scan was skipped (default: util.debuglog("remove-temp-home")) * @throws when a child outlives the bound or the home comes back; the message names each pid */ async function removeTempHome(home, o) { var opts = o || {}; var waitMs = opts.waitMs || DEFAULT_WAIT_MS; var problems = []; + var log = opts.log || debugLog; + + if (opts.quiesce) { + try { await opts.quiesce(); } catch (e) { problems.push("quiesce failed: " + (e && e.message ? e.message : e)); } + } var verdicts = await Promise.all((opts.children || []).map(function (c) { return waitForChild(c, waitMs); })); verdicts.forEach(function (v) { if (v) problems.push(v + " (it may still write into " + home + ")"); }); if (opts.token) { - var survivors = await stopOwned(opts.token, waitMs); + var survivors = await stopOwned(opts.token, waitMs, log); survivors.forEach(function (pid) { problems.push("pid " + pid + " carrying the harness token survived SIGKILL (it may still write into " + home + ")"); }); @@ -158,4 +204,4 @@ async function removeTempHome(home, o) { if (problems.length > 0) throw new Error(problems.join("; ")); } -module.exports = { removeTempHome, pidAlive, REMOVE_OPTIONS }; +module.exports = { removeTempHome, pidAlive, startTimeOf, stillOwned, REMOVE_OPTIONS }; diff --git a/test/remove-temp-home.test.js b/test/remove-temp-home.test.js index f9c809dd..6645bb6e 100644 --- a/test/remove-temp-home.test.js +++ b/test/remove-temp-home.test.js @@ -10,7 +10,7 @@ var os = require("os"); var path = require("path"); var { spawn } = require("child_process"); -var { removeTempHome, pidAlive } = require("./helpers/remove-temp-home"); +var { removeTempHome, pidAlive, startTimeOf, stillOwned } = require("./helpers/remove-temp-home"); var { TOKEN_ENV, newHarnessToken } = require("./fixtures/process-ownership"); // lib/store loads lib/config, which reads and may migrate files under the @@ -135,3 +135,94 @@ test("removeTempHome lets an in-flight store write finish instead of racing it", await new Promise(function (resolve) { setTimeout(resolve, 100); }); assert.equal(fs.existsSync(home), false); }); + +// --- writers that start their write from a timer --- + +// The drain sees fs requests that exist; a timer that has not fired has none. +function timerWriter(home, delayMs) { + var timer = setTimeout(function () { + writeJsonAt(path.join(home, "console", "late.json"), { a: 1 }).catch(function () {}); + }, delayMs); + return { cancel: function () { clearTimeout(timer); } }; +} + +test("removeTempHome cannot see a timer-scheduled write: the caller must quiesce it (the contract's limit)", async function () { + var home = makeHome(); + var writer = timerWriter(home, 150); + try { + await removeTempHome(home); + assert.equal(fs.existsSync(home), false, "gone when the helper returns"); + await new Promise(function (resolve) { setTimeout(resolve, 400); }); + assert.equal(fs.existsSync(home), true, "the unquiesced timer wrote after the helper returned"); + } finally { + writer.cancel(); + fs.rmSync(home, { recursive: true, force: true }); + } +}); + +test("removeTempHome runs the caller's quiesce first, so a timer-scheduled write never recreates the home", async function () { + var home = makeHome(); + var writer = timerWriter(home, 150); + var order = []; + await removeTempHome(home, { + quiesce: function () { order.push("quiesce"); writer.cancel(); }, + }); + order.push("returned"); + await new Promise(function (resolve) { setTimeout(resolve, 400); }); + assert.deepEqual(order, ["quiesce", "returned"]); + assert.equal(fs.existsSync(home), false, "nothing recreated the home"); +}); + +test("removeTempHome awaits an async quiesce and reports one that throws", async function () { + var home = makeHome(); + var done = false; + await removeTempHome(home, { + quiesce: async function () { await new Promise(function (resolve) { setTimeout(resolve, 50); }); done = true; }, + }); + assert.equal(done, true); + var home2 = makeHome(); + await assert.rejects(removeTempHome(home2, { quiesce: function () { throw new Error("close failed"); } }), /quiesce failed: close failed/); + assert.equal(fs.existsSync(home2), false, "the home is still removed"); +}); + +// --- a recycled pid is never signalled --- + +test("stillOwned accepts the process it found and refuses a pid whose start time differs", async function () { + var token = newHarnessToken(); + var env = Object.assign({}, process.env); + env[TOKEN_ENV] = token; + var child = spawn(process.execPath, ["-e", "setInterval(function () {}, 1000)"], { env: env, stdio: "ignore" }); + try { + var start = startTimeOf(child.pid); + assert.ok(start, "the start time is readable from /proc"); + assert.equal(stillOwned({ pid: child.pid, start: start }, token), true); + assert.equal(stillOwned({ pid: child.pid, start: String(Number(start) + 1) }, token), false, "a different start time is another process"); + assert.equal(stillOwned({ pid: child.pid, start: null }, token), false, "an unverified start time is not owned"); + assert.equal(stillOwned({ pid: child.pid, start: start }, newHarnessToken()), false, "another token is not owned"); + } finally { + child.kill("SIGKILL"); + await new Promise(function (resolve) { child.once("exit", resolve); }); + } +}); + +// --- no /proc --- + +test("without /proc the helper still waits for listed children, skips the token scan and says so once", async function () { + var home = makeHome(); + var child = lateWriter(home); + var realReaddir = fs.readdirSync; + var notices = []; + fs.readdirSync = function (dir) { + if (dir === "/proc") { var e = new Error("ENOENT: no such file or directory, scandir '/proc'"); e.code = "ENOENT"; throw e; } + return realReaddir.apply(this, arguments); + }; + try { + await removeTempHome(home, { children: [child], token: newHarnessToken(), log: function (m) { notices.push(m); } }); + } finally { + fs.readdirSync = realReaddir; + } + assert.equal(child.exitCode, 0, "the listed child was still waited for"); + assert.equal(fs.existsSync(home), false); + assert.equal(notices.length, 1, "one notice: " + JSON.stringify(notices)); + assert.match(notices[0], /token scan skipped/); +}); From db4d353700b9f3b966177b571a1e722d75acf48a Mon Sep 17 00:00:00 2001 From: "clagentic-builder[bot]" Date: Fri, 23 Oct 2026 14:31:37 -0400 Subject: [PATCH 4/5] fix(test): stop timer-scheduled writers through their own API before temp-home removal (lr-c3e80a) Co-Authored-By: Claude Sonnet 5.5 --- test/automation-sender.test.js | 11 ++++++----- test/message-sender-record.test.js | 14 ++++++++++++-- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/test/automation-sender.test.js b/test/automation-sender.test.js index d4248b85..99250aee 100644 --- a/test/automation-sender.test.js +++ b/test/automation-sender.test.js @@ -306,13 +306,14 @@ test("a loop iteration is recorded as the loop, with its name and iteration, and assert.deepEqual(turnInitiator.claudeOrigin(entry.initiator), { kind: "task-notification", subkind: "scheduled-trigger", fireReason: "loop" }); } finally { global.setTimeout = origSetTimeout; - armed.forEach(function (t) { clearTimeout(t); }); if (origHome === undefined) delete process.env.CLAGENTIC_CONSOLE_HOME; else process.env.CLAGENTIC_CONSOLE_HOME = origHome; - // The loop persists through lib/store in several async steps; the helper - // lets them finish before it removes either directory. - await removeTempHome(cwd); - await removeTempHome(home); + // The loop persists through lib/store in several async steps, which the + // helper lets finish. Its retry and watchdog timers (lib/project-loop) can + // still start a save later, so every timer it armed is cancelled first. + var cancelLoopTimers = function () { armed.forEach(function (t) { clearTimeout(t); }); }; + await removeTempHome(cwd, { quiesce: cancelLoopTimers }); + await removeTempHome(home, { quiesce: cancelLoopTimers }); } }); diff --git a/test/message-sender-record.test.js b/test/message-sender-record.test.js index 67510b0e..5cbfd2de 100644 --- a/test/message-sender-record.test.js +++ b/test/message-sender-record.test.js @@ -97,6 +97,7 @@ function makeWorld(vendor, opts) { }); var world = { vendor: vendor, home: home, recordsDir: recordsDir, sm: sm, sdk: sdk, calls: calls, project: project, processor: processor, + services: [], newSession: function (extra) { var s = sm.createSessionRaw({ vendor: vendor }); return Object.assign(s, extra || {}); @@ -117,8 +118,16 @@ function makeWorld(vendor, opts) { ["../lib/config", "../lib/sessions", "../lib/utils"].forEach(function (m) { delete require.cache[require.resolve(m)]; }); // A loop saves its state asynchronously into this directory (lib/store // writes in several fs.promises steps); the helper lets those finish - // before removing, which a synchronous removal cannot. - await removeTempHome(home); + // before removing, which a synchronous removal cannot. Two writers start + // work from a timer instead: the session manager batches appends behind a + // 50 ms timer, and the scheduled-input service re-arms its own. Each is + // stopped through its existing API before the helper drains. + await removeTempHome(home, { + quiesce: function () { + world.services.forEach(function (service) { service.stop(); }); + sm.sessions.forEach(function (session) { sm.flushSessionBuffer(session); }); + }, + }); }, }; return world; @@ -167,6 +176,7 @@ function makeScheduledInputService(w) { sessionInput: w.sessionInput(), }); service.attachProject("p"); + w.services.push(service); return service; } From 8f8d0d572a88d508a87fcbf2fd118db112d4d4af Mon Sep 17 00:00:00 2001 From: "clagentic-builder[bot]" Date: Fri, 23 Oct 2026 15:03:11 -0400 Subject: [PATCH 5/5] test(remove-temp-home): skip /proc-dependent tests without /proc and drive the pre-SIGKILL identity guard (lr-c3e80a) Co-Authored-By: Claude Sonnet 5.5 --- test/helpers/remove-temp-home.js | 15 ++++++----- test/remove-temp-home.test.js | 46 +++++++++++++++++++++++++++++--- 2 files changed, 52 insertions(+), 9 deletions(-) diff --git a/test/helpers/remove-temp-home.js b/test/helpers/remove-temp-home.js index e8dbb135..02470f95 100644 --- a/test/helpers/remove-temp-home.js +++ b/test/helpers/remove-temp-home.js @@ -90,24 +90,27 @@ function signalAll(pids, signal) { // process that carries this instance's token is signalled, and it is checked // again right before SIGKILL. Returns the pids that survived, so the caller // fails loudly rather than skipping them. -async function stopOwned(token, ms, log) { - var found = ownedProcesses(token); +async function stopOwned(token, ms, log, io) { + // `io` lets a test observe the signals and fake the identity check; callers + // outside this file's tests pass nothing. + var deps = Object.assign({ scan: ownedProcesses, owned: stillOwned, signal: signalAll }, io); + var found = deps.scan(token); if (!found.scanned) { log("token scan skipped: /proc cannot be listed on this platform"); return []; } var procs = found.procs; if (procs.length === 0) return []; - function owned(p) { return stillOwned(p, token); } + function owned(p) { return deps.owned(p, token); } function pidOf(p) { return p.pid; } - signalAll(procs.map(pidOf), "SIGTERM"); + deps.signal(procs.map(pidOf), "SIGTERM"); var end = Date.now() + ms; var killedAt = Date.now() + ms / 2; var killed = false; while (Date.now() < end) { procs = procs.filter(owned); if (procs.length === 0) return []; - if (!killed && Date.now() >= killedAt) { signalAll(procs.map(pidOf), "SIGKILL"); killed = true; } + if (!killed && Date.now() >= killedAt) { deps.signal(procs.map(pidOf), "SIGKILL"); killed = true; } await sleep(POLL_MS); } return procs.filter(owned).map(pidOf); @@ -204,4 +207,4 @@ async function removeTempHome(home, o) { if (problems.length > 0) throw new Error(problems.join("; ")); } -module.exports = { removeTempHome, pidAlive, startTimeOf, stillOwned, REMOVE_OPTIONS }; +module.exports = { removeTempHome, pidAlive, startTimeOf, stillOwned, stopOwned, REMOVE_OPTIONS }; diff --git a/test/remove-temp-home.test.js b/test/remove-temp-home.test.js index 6645bb6e..c0316ed5 100644 --- a/test/remove-temp-home.test.js +++ b/test/remove-temp-home.test.js @@ -10,9 +10,21 @@ var os = require("os"); var path = require("path"); var { spawn } = require("child_process"); -var { removeTempHome, pidAlive, startTimeOf, stillOwned } = require("./helpers/remove-temp-home"); +var { removeTempHome, pidAlive, startTimeOf, stillOwned, stopOwned } = require("./helpers/remove-temp-home"); var { TOKEN_ENV, newHarnessToken } = require("./fixtures/process-ownership"); +// Decided once at load: tests that read another process's /proc entries cannot +// run where /proc is absent or unreadable (macOS). +var NO_PROC = (function () { + try { + fs.readdirSync("/proc"); + fs.readFileSync("/proc/self/environ"); + return false; + } catch (e) { + return "/proc is not readable on this platform"; + } +})(); + // lib/store loads lib/config, which reads and may migrate files under the // console home; keep that off the operator's real one. var STORE_HOME = fs.mkdtempSync(path.join(os.tmpdir(), "console-remove-home-cfg-")); @@ -100,7 +112,7 @@ var ORPHAN_LAUNCHER = [ "process.stdout.write(String(w.pid));", ].join("\n"); -test("removeTempHome stops an orphaned writer that carries the harness token", async function () { +test("removeTempHome stops an orphaned writer that carries the harness token", { skip: NO_PROC }, async function () { var home = makeHome(); var token = newHarnessToken(); var env = Object.assign({}, process.env); @@ -187,7 +199,7 @@ test("removeTempHome awaits an async quiesce and reports one that throws", async // --- a recycled pid is never signalled --- -test("stillOwned accepts the process it found and refuses a pid whose start time differs", async function () { +test("stillOwned accepts the process it found and refuses a pid whose start time differs", { skip: NO_PROC }, async function () { var token = newHarnessToken(); var env = Object.assign({}, process.env); env[TOKEN_ENV] = token; @@ -205,6 +217,34 @@ test("stillOwned accepts the process it found and refuses a pid whose start time } }); +// The stop path itself, with the scan, identity check and signals faked so no +// real process is touched: a pid whose identity fails between SIGTERM and the +// SIGKILL step must never receive the SIGKILL. +function fakeStop(ownedFn) { + var signals = []; + var t0 = Date.now(); + var io = { + scan: function () { return { scanned: true, procs: [{ pid: 4242, start: "1" }] }; }, + owned: function () { return ownedFn(Date.now() - t0); }, + signal: function (pids, sig) { signals.push(sig + ":" + pids.join(",")); }, + }; + return { io: io, signals: signals }; +} + +test("stopOwned sends no SIGKILL to a pid that stopped being the owned process after SIGTERM", async function () { + var fake = fakeStop(function (elapsed) { return elapsed < 30; }); + var survivors = await stopOwned("tok", 200, function () {}, fake.io); + assert.deepEqual(survivors, []); + assert.deepEqual(fake.signals, ["SIGTERM:4242"], "only SIGTERM; the recycled pid was not killed"); +}); + +test("stopOwned escalates to SIGKILL for a pid that is still the owned process, and reports it if it survives", async function () { + var fake = fakeStop(function () { return true; }); + var survivors = await stopOwned("tok", 100, function () {}, fake.io); + assert.deepEqual(survivors, [4242]); + assert.deepEqual(fake.signals, ["SIGTERM:4242", "SIGKILL:4242"]); +}); + // --- no /proc --- test("without /proc the helper still waits for listed children, skips the token scan and says so once", async function () {