smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(launch): scope daemon prune to the daemons container

Browse Source 
smokeTestDaemonBroker mkdtemp'd its runtime dir directly under
os.tmpdir(), and the broker's startup sweep pruneDeadDaemonRuntimeDirs
reclaims path.dirname(runtimeDir). On a default session that is /tmp, so
--smoke-test recursively deleted every aged sibling with no live
broker/clients (tmux/ssh sockets, editor state, build trees) while still
exiting 0.

- client.ts: keep the smoke broker's project and runtime dirs under a
  single private mkdtemp parent the process owns.
- presence.ts: refuse any prune root that is not the daemons container
  and only prune entries named like a 16-hex daemon scope key, so a
  relocated runtime dir can never rm -rf unrelated neighbours.

Fixes #8721
This commit is contained in:
roboomp
2026-08-16 11:50:00 +00:00
parent 37eee71978
commit 74e73debaf
4 changed files with 63 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/CHANGELOG.md
+4
View File
@@ -2,6 +2,10 @@
## [Unreleased]
### Fixed
- Fixed `omp --smoke-test` recursively deleting unrelated directories in `os.tmpdir()` (tmux/ssh sockets, editor state, build trees). The smoke broker now keeps its runtime dir under a private parent, and the dead-scope reclaim refuses any root that is not the `daemons` container and only prunes entries named like a 16-hex daemon scope key ([#8721](https://github.com/can1357/oh-my-pi/issues/8721)).
## [17.3.5] - 2026-08-16
### Added
packages/coding-agent/src/launch/client.ts
+9 -4
View File
@@ -516,8 +516,14 @@ export async function closeDaemonClients(): Promise<void> {
/** Exercise worker-host broker startup and authenticated RPC for distribution smoke tests. */
export async function smokeTestDaemonBroker(): Promise<void> {
const projectDir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-daemon-smoke-project-"));
const runtimeDir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-daemon-smoke-run-"));
// Keep the broker's runtime dir under a private parent this process owns, so
// the broker's dead-scope sweep (pruneDeadDaemonRuntimeDirs, fired on startup)
// can only ever reclaim siblings inside it — never unrelated neighbours in
// os.tmpdir() such as tmux/ssh sockets or build trees (issue #8721).
const smokeRoot = await fs.mkdtemp(path.join(os.tmpdir(), "omp-daemon-smoke-"));
const projectDir = path.join(smokeRoot, "project");
const runtimeDir = path.join(smokeRoot, "run");
await fs.mkdir(projectDir, { recursive: true });
const client = await createDaemonBrokerClient(projectDir, { runtimeDir, idleGraceMs: 5_000 });
try {
const ping = await client.request({ op: "ping" });
@@ -525,7 +531,6 @@ export async function smokeTestDaemonBroker(): Promise<void> {
await client.request({ op: "shutdown" });
} finally {
client.close();
await fs.rm(projectDir, { recursive: true, force: true });
await fs.rm(runtimeDir, { recursive: true, force: true });
await fs.rm(smokeRoot, { recursive: true, force: true });
}
}
packages/coding-agent/src/launch/presence.ts
+19 -4
View File
@@ -6,7 +6,19 @@ import { daemonRuntimeDir } from "./paths";
const CLIENTS_DIR = "clients";
const BROKER_PID_FILE = "broker.pid";
const GLOBAL_DAEMON_DIR = "global";
/**
* Basename of the container holding per-project daemon scopes
* (`<state>/run/daemons`). {@link pruneDeadDaemonRuntimeDirs} refuses to sweep
* any other root so a runtime dir passed from outside the state tree cannot
* turn the reclaim into an rm -rf of unrelated neighbours (issue #8721).
*/
const DAEMONS_DIR = "daemons";
/**
* Name shape of a project daemon scope: the 16-hex wyhash of the project dir
* produced by `getDaemonRuntimeDir`. Only entries matching this are pruned,
* which excludes the machine-global `global` container and any foreign dir.
*/
const DAEMON_SCOPE_KEY = /^[0-9a-f]{16}$/;
/**
* Grace before a dead daemon runtime dir becomes prune-eligible. Guards against
* deleting a scope whose owning omp process is mid-startup (token written, broker
@@ -118,11 +130,14 @@ async function hasLiveDaemonBroker(runtimeDir: string): Promise<boolean> {
* Best-effort and non-throwing: a scope is deleted only when its `broker.pid`
* is absent/dead, no live client presence remains, and it has been untouched
* for {@link DAEMON_RUNTIME_STALE_GRACE_MS}. The caller's own `currentRuntimeDir`
* and the machine-global daemon container are always skipped.
* is always skipped, and the sweep runs only inside the {@link DAEMONS_DIR}
* container over entries named like a {@link DAEMON_SCOPE_KEY} — so a runtime
* dir relocated elsewhere (e.g. the smoke test under `os.tmpdir()`) never
* reclaims unrelated neighbours (issue #8721).
*/
export async function pruneDeadDaemonRuntimeDirs(currentRuntimeDir: string): Promise<void> {
const root = path.dirname(currentRuntimeDir);
if (path.basename(root) === GLOBAL_DAEMON_DIR) return;
if (path.basename(root) !== DAEMONS_DIR) return;
const current = path.resolve(currentRuntimeDir);
let entries: Dirent[];
try {
@@ -138,7 +153,7 @@ export async function pruneDeadDaemonRuntimeDirs(currentRuntimeDir: string): Pro
}
const now = Date.now();
for (const entry of entries) {
if (!entry.isDirectory() || entry.name === GLOBAL_DAEMON_DIR) continue;
if (!entry.isDirectory() || !DAEMON_SCOPE_KEY.test(entry.name)) continue;
const dir = path.join(root, entry.name);
if (path.resolve(dir) === current) continue;
try {
packages/coding-agent/test/launch/daemon-prune.test.ts
+31 -11
View File
@@ -41,11 +41,11 @@ describe("pruneDeadDaemonRuntimeDirs", () => {
const daemons = path.join(tempDir.path(), "run", "daemons");
await fs.mkdir(daemons, { recursive: true });
const current = await scope(daemons, "current000000000", { pid: "dead", stale: true });
await scope(daemons, "deadstale0000000", { pid: "dead", stale: true });
await scope(daemons, "livebroker000000", { pid: process.pid, stale: true });
await scope(daemons, "liveclient000000", { clients: [process.pid], stale: true });
await scope(daemons, "deadfresh0000000", { pid: "dead" });
const current = await scope(daemons, "aaaaaaaaaaaaaaaa", { pid: "dead", stale: true });
await scope(daemons, "bbbbbbbbbbbbbbbb", { pid: "dead", stale: true });
await scope(daemons, "cccccccccccccccc", { pid: process.pid, stale: true });
await scope(daemons, "dddddddddddddddd", { clients: [process.pid], stale: true });
await scope(daemons, "eeeeeeeeeeeeeeee", { pid: "dead" });
// Machine-global daemon container must never be swept as a project scope.
await fs.mkdir(path.join(daemons, "global", "some-service"), { recursive: true });
await fs.utimes(path.join(daemons, "global"), STALE, STALE);
@@ -53,12 +53,12 @@ describe("pruneDeadDaemonRuntimeDirs", () => {
await pruneDeadDaemonRuntimeDirs(current);
const remaining = new Set(await fs.readdir(daemons));
expect(remaining.has("deadstale0000000")).toBe(false); // pruned
expect(remaining.has("current000000000")).toBe(true); // never prunes itself
expect(remaining.has("livebroker000000")).toBe(true); // live broker
expect(remaining.has("liveclient000000")).toBe(true); // live client presence
expect(remaining.has("deadfresh0000000")).toBe(true); // within stale grace
expect(remaining.has("global")).toBe(true); // global container skipped
expect(remaining.has("bbbbbbbbbbbbbbbb")).toBe(false); // pruned
expect(remaining.has("aaaaaaaaaaaaaaaa")).toBe(true); // never prunes itself
expect(remaining.has("cccccccccccccccc")).toBe(true); // live broker
expect(remaining.has("dddddddddddddddd")).toBe(true); // live client presence
expect(remaining.has("eeeeeeeeeeeeeeee")).toBe(true); // within stale grace
expect(remaining.has("global")).toBe(true); // non-scope name skipped
});
it("does not sweep sibling machine-global service runtimes", async () => {
@@ -72,6 +72,26 @@ describe("pruneDeadDaemonRuntimeDirs", () => {
expect(await fs.exists(sibling)).toBe(true);
});
it("never sweeps outside the daemons container when a runtime dir is relocated (issue #8721)", async () => {
using tempDir = TempDir.createSync("@omp-daemon-prune-tmpdir-");
// Simulate the smoke test relocating its runtime dir directly under a
// shared temp root full of unrelated, aged directories.
const fakeTmp = tempDir.path();
for (const name of ["tmux-1000", "ssh-XVn1oP", "my-build-tree"]) {
await fs.mkdir(path.join(fakeTmp, name, "src"), { recursive: true });
await fs.utimes(path.join(fakeTmp, name), STALE, STALE);
}
const runtimeDir = path.join(fakeTmp, "omp-daemon-smoke-run-xxxx");
await fs.mkdir(runtimeDir, { recursive: true });
await pruneDeadDaemonRuntimeDirs(runtimeDir);
const remaining = new Set(await fs.readdir(fakeTmp));
expect(remaining.has("tmux-1000")).toBe(true);
expect(remaining.has("ssh-XVn1oP")).toBe(true);
expect(remaining.has("my-build-tree")).toBe(true);
});
it("does nothing when the runtime root does not exist", async () => {
using tempDir = TempDir.createSync("@omp-daemon-prune-missing-");
const current = path.join(tempDir.path(), "run", "daemons", "hash0000000000000");