test(launch): move broker idle-shutdown regression into collected test tree
The coding-agent CI runner collects test files only from packages/coding-agent/test (scripts/ci-test-ts.ts), so the regression at src/launch/broker.test.ts was skipped by bun run test and CI. Relocated to test/launch/broker-idle-shutdown.test.ts and rewrote it to the in-process broker pattern used by the sibling broker tests: it awaits the broker's own run() promise (resolves on idle shutdown) instead of watching the PID file, so shutdown is an awaited signal rather than a poll. Fixes #8110
This commit is contained in:
roboomp
@@ -1,95 +0,0 @@
|
||||
import { afterEach, expect, test } from "bun:test";
|
||||
import * as fs from "node:fs/promises";
|
||||
import * as os from "node:os";
|
||||
import * as path from "node:path";
|
||||
import { createDaemonBrokerClient, type DaemonBrokerClient } from "./client";
|
||||
|
||||
// Broker PID lease file (internal to broker.ts); its removal is the observable
|
||||
// signal that the broker process ran its shutdown path and released the lease.
|
||||
const PID_FILE = "broker.pid";
|
||||
|
||||
const tempDirs: string[] = [];
|
||||
let openClient: DaemonBrokerClient | undefined;
|
||||
|
||||
afterEach(async () => {
|
||||
openClient?.close();
|
||||
openClient = undefined;
|
||||
// A regression leaves the broker running; kill it so it does not outlive the
|
||||
// suite and hold the temp runtime dir.
|
||||
for (const dir of tempDirs) {
|
||||
try {
|
||||
const raw: unknown = await Bun.file(path.join(dir, PID_FILE)).json();
|
||||
if (typeof raw === "object" && raw !== null && "pid" in raw && typeof raw.pid === "number") {
|
||||
try {
|
||||
process.kill(raw.pid, "SIGKILL");
|
||||
} catch {
|
||||
// Already gone — the expected outcome.
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
// No PID file — broker already exited.
|
||||
}
|
||||
}
|
||||
await Promise.all(tempDirs.map(dir => fs.rm(dir, { recursive: true, force: true })));
|
||||
tempDirs.length = 0;
|
||||
});
|
||||
|
||||
test("broker shuts down after its last persistent daemon exits with no clients", async () => {
|
||||
const projectDir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-broker-idle-project-"));
|
||||
const runtimeDir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-broker-idle-run-"));
|
||||
tempDirs.push(projectDir, runtimeDir);
|
||||
|
||||
const client = await createDaemonBrokerClient(projectDir, { runtimeDir, idleGraceMs: 200 });
|
||||
openClient = client;
|
||||
|
||||
// A persistent daemon that outlives the first idle-shutdown timer (200ms) and
|
||||
// then self-exits (~700ms). The first idle check must observe it live and
|
||||
// return; the broker must rearm idle shutdown once it settles.
|
||||
const started = await client.request({
|
||||
op: "start",
|
||||
spec: {
|
||||
name: "persistent-temp",
|
||||
application: process.execPath,
|
||||
args: ["-e", "setTimeout(() => {}, 700)"],
|
||||
env: {},
|
||||
cwd: projectDir,
|
||||
pty: false,
|
||||
restart: "no",
|
||||
persist: true,
|
||||
detached: false,
|
||||
},
|
||||
});
|
||||
expect(started.op).toBe("start");
|
||||
|
||||
const pidPath = path.join(runtimeDir, PID_FILE);
|
||||
expect(await Bun.file(pidPath).exists()).toBe(true);
|
||||
|
||||
// Disconnect the final client. The broker keeps the persistent daemon alive.
|
||||
client.close();
|
||||
openClient = undefined;
|
||||
|
||||
// After the daemon self-exits, terminal settlement must rearm idle shutdown so
|
||||
// the broker releases its lease (removes the PID file). Await that filesystem
|
||||
// signal directly rather than polling. Integration test: the broker's idle
|
||||
// timer runs in a separate process against the real clock, so fake timers
|
||||
// cannot drive it — the removal event is the observable shutdown proof.
|
||||
let brokerExited = !(await Bun.file(pidPath).exists());
|
||||
if (!brokerExited) {
|
||||
const watcher = fs.watch(runtimeDir, { signal: AbortSignal.timeout(15_000) });
|
||||
try {
|
||||
for await (const event of watcher) {
|
||||
if (event.filename === PID_FILE && !(await Bun.file(pidPath).exists())) {
|
||||
brokerExited = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
} catch (error) {
|
||||
// AbortSignal.timeout aborts the watch with AbortError/TimeoutError once
|
||||
// the window elapses; that just means the broker never exited, so let the
|
||||
// assertion below report it. Rethrow anything else.
|
||||
const name = error instanceof Error ? error.name : "";
|
||||
if (name !== "AbortError" && name !== "TimeoutError") throw error;
|
||||
}
|
||||
}
|
||||
expect(brokerExited).toBe(true);
|
||||
}, 20_000);
|
||||
@@ -0,0 +1,77 @@
|
||||
// Integration test — real timers are required (ts-no-test-timers exception): this drives the actual
|
||||
// cross-process daemon broker running a real child process, and the bug is a missing idle-shutdown
|
||||
// rearm in #settle. Fake timers cannot control the OS process-exit promise or the unix-socket RPC,
|
||||
// and shutdown is observed by awaiting the broker's own run() promise — its resolution IS the signal
|
||||
// (no polling, no fixed sleep). A regression leaves the broker alive, so the test's own timeout
|
||||
// surfaces the failure.
|
||||
import { describe, expect, it } from "bun:test";
|
||||
import * as fs from "node:fs/promises";
|
||||
import * as path from "node:path";
|
||||
import { TempDir } from "@oh-my-pi/pi-utils";
|
||||
import { startDaemonBrokerFromEnvironment } from "../../src/launch/broker";
|
||||
import { createDaemonBrokerClient } from "../../src/launch/client";
|
||||
import { DAEMON_IDLE_GRACE_ENV, DAEMON_PROJECT_DIR_ENV, DAEMON_RUNTIME_DIR_ENV } from "../../src/launch/protocol";
|
||||
|
||||
function restoreEnv(name: string, value: string | undefined): void {
|
||||
if (value === undefined) delete process.env[name];
|
||||
else process.env[name] = value;
|
||||
}
|
||||
|
||||
function startBroker(projectDir: string, runtimeDir: string, idleGraceMs: number): Promise<void> {
|
||||
const previousProjectDir = process.env[DAEMON_PROJECT_DIR_ENV];
|
||||
const previousRuntimeDir = process.env[DAEMON_RUNTIME_DIR_ENV];
|
||||
const previousGrace = process.env[DAEMON_IDLE_GRACE_ENV];
|
||||
process.env[DAEMON_PROJECT_DIR_ENV] = projectDir;
|
||||
process.env[DAEMON_RUNTIME_DIR_ENV] = runtimeDir;
|
||||
process.env[DAEMON_IDLE_GRACE_ENV] = String(idleGraceMs);
|
||||
const broker = startDaemonBrokerFromEnvironment();
|
||||
restoreEnv(DAEMON_PROJECT_DIR_ENV, previousProjectDir);
|
||||
restoreEnv(DAEMON_RUNTIME_DIR_ENV, previousRuntimeDir);
|
||||
restoreEnv(DAEMON_IDLE_GRACE_ENV, previousGrace);
|
||||
return broker;
|
||||
}
|
||||
|
||||
describe("daemon broker idle shutdown", () => {
|
||||
it("shuts down after its last persistent daemon exits with no clients", async () => {
|
||||
using tempDir = TempDir.createSync("@omp-launch-idle-");
|
||||
const projectDir = path.join(tempDir.path(), "project");
|
||||
const runtimeDir = path.join(tempDir.path(), "runtime");
|
||||
await fs.mkdir(projectDir);
|
||||
|
||||
const previousTitle = process.title;
|
||||
// Create the client (writes broker.token) before starting the broker, which reads that token.
|
||||
const client = await createDaemonBrokerClient(projectDir, { runtimeDir, idleGraceMs: 200 });
|
||||
const broker = startBroker(projectDir, runtimeDir, 200);
|
||||
try {
|
||||
// A persistent daemon that outlives the first idle-shutdown timer (200ms) and then
|
||||
// self-exits (~700ms). restart:"no" so its exit is terminal.
|
||||
const started = await client.request({
|
||||
op: "start",
|
||||
spec: {
|
||||
name: "persistent-temp",
|
||||
application: process.execPath,
|
||||
args: ["-e", "setTimeout(() => {}, 700)"],
|
||||
env: {},
|
||||
cwd: projectDir,
|
||||
pty: false,
|
||||
restart: "no",
|
||||
persist: true,
|
||||
detached: false,
|
||||
},
|
||||
});
|
||||
expect(started.op).toBe("start");
|
||||
|
||||
// Disconnect the final client. The broker keeps the persistent daemon alive, so the
|
||||
// idle timer this arms fires while the daemon is still live and returns without rearming.
|
||||
client.close();
|
||||
|
||||
// When the daemon self-exits, terminal settlement must rearm idle shutdown; the broker
|
||||
// then releases its lease and run() resolves. Awaiting the broker promise IS the shutdown
|
||||
// signal. Before the fix nothing rearmed, so this await never resolved and the test timed
|
||||
// out — the regression this guards.
|
||||
await broker;
|
||||
} finally {
|
||||
process.title = previousTitle;
|
||||
}
|
||||
}, 30_000);
|
||||
});
|
||||
Reference in New Issue
Block a user