diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index a4d66e8d2..f958a808d 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed the launch broker staying alive indefinitely after its last persistent daemon exited with no clients connected: the idle-shutdown timer that fired while the daemon was still live returned without rearming, and terminal settlement never scheduled another idle check, so the broker process, endpoint, timers, and record maps leaked. Terminal settlement now rearms idle shutdown, which re-checks clients, remaining live persistent daemons, and detached project presence before exiting ([#8110](https://github.com/can1357/oh-my-pi/issues/8110)). + ## [17.2.12] - 2026-08-08 ### Fixed diff --git a/packages/coding-agent/src/launch/broker.test.ts b/packages/coding-agent/src/launch/broker.test.ts new file mode 100644 index 000000000..880e9ef03 --- /dev/null +++ b/packages/coding-agent/src/launch/broker.test.ts @@ -0,0 +1,95 @@ +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); diff --git a/packages/coding-agent/src/launch/broker.ts b/packages/coding-agent/src/launch/broker.ts index a85c36922..bf58ddaef 100644 --- a/packages/coding-agent/src/launch/broker.ts +++ b/packages/coding-agent/src/launch/broker.ts @@ -990,6 +990,13 @@ class DaemonBroker { ) { this.#notifyCompletion(completion); } + // Terminal settlement can free the last live persistent daemon. The idle + // timer that fired while that daemon was alive returned without rearming + // (see #scheduleIdleShutdown), so rearm here or the broker, its endpoint, + // timers, and record maps stay alive forever after the daemon exits. The + // timer re-checks clients, remaining live persistent records, and detached + // project presence before it shuts anything down. + this.#scheduleIdleShutdown(); } async #logs(operation: Extract): Promise {