fix(bootstrap): never stop the dashboard from inside its own systemd cgroup
A plugin self-update took the dashboard down and left it down. The chain
ran correctly up to the point where the bootstrap SIGTERMs the server it is
replacing — and that server was systemd-managed, with the bootstrap running
as its descendant, i.e. inside its cgroup. Two failures fired at once:
* the server exits 0, and `Restart=on-failure` reads a clean exit as
"meant to stop", so systemd did not bring it back;
* `KillMode=control-group` reaped the bootstrap itself, mid-run, before it
could repoint the unit at the new plugin root or write state.json.
Evidence from the incident: bootstrap.log ends at "stopped the server
started from the previous plugin version" with no systemd line after it,
.bootstrap.lock survived with a dead pid, state.json still named the
previous version, and the unit's ExecStart still pointed at the old cache
dir. The same truncation is visible on three earlier updates — those only
recovered because a later SessionStart bootstrapped again.
systemd-service.js now splits install into writeUserService (repoints the
unit, starts nothing) and restartUserService (--no-block). The bootstrap
skips the manual stop whenever a unit of ours owns the server, and defers
the restart past writeState and releaseLock, so being killed by its own
request costs nothing. The unit is Restart=always: a graceful SIGTERM from
anything must no longer read as "leave it stopped".
installUserService keeps its old semantics for `agenthub service install`,
which runs from a user's shell and is not in the cgroup.
This commit is contained in:
@@ -0,0 +1,147 @@
|
||||
/**
|
||||
* @file Tests scripts/systemd-service.js, and specifically the split that
|
||||
* exists because of one incident: the plugin self-update killed the dashboard
|
||||
* and left it dead.
|
||||
*
|
||||
* The bootstrap that performs an update is spawned by the running server, so
|
||||
* it lives in that server's systemd cgroup. The old code SIGTERM-ed the server
|
||||
* from inside that cgroup, which (a) exits 0, and `Restart=on-failure` reads a
|
||||
* clean exit as "meant to stop", and (b) trips `KillMode=control-group`, so
|
||||
* systemd reaped the bootstrap before it could write state or repoint the unit
|
||||
* at the new plugin root. The dashboard then stayed down until some later
|
||||
* Claude Code session happened to bootstrap again.
|
||||
*
|
||||
* Hence: writing the unit must not start anything, restarting must not block,
|
||||
* and a clean exit must no longer be read as "leave it stopped".
|
||||
* @author Nguyễn Ngọc Trí Vĩ <vinnt@smartgift.vn>
|
||||
*/
|
||||
|
||||
const { describe, it, beforeEach, after } = require("node:test");
|
||||
const assert = require("node:assert/strict");
|
||||
const fs = require("node:fs");
|
||||
const os = require("node:os");
|
||||
const path = require("node:path");
|
||||
|
||||
const TMP_HOME = fs.mkdtempSync(path.join(os.tmpdir(), "agenthub-systemd-"));
|
||||
const prevHome = process.env.HOME;
|
||||
process.env.HOME = TMP_HOME;
|
||||
|
||||
const svc = require("../../scripts/systemd-service");
|
||||
|
||||
/** Every systemctl invocation this test saw, as flat argv strings. */
|
||||
let calls = [];
|
||||
|
||||
beforeEach(() => {
|
||||
calls = [];
|
||||
// Each test starts from "no unit on disk" — otherwise a previous test's
|
||||
// write turns the next one's `written` into `unchanged`.
|
||||
fs.rmSync(svc.unitPath("agenthub"), { force: true });
|
||||
svc.__setExecImpl((args) => {
|
||||
// `--version` is the availability probe every entry point runs first, not
|
||||
// an action — recording it would bury the calls these tests are about.
|
||||
if (args[0] !== "--version") calls.push(args.join(" "));
|
||||
return { status: 0 };
|
||||
});
|
||||
});
|
||||
|
||||
after(() => {
|
||||
svc.__reset();
|
||||
if (prevHome === undefined) delete process.env.HOME;
|
||||
else process.env.HOME = prevHome;
|
||||
fs.rmSync(TMP_HOME, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
const UNIT = {
|
||||
name: "agenthub",
|
||||
description: "AgentHub dashboard server (Claude Code plugin)",
|
||||
workingDir: "/plugin/root/new",
|
||||
execStart: "/usr/bin/node /plugin/root/new/server/index.js",
|
||||
env: { NODE_ENV: "production" },
|
||||
};
|
||||
|
||||
function unitFile() {
|
||||
return fs.readFileSync(svc.unitPath("agenthub"), "utf8");
|
||||
}
|
||||
|
||||
const linuxOnly = { skip: process.platform !== "linux" ? "systemd is Linux-only" : false };
|
||||
|
||||
describe("systemd-service — unit content", linuxOnly, () => {
|
||||
it("restarts always, because a graceful SIGTERM exits 0", () => {
|
||||
svc.writeUserService(UNIT);
|
||||
const body = unitFile();
|
||||
|
||||
// `on-failure` is what let one stray kill (or an out-of-band stop during
|
||||
// an update) leave the dashboard down indefinitely.
|
||||
assert.match(body, /^Restart=always$/m);
|
||||
assert.doesNotMatch(body, /^Restart=on-failure$/m);
|
||||
});
|
||||
|
||||
it("carries the ownership marker, so a hand-written unit is never clobbered", () => {
|
||||
svc.writeUserService(UNIT);
|
||||
assert.match(unitFile(), /agenthub-managed-unit/);
|
||||
});
|
||||
});
|
||||
|
||||
describe("systemd-service — writeUserService", linuxOnly, () => {
|
||||
it("writes and enables the unit but starts NOTHING", () => {
|
||||
// The point of the split: the update must be able to repoint the unit at
|
||||
// the new plugin root while the old server is still serving requests.
|
||||
const result = svc.writeUserService(UNIT);
|
||||
|
||||
assert.equal(result, "written");
|
||||
assert.ok(
|
||||
calls.some((c) => c.startsWith("daemon-reload")),
|
||||
"a rewritten unit must be reloaded"
|
||||
);
|
||||
assert.ok(
|
||||
calls.some((c) => c === "enable agenthub.service"),
|
||||
"it must still be enabled for boot"
|
||||
);
|
||||
assert.ok(
|
||||
!calls.some((c) => c.includes("--now") || c.startsWith("restart") || c.startsWith("start")),
|
||||
`nothing may be started here, got: ${JSON.stringify(calls)}`
|
||||
);
|
||||
});
|
||||
|
||||
it("reports an identical unit as unchanged and touches systemd not at all", () => {
|
||||
svc.writeUserService(UNIT);
|
||||
calls = [];
|
||||
|
||||
assert.equal(svc.writeUserService(UNIT), "unchanged");
|
||||
assert.deepEqual(calls, [], "an unchanged unit needs no systemctl call");
|
||||
});
|
||||
|
||||
it("refuses a unit it did not write", () => {
|
||||
fs.mkdirSync(path.dirname(svc.unitPath("agenthub")), { recursive: true });
|
||||
fs.writeFileSync(svc.unitPath("agenthub"), "[Service]\nExecStart=/opt/mine\n", "utf8");
|
||||
|
||||
assert.equal(svc.writeUserService(UNIT), "foreign");
|
||||
assert.equal(svc.unitIsOurs("agenthub"), false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("systemd-service — restartUserService", linuxOnly, () => {
|
||||
it("never blocks, because the caller is inside the cgroup being restarted", () => {
|
||||
assert.equal(svc.restartUserService("agenthub"), "restarted");
|
||||
assert.deepEqual(calls, ["restart --no-block agenthub.service"]);
|
||||
});
|
||||
|
||||
it("reports failure rather than throwing when systemctl refuses", () => {
|
||||
// Available, but the restart itself is rejected — distinct from "no
|
||||
// systemd here", which must not be reported as a failure.
|
||||
svc.__setExecImpl((args) => ({ status: args[0] === "--version" ? 0 : 1 }));
|
||||
assert.equal(svc.restartUserService("agenthub"), "failed");
|
||||
});
|
||||
});
|
||||
|
||||
describe("systemd-service — unitIsOurs", linuxOnly, () => {
|
||||
it("is false when no unit exists at all", () => {
|
||||
fs.rmSync(svc.unitPath("agenthub"), { force: true });
|
||||
assert.equal(svc.unitIsOurs("agenthub"), false);
|
||||
});
|
||||
|
||||
it("is true for a unit this code wrote", () => {
|
||||
svc.writeUserService(UNIT);
|
||||
assert.equal(svc.unitIsOurs("agenthub"), true);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user