mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-31 04:12:10 +03:00
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.
265 lines
9.5 KiB
TypeScript
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();
|
|
}
|
|
});
|