Files
OmniRoute/tests/unit/services/ServiceSupervisor.test.ts
Sean Ford 337373f8f0 fix(services): resolve and record a real pid when adopting a service (#8218)
ServiceSupervisor.start(), when probeBeforeSpawn detects an already-healthy
instance on the target port, "adopts" it (marks the service running without
spawning a duplicate that would die with EADDRINUSE). This adopt path calls
setToolStatus(tool, "running") with no pid argument at all, so this.pid stays
at its constructor default of null for the rest of that supervisor's life --
only the spawn path (a genuinely new child process) ever sets a real pid.

In production this "adopt" path is common, not an edge case: any embedded
sidecar service (cliproxy, 9router, bifrost, mux) whose child process
survives a `systemctl --user restart omniroute.service` gets adopted by the
new supervisor instance on the next start(), and its pid is lost from that
point on -- even though the service is genuinely healthy and running. The
observed symptom: a service shows state "running" but pid null, and
something downstream that keys liveness tracking off pid eventually treats
it as untrustworthy/stale despite nothing actually being wrong.

Fix: resolve the real pid of the process holding the port (via `lsof -ti
:<port>`, best-effort -- a lookup failure leaves pid null rather than
blocking adoption) and record it the same way the spawn path does.

An existing test asserted pid === null on adopt. That was accurate for the
old behavior but reflected a missing resolution, not a deliberate "adopted
services never get a pid" design choice -- updated its assertion to match
the corrected behavior and added a dedicated regression test proving the
resolved pid matches the real process actually bound to the port.
2026-07-23 05:24:30 -03:00

265 lines
9.5 KiB
TypeScript

/**
* ServiceSupervisor unit tests.
*
* Uses a real Node.js child process (`node -e "..."`) to test lifecycle
* without mocking child_process — this gives realistic signal/exit behavior.
*
* A tiny HTTP health server is spawned inline for health-check tests.
*/
import test from "node:test";
import assert from "node:assert/strict";
import fs from "node:fs";
import os from "node:os";
import path from "node:path";
import http from "node:http";
const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-supervisor-"));
process.env.DATA_DIR = TEST_DATA_DIR;
process.env.NODE_ENV = "test";
process.env.DISABLE_SQLITE_AUTO_BACKUP = "true";
// Import DB core first to trigger migration (creates version_manager with new columns)
const core = await import("../../../src/lib/db/core.ts");
// Seed the tool rows needed by tests
const db = core.getDbInstance();
db.prepare(
`INSERT OR IGNORE INTO version_manager (tool, status, port, auto_start, auto_update, provider_expose)
VALUES ('test-svc', 'stopped', 29999, 0, 0, 0)`
).run();
db.prepare(
`INSERT OR IGNORE INTO version_manager (tool, status, port, auto_start, auto_update, provider_expose)
VALUES ('test-crash', 'stopped', 29998, 0, 0, 0)`
).run();
db.prepare(
`INSERT OR IGNORE INTO version_manager (tool, status, port, auto_start, auto_update, provider_expose)
VALUES ('test-lock', 'stopped', 29997, 0, 0, 0)`
).run();
db.prepare(
`INSERT OR IGNORE INTO version_manager (tool, status, port, auto_start, auto_update, provider_expose)
VALUES ('test-adopt', 'stopped', 29996, 0, 0, 0)`
).run();
const { ServiceSupervisor } = await import("../../../src/lib/services/ServiceSupervisor.ts");
/** Starts a tiny HTTP health server on the given port that always returns 200. */
function startHealthServer(port: number): http.Server {
const server = http.createServer((_, res) => res.writeHead(200).end("ok"));
server.listen(port);
return server;
}
/** Config for a service that logs "tick" every second and stays alive. */
function tickConfig(tool: string, port: number) {
return {
tool,
port,
spawnArgs: () => ({
command: process.execPath,
args: ["-e", "setInterval(() => console.log('tick'), 500)"],
env: { ...process.env },
cwd: process.cwd(),
}),
healthUrl: () => `http://127.0.0.1:${port}/health`,
healthIntervalMs: 500,
stopTimeoutMs: 3_000,
logsBufferBytes: 1_048_576,
};
}
test.after(() => {
core.resetDbInstance();
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
});
test("start spawns process and captures logs in ring buffer", async () => {
const healthServer = startHealthServer(29999);
const sup = new ServiceSupervisor(tickConfig("test-svc", 29999));
try {
const status = await sup.start();
assert.equal(status.state, "running");
assert.ok(status.pid !== null, "pid should be set");
// Wait briefly to let ticks accumulate
await new Promise((r) => setTimeout(r, 600));
const snap = sup.getRingBuffer().snapshot();
assert.ok(snap.length > 0, "ring buffer should have log entries");
assert.ok(
snap.some((e) => e.line.includes("tick")),
"should capture stdout lines"
);
} finally {
await sup.stop();
healthServer.close();
}
});
test("stop sends SIGTERM and waits, then SIGKILL if needed", async () => {
const healthServer = startHealthServer(29999);
const sup = new ServiceSupervisor({
...tickConfig("test-svc", 29999),
stopTimeoutMs: 500,
});
try {
await sup.start();
const status = await sup.stop();
assert.equal(status.state, "stopped");
assert.equal(status.pid, null);
} finally {
healthServer.close();
}
});
test("crash sets state=error and lastError (no auto-restart)", async () => {
const healthServer = startHealthServer(29998);
const crashConfig = {
...tickConfig("test-crash", 29998),
spawnArgs: () => ({
command: process.execPath,
// Exit after 1.5s — health server is up, so start() can return "running" first
args: ["-e", "setTimeout(() => process.exit(1), 1500)"],
env: { ...process.env },
cwd: process.cwd(),
}),
healthIntervalMs: 300,
};
const sup = new ServiceSupervisor(crashConfig);
const stateChanges: string[] = [];
sup.on("stateChange", (s) => stateChanges.push(s.state));
try {
await sup.start();
assert.equal(sup.getStatus().state, "running");
// Poll for the crash to be detected (process exits at 1.5s, health checker detects it
// within ~3 intervals). A fixed sleep flakes under CPU contention because the child's
// exit timer and the health-check intervals all slip; poll with a generous deadline.
const deadline = Date.now() + 10_000;
while (sup.getStatus().state !== "error" && Date.now() < deadline) {
await new Promise((r) => setTimeout(r, 100));
}
const status = sup.getStatus();
assert.equal(status.state, "error", "state should be error after crash");
assert.ok(status.lastError !== null, "lastError should be set");
assert.ok(
!stateChanges.filter((s) => s === "starting").length ||
stateChanges[stateChanges.length - 1] !== "starting",
"supervisor must not restart after crash"
);
} finally {
healthServer.close();
}
});
test("restart is atomic (concurrent calls serialize)", async () => {
const healthServer = startHealthServer(29999);
const sup = new ServiceSupervisor(tickConfig("test-svc", 29999));
try {
await sup.start();
// Fire 3 concurrent restarts — all should resolve without throwing
const [s1, s2, s3] = await Promise.all([sup.restart(), sup.restart(), sup.restart()]);
assert.equal(s1.state, "running");
assert.equal(s2.state, "running");
assert.equal(s3.state, "running");
const final = sup.getStatus();
assert.equal(final.state, "running");
} finally {
await sup.stop();
healthServer.close();
}
});
test("does NOT auto-restart on crash", async () => {
const healthServer = startHealthServer(29998);
const crashConfig = {
...tickConfig("test-crash", 29998),
spawnArgs: () => ({
command: process.execPath,
// Exit after 1.5s — same as crash test above
args: ["-e", "setTimeout(() => process.exit(2), 1500)"],
env: { ...process.env },
cwd: process.cwd(),
}),
healthIntervalMs: 300,
};
const sup = new ServiceSupervisor(crashConfig);
try {
await sup.start();
// Wait for crash + one more health interval
await new Promise((r) => setTimeout(r, 2_200));
const status = sup.getStatus();
// After crash: state must be "error" or "stopped", never "starting" again
assert.ok(
status.state === "error" || status.state === "stopped",
`supervisor should not auto-restart: state was "${status.state}"`
);
} finally {
healthServer.close();
}
});
// #6205: when probeBeforeSpawn is enabled and a healthy instance already serves
// the port, the supervisor ADOPTS it (marks running, no child spawned) instead
// of spawning a duplicate that would die with EADDRINUSE.
test("#6205: probeBeforeSpawn adopts a healthy existing instance (no spawn)", async () => {
const healthServer = startHealthServer(29996);
const cfg = { ...tickConfig("test-adopt", 29996), probeBeforeSpawn: true };
const sup = new ServiceSupervisor(cfg);
try {
const status = await sup.start();
assert.equal(status.state, "running", "adopted instance is marked running");
// No child process handle exists on adoption (nothing was spawned), but a
// real pid is still resolved from the OS — see the dedicated pid-tracking
// test below. Historically this asserted pid === null, which just
// reflected the missing resolution rather than a deliberate "no pid for
// adopted services" design choice.
assert.ok(status.pid !== null, "adopted instance still gets a tracked pid");
// No child means no captured stdout ticks.
await new Promise((r) => setTimeout(r, 300));
assert.equal(sup.getRingBuffer().snapshot().length, 0, "no logs — nothing was spawned");
} finally {
await sup.stop();
healthServer.close();
}
});
// Regression test: the adopt branch used to call setToolStatus(tool, "running")
// with no pid argument at all, leaving `pid` permanently null for any service
// adopted on a supervisor restart (common in production — e.g. after
// `systemctl --user restart omniroute.service`, sidecar child processes can
// outlive the restart and get adopted rather than spawned fresh). Downstream
// consumers that key liveness tracking off pid would then treat a genuinely
// healthy, running service as untrustworthy/stale. This asserts the resolved
// pid on adoption matches the real process actually holding the port.
test("adopted service resolves and records the real pid of the process holding the port", async () => {
const healthServer = startHealthServer(29996);
const cfg = { ...tickConfig("test-adopt", 29996), probeBeforeSpawn: true };
const sup = new ServiceSupervisor(cfg);
try {
const status = await sup.start();
assert.equal(status.state, "running");
// The health server above runs inline in this test process (no child
// process spawned for it), so the pid actually bound to port 29996 is
// this test process's own pid — that's exactly what resolvePortPid()
// should find and what the supervisor should record.
assert.equal(status.pid, process.pid, "resolved pid matches the process holding the port");
assert.equal(sup.getStatus().pid, process.pid, "in-memory supervisor state also has the pid");
} finally {
await sup.stop();
healthServer.close();
}
});