diff --git a/CLAUDE.md b/CLAUDE.md index b7de34d8..6fc39851 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,6 +1,6 @@ # Claude Code Working Guide -_Last synced: eb63569 (2026-09-10) + release-based update detection (`server/lib/update-check.js` reads the forge releases API — a checkout via its `origin` remote, a plugin-cache install via the marketplace manifest on disk, since an archive-sourced cache dir has no `.git`; a `plugin_managed` install self-updates via the detached `plugin-update` chain) + the install-wide **Update all** batch and session reload (`server/lib/cc-update-all.js`, `server/lib/cc-reload.js`)._ A change that makes any statement in this file, `AGENTS.md`, or a README wrong — a moved path, a new or changed extension point, a retired rule — updates that file in the same change-set and bumps this marker. +_Last synced: v1.5.6 (2026-09-11) + the systemd handover rule below (`scripts/systemd-service.js`'s `writeUserService`/`restartUserService` split and `plugin-bootstrap.js`'s deferred `pendingSystemdRestart`) + eb63569 (2026-09-10) + release-based update detection (`server/lib/update-check.js` reads the forge releases API — a checkout via its `origin` remote, a plugin-cache install via the marketplace manifest on disk, since an archive-sourced cache dir has no `.git`; a `plugin_managed` install self-updates via the detached `plugin-update` chain) + the install-wide **Update all** batch and session reload (`server/lib/cc-update-all.js`, `server/lib/cc-reload.js`)._ A change that makes any statement in this file, `AGENTS.md`, or a README wrong — a moved path, a new or changed extension point, a retired rule — updates that file in the same change-set and bumps this marker. ## Project mission - Maintain a reliable local-first dashboard for Claude Code session monitoring. @@ -43,6 +43,7 @@ A **lane** is a durable unit of parallel agent work — one working directory, m See `docs/LANES.md` for full guide: stage reporting, custom templates, lane actions, and the five node states. ## Non-negotiable engineering rules +- **When a `agenthub.service` unit of ours owns the server, nothing else may stop it.** The bootstrap that performs a self-update is spawned by the running server, so it lives in that server's systemd cgroup: SIGTERM-ing the server from there exits 0 (which `Restart=on-failure` read as "meant to stop") AND trips `KillMode=control-group`, reaping the bootstrap before it can repoint the unit or write `state.json` — a dashboard dead after every update until some later session bootstrapped again. So `scripts/systemd-service.js` splits the API: `writeUserService` repoints the unit and starts NOTHING, `restartUserService` uses `--no-block`, and `plugin-bootstrap.js` defers that restart past `writeState` and `releaseLock` (`pendingSystemdRestart`). The unit is `Restart=always`. Do not add a path that stops the dashboard from inside its own cgroup, and do not move the restart earlier. `agenthub service install` may keep using `installUserService` — it runs from a user's shell, outside the cgroup. - **A session reload is typed into a live pane, never spawned.** `/reload-plugins` and `/reload-skills` are scoped BY CLAUDE CODE to the session that runs them, so nothing can reload a session from the outside. `server/lib/cc-reload.js` therefore types them into the dashboard's existing tmux lane panes and spawns NO `claude` process — a one-shot `claude -p "/reload-plugins"` reloads only its own short-lived session, exits, and leaves a junk session in the dashboard's own history. Do not add a path that starts a session in order to reload one. - Preserve existing behavior unless explicitly asked to change it. - Prefer minimal, reversible diffs. diff --git a/docs/DEPLOYMENT.md b/docs/DEPLOYMENT.md index 170f4236..b0e5b878 100644 --- a/docs/DEPLOYMENT.md +++ b/docs/DEPLOYMENT.md @@ -456,7 +456,10 @@ Environment=NODE_ENV=production Environment=PORT=4820 Environment=DASHBOARD_DB_PATH=/var/lib/agenthub/dashboard.db ExecStart=/usr/bin/node server/index.js -Restart=on-failure +# `always`, not `on-failure`: a graceful SIGTERM exits 0, which `on-failure` +# reads as "meant to stop" — one stray kill and the dashboard stays down. +# `systemctl stop` still stops it. Same reasoning as the plugin's own unit. +Restart=always RestartSec=10 StandardOutput=journal StandardError=journal diff --git a/docs/PLUGINS.md b/docs/PLUGINS.md index 426af1b9..41da8d1c 100644 --- a/docs/PLUGINS.md +++ b/docs/PLUGINS.md @@ -144,6 +144,9 @@ waits on an install: `client-dist` — scoped to this session, same as before this fell back to systemd. 9. **Record state** — `runtime/state.json`. +10. **Hand over to systemd** — only when a unit of ours already owns the server + (see [Who may stop the server](#who-may-stop-the-server)). Deliberately + after step 9. The first run takes a few minutes — most of it is the UI build, which is what makes client-only routes (`http://localhost:4820/run`, and every other page) @@ -172,6 +175,41 @@ On macOS, Windows, or a Linux box without systemd, nothing changes: the server is a detached spawn started fresh on the next `SessionStart` after a reboot, same as always. +### Who may stop the server + +**When a `agenthub.service` unit of ours exists, nothing else may stop the +dashboard.** The bootstrap that performs a self-update is spawned by the +running server, so it lives inside that server's systemd cgroup. SIGTERM-ing +the server from there breaks two ways at once: + +- the server exits **0**, and the unit's old `Restart=on-failure` read a clean + exit as "meant to stop" — so systemd did not bring it back; +- `KillMode=control-group` (systemd's default) reaps **every** process in the + cgroup when the main process exits, including the bootstrap itself — mid-run, + before it could repoint the unit at the new plugin root or write + `runtime/state.json`. + +The result was a dashboard that went down on every self-update and stayed down +until some later Claude Code session happened to bootstrap again, leaving a +stale `.bootstrap.lock` and a `state.json` still naming the previous version. + +So the order is fixed, and `scripts/systemd-service.js` splits its API to +enforce it: + +| Situation | What the bootstrap does | +|---|---| +| A unit of ours owns the server (`unitIsOurs`) | Never stops it. `writeUserService` repoints the unit at the new plugin root **without starting anything**, the rest of the bootstrap runs, state is written, the lock is released — and only then `restartUserService` asks systemd to restart, with `--no-block` so being killed by its own request costs nothing. | +| No unit, or a foreign one | Unchanged: stop the old server, then `installUserService` (or a detached spawn where there is no systemd). Nothing here is inside a unit's cgroup, so the stop is safe. | + +The unit is written with **`Restart=always`**, not `on-failure`: a graceful +SIGTERM from anything at all would otherwise leave the dashboard down for good. +An explicit `systemctl stop` — which is what `agenthub service uninstall` and +the Settings panel do — still stops it. + +`agenthub service install` (the checkout path) keeps using `installUserService`: +it runs from your own shell, outside the unit's cgroup, so it may block and +start in one step. + ### Commands | Command | Does | @@ -268,8 +306,8 @@ not assumed. It does **not**: For a clean machine, do all of this yourself: ```bash -# 1. Stop the server. On Linux, check for the systemd unit first — a bare -# `kill` just gets restarted by it (Restart=on-failure). +# 1. Stop the server. On Linux, disable the systemd unit FIRST — the unit is +# Restart=always, so a bare `kill` is always undone by systemd. systemctl --user disable --now agenthub.service 2>/dev/null rm -f ~/.config/systemd/user/agenthub.service && systemctl --user daemon-reload 2>/dev/null # Fallback for a detached (non-systemd) server, or if the unit above was absent: diff --git a/scripts/plugin-bootstrap.js b/scripts/plugin-bootstrap.js index ec33eedb..cd478692 100644 --- a/scripts/plugin-bootstrap.js +++ b/scripts/plugin-bootstrap.js @@ -465,6 +465,11 @@ function bootstrap(rt = runtimeDir(), { force = false } = {}) { return "locked"; } + // Declared out here because the restart it guards has to happen in the + // `finally`, after the lock is released and the state is on disk: it is the + // step that may cost this process its own life. + let pendingSystemdRestart = false; + try { const wantDeps = depsHash(); const state = readState(rt) || {}; @@ -504,34 +509,55 @@ function bootstrap(rt = runtimeDir(), { force = false } = {}) { log(rt, "built the dashboard UI bundle"); } + const svc = require("./systemd-service"); + // Who owns the running server decides how it may be retired. A unit of + // ours means systemd owns it — and this process is a descendant of that + // server, so it sits in the unit's cgroup. SIGTERM-ing the main process + // there does two fatal things at once: the clean exit(0) tells systemd + // "meant to stop", and `KillMode=control-group` reaps this bootstrap + // before it can write state or point the unit at the new cache dir. That + // is the bug that left a dashboard dead after every self-update until the + // next Claude Code session happened to bootstrap again. + const systemdOwned = svc.systemctlAvailable() && svc.unitIsOurs("agenthub"); + // After a plugin update the running server executes code from a cache - // directory Claude Code has already discarded — retire it first. + // directory Claude Code has already discarded — retire it first. Only + // when nothing else owns its lifecycle; otherwise systemd retires it, in + // one restart job, once everything below is durable. const movedRoot = force || (state.pluginRoot && state.pluginRoot !== PLUGIN_ROOT); - if (movedRoot && stopDashboard()) { + if (movedRoot && !systemdOwned && stopDashboard()) { log(rt, "stopped the server started from the previous plugin version"); waitForPortsFree(); } if (movedRoot || !serverIsLive()) { - const svc = require("./systemd-service"); - const result = svc.systemctlAvailable() - ? svc.installUserService({ - name: "agenthub", - description: "AgentHub dashboard server (Claude Code plugin)", - workingDir: PLUGIN_ROOT, - execStart: `${process.execPath} ${path.join(PLUGIN_ROOT, "server", "index.js")}`, - env: { - NODE_ENV: "production", - NODE_PATH: path.join(rt, "node_modules"), - DASHBOARD_CLIENT_DIST: path.join(rt, "client-dist"), - // systemd --user units get systemd's own minimal PATH, not the - // shell's — forward it so the server can find `claude`/`codex`. - PATH: process.env.PATH || "", - }, - }) - : "unavailable"; + const unit = { + name: "agenthub", + description: "AgentHub dashboard server (Claude Code plugin)", + workingDir: PLUGIN_ROOT, + execStart: `${process.execPath} ${path.join(PLUGIN_ROOT, "server", "index.js")}`, + env: { + NODE_ENV: "production", + NODE_PATH: path.join(rt, "node_modules"), + DASHBOARD_CLIENT_DIST: path.join(rt, "client-dist"), + // systemd --user units get systemd's own minimal PATH, not the + // shell's — forward it so the server can find `claude`/`codex`. + PATH: process.env.PATH || "", + }, + }; + // Two shapes of the same step. When systemd already owns the server the + // unit is only REWRITTEN here — pointing at the new plugin root — and + // the restart is deferred to `pendingSystemdRestart` below, because + // starting it now would kill this process mid-bootstrap. Otherwise + // there is no cgroup to be reaped by, and install-and-start is fine. + const result = systemdOwned + ? svc.writeUserService(unit) + : svc.systemctlAvailable() + ? svc.installUserService(unit) + : "unavailable"; if (result === "written" || result === "unchanged") { if (result === "written") svc.enableLinger(); + pendingSystemdRestart = systemdOwned; log( rt, result === "written" @@ -572,6 +598,15 @@ function bootstrap(rt = runtimeDir(), { force = false } = {}) { return "ok"; } finally { releaseLock(rt); + // LAST, and after the lock: systemd stops the old main process to start + // the new one, and this process shares its cgroup, so it may not outlive + // this call. Everything that had to be durable already is — the state + // file is written and the lock is gone, so even being killed here leaves + // no debris and no half-recorded bootstrap. + if (pendingSystemdRestart) { + log(rt, "handing the server over to systemd (restarting the unit)"); + require("./systemd-service").restartUserService("agenthub"); + } } } diff --git a/scripts/systemd-service.js b/scripts/systemd-service.js index 70678fa8..79dccee6 100644 --- a/scripts/systemd-service.js +++ b/scripts/systemd-service.js @@ -26,11 +26,43 @@ function unitPath(name) { return path.join(unitDir(), `${name}.service`); } -function systemctlAvailable() { - if (process.platform !== "linux") return false; - return spawnSync("systemctl", ["--user", "--version"], { stdio: "ignore" }).status === 0; +// Test seam: swap the systemctl implementation so unit tests never talk to a +// real service manager. Mirrors tmux.js's __setExecImpl/__reset style. +let execImpl = (args, opts) => spawnSync("systemctl", ["--user", ...args], opts); + +function __setExecImpl(fn) { + execImpl = fn; +} +function __reset() { + execImpl = (args, opts) => spawnSync("systemctl", ["--user", ...args], opts); } +function systemctlAvailable() { + if (process.platform !== "linux") return false; + return execImpl(["--version"], { stdio: "ignore" }).status === 0; +} + +/** + * Is there a unit by this name, and did WE write it? The distinction decides + * whether the dashboard may be stopped by hand: a unit of ours means systemd + * owns the process, and SIGTERM-ing it out of band is the one thing that must + * never happen (see `restartUserService`). + */ +function unitIsOurs(name) { + try { + return fs.readFileSync(unitPath(name), "utf8").includes(MARKER); + } catch { + return false; + } +} + +/** + * `Restart=always`, not `on-failure`: a graceful SIGTERM exits 0, which + * `on-failure` reads as "meant to stop". One stray kill — or an out-of-band + * stop during a plugin self-update — then left the dashboard down until some + * later Claude Code session happened to run the bootstrap. An explicit + * `systemctl stop` (what uninstall does) still stops it for good. + */ function unitContent({ description, workingDir, execStart, env }) { const envLines = Object.entries(env) .map(([k, v]) => `Environment=${k}=${v}`) @@ -44,7 +76,7 @@ After=network.target Type=simple WorkingDirectory=${workingDir} ExecStart=${execStart} -Restart=on-failure +Restart=always RestartSec=3 ${envLines} @@ -54,17 +86,18 @@ WantedBy=default.target } function systemctl(args) { - return spawnSync("systemctl", ["--user", ...args], { stdio: "inherit" }); + return execImpl(args, { stdio: "inherit" }); } /** - * Writes (or refreshes) the unit file and enables + starts it. Idempotent: - * an unchanged unit is left running as-is; a changed one (e.g. a plugin - * cache path after an update) is reloaded and restarted. + * Writes (or refreshes) the unit file and enables it for boot — but starts + * nothing. Split out of `installUserService` for the plugin self-update, which + * must be able to point the unit at the new cache directory *before* anything + * stops the running server, and defer the restart to its very last act. * * @returns {"written"|"unchanged"|"foreign"|"unavailable"|"failed"} */ -function installUserService({ name, description, workingDir, execStart, env = {} }) { +function writeUserService({ name, description, workingDir, execStart, env = {} }) { if (!systemctlAvailable()) return "unavailable"; const body = unitContent({ description, workingDir, execStart, env }); const target = unitPath(name); @@ -72,22 +105,56 @@ function installUserService({ name, description, workingDir, execStart, env = {} if (fs.existsSync(target)) { const current = fs.readFileSync(target, "utf8"); if (!current.includes(MARKER)) return "foreign"; - if (current === body) { - systemctl(["enable", "--now", `${name}.service`]); - return "unchanged"; - } + if (current === body) return "unchanged"; } fs.mkdirSync(unitDir(), { recursive: true }); fs.writeFileSync(target, body, "utf8"); systemctl(["daemon-reload"]); - systemctl(["enable", "--now", `${name}.service`]); - systemctl(["restart", `${name}.service`]); + systemctl(["enable", `${name}.service`]); return "written"; } catch { return "failed"; } } +/** + * Ask systemd to restart the unit and do NOT wait for it. + * + * `--no-block` is the whole point, not a nicety. The caller is the bootstrap + * spawned by the running server, so it lives in that server's cgroup; the + * moment systemd stops the old main process, `KillMode=control-group` reaps + * the caller too. Blocking here means being killed mid-call, which is why the + * restart must be the last thing the bootstrap does — by then there is nothing + * left to lose. + */ +function restartUserService(name) { + if (!systemctlAvailable()) return "unavailable"; + const res = systemctl(["restart", "--no-block", `${name}.service`]); + return res.status === 0 ? "restarted" : "failed"; +} + +/** + * Writes (or refreshes) the unit file and enables + starts it. Idempotent: + * an unchanged unit is left running as-is; a changed one (e.g. a plugin + * cache path after an update) is reloaded and restarted. + * + * Safe for `agenthub service install`, which is invoked from a user's own + * shell and is therefore not inside the unit's cgroup. The plugin bootstrap + * is, and uses `writeUserService` + `restartUserService` instead. + * + * @returns {"written"|"unchanged"|"foreign"|"unavailable"|"failed"} + */ +function installUserService(opts) { + const result = writeUserService(opts); + if (result === "unchanged") { + systemctl(["enable", "--now", `${opts.name}.service`]); + } else if (result === "written") { + systemctl(["enable", "--now", `${opts.name}.service`]); + systemctl(["restart", `${opts.name}.service`]); + } + return result; +} + /** @returns {"removed"|"absent"|"foreign"|"unavailable"} */ function uninstallUserService(name) { if (!systemctlAvailable()) return "unavailable"; @@ -110,8 +177,13 @@ function enableLinger() { module.exports = { systemctlAvailable, + unitIsOurs, + writeUserService, + restartUserService, installUserService, uninstallUserService, enableLinger, unitPath, + __setExecImpl, + __reset, }; diff --git a/server/__tests__/systemd-service.test.js b/server/__tests__/systemd-service.test.js new file mode 100644 index 00000000..f017db0a --- /dev/null +++ b/server/__tests__/systemd-service.test.js @@ -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ĩ + */ + +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); + }); +});