fix(plugins): serverIsLive() falsely reported a server as running
resolveAllDashboardPorts() falls back to [DEFAULT_PORT] when the discovery file has no live entry — a reasonable guess for the CLI/hook handler, but wrong for the bootstrap's own liveness check: with no server running at all, the bootstrap believed one was already up and never called startDashboard(), confirmed against a real plugin install where the dashboard never started. plugin-doctor.js's "Server" row had the same bug. Both now read the discovery file directly and check PID liveness via the new liveServers() (livePids() reused it instead of duplicating the read).
This commit is contained in:
+21
-11
@@ -23,7 +23,7 @@ const crypto = require("crypto");
|
||||
const { spawn, spawnSync } = require("child_process");
|
||||
|
||||
const { getDataDir, getClaudeHome, getSettingsPath } = require("../server/lib/claude-home");
|
||||
const { resolveAllDashboardPorts, getServerInfoPath } = require("../server/lib/server-info");
|
||||
const { getServerInfoPath } = require("../server/lib/server-info");
|
||||
const { isOurEntry } = require("./install-hooks");
|
||||
|
||||
const PLUGIN_ROOT = path.resolve(__dirname, "..");
|
||||
@@ -66,11 +66,12 @@ function writeState(state, rt = runtimeDir()) {
|
||||
|
||||
/** A dashboard server that is actually listening (the discovery file is PID-checked). */
|
||||
function serverIsLive() {
|
||||
try {
|
||||
return resolveAllDashboardPorts().length > 0;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
// NOT resolveAllDashboardPorts(): it falls back to [DEFAULT_PORT] when the
|
||||
// discovery file has no live entry (a reasonable guess for the CLI/hook
|
||||
// handler), which would make bootstrap believe a server is already running
|
||||
// when none is. livePids() only counts entries with a PID that is actually
|
||||
// alive.
|
||||
return livePids().length > 0;
|
||||
}
|
||||
|
||||
/** Hash of the dependency manifest — changes mean the runtime tree must be reinstalled. */
|
||||
@@ -282,21 +283,28 @@ function installDeps(rt = runtimeDir(), root = PLUGIN_ROOT) {
|
||||
/* ------------------------------------------------------------------ server */
|
||||
|
||||
/**
|
||||
* PIDs of dashboard servers recorded in the discovery file that are still
|
||||
* running. Reads the file directly because server-info exposes ports only.
|
||||
* Discovery-file entries whose PID is actually alive right now. Reads the file
|
||||
* directly rather than server-info's `resolveAllDashboardPorts()`, which falls
|
||||
* back to `[DEFAULT_PORT]` when nothing is live — a reasonable guess for the
|
||||
* CLI/hook handler, but wrong here: it would make the bootstrap believe a
|
||||
* server is already running when none is.
|
||||
*
|
||||
* @returns {number[]}
|
||||
* @returns {{port:number, pid:number}[]}
|
||||
*/
|
||||
function livePids() {
|
||||
function liveServers() {
|
||||
try {
|
||||
const parsed = JSON.parse(fs.readFileSync(getServerInfoPath(), "utf8"));
|
||||
const servers = Array.isArray(parsed.servers) ? parsed.servers : [parsed];
|
||||
return servers.map((s) => s && s.pid).filter((pid) => isPidAlive(pid));
|
||||
return servers.filter((s) => s && isPidAlive(s.pid));
|
||||
} catch {
|
||||
return [];
|
||||
}
|
||||
}
|
||||
|
||||
function livePids() {
|
||||
return liveServers().map((s) => s.pid);
|
||||
}
|
||||
|
||||
/**
|
||||
* Stop running dashboard servers. Needed after a plugin update: the old server
|
||||
* runs from a cache directory Claude Code has already replaced, so it must be
|
||||
@@ -501,6 +509,8 @@ module.exports = {
|
||||
stripLegacyHooks,
|
||||
linkCli,
|
||||
installDeps,
|
||||
serverIsLive,
|
||||
liveServers,
|
||||
livePids,
|
||||
stopDashboard,
|
||||
startDashboard,
|
||||
|
||||
@@ -16,7 +16,6 @@ const path = require("path");
|
||||
const boot = require("./plugin-bootstrap");
|
||||
const { isOurEntry } = require("./install-hooks");
|
||||
const { getSettingsPath, getDataDir } = require("../server/lib/claude-home");
|
||||
const { resolveAllDashboardPorts } = require("../server/lib/server-info");
|
||||
const { mcpBuildStatus } = require("./check-mcp-build");
|
||||
|
||||
const PLUGIN_ROOT = path.resolve(__dirname, "..");
|
||||
@@ -63,18 +62,12 @@ function diagnose() {
|
||||
deps ? path.join(rt, "node_modules") : "missing — run /ccam-update"
|
||||
);
|
||||
|
||||
const ports = (() => {
|
||||
try {
|
||||
return resolveAllDashboardPorts();
|
||||
} catch {
|
||||
return [];
|
||||
}
|
||||
})();
|
||||
const live = boot.liveServers();
|
||||
add(
|
||||
ports.length ? "ok" : "fail",
|
||||
live.length ? "ok" : "fail",
|
||||
"Server",
|
||||
ports.length
|
||||
? `listening on ${ports.map((p) => `http://localhost:${p}`).join(", ")}`
|
||||
live.length
|
||||
? `listening on ${live.map((s) => `http://localhost:${s.port}`).join(", ")}`
|
||||
: `not running — see ${path.join(rt, "server.log")}`
|
||||
);
|
||||
|
||||
|
||||
@@ -31,6 +31,38 @@ after(() => {
|
||||
fs.rmSync(TMP_HOME, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
describe("serverIsLive", () => {
|
||||
const infoPath = path.join(TMP_HOME, ".agent-dashboard.json");
|
||||
|
||||
beforeEach(() => fs.rmSync(infoPath, { force: true }));
|
||||
after(() => fs.rmSync(infoPath, { force: true }));
|
||||
|
||||
it("is false with no discovery file at all", () => {
|
||||
// Regression: server-info's resolveAllDashboardPorts() falls back to
|
||||
// [DEFAULT_PORT] when nothing is live (a reasonable guess for the CLI),
|
||||
// which made the bootstrap wrongly believe a server was already running
|
||||
// and skip starting its own.
|
||||
assert.equal(boot.serverIsLive(), false);
|
||||
});
|
||||
|
||||
it("is false when every recorded pid is dead", () => {
|
||||
fs.writeFileSync(
|
||||
infoPath,
|
||||
JSON.stringify({ servers: [{ port: 4820, pid: 999999, startedAt: "x" }] })
|
||||
);
|
||||
assert.equal(boot.serverIsLive(), false);
|
||||
});
|
||||
|
||||
it("is true when a recorded pid is actually alive", () => {
|
||||
fs.writeFileSync(
|
||||
infoPath,
|
||||
JSON.stringify({ servers: [{ port: 4820, pid: process.pid, startedAt: "x" }] })
|
||||
);
|
||||
assert.equal(boot.serverIsLive(), true);
|
||||
assert.deepEqual(boot.liveServers(), [{ port: 4820, pid: process.pid, startedAt: "x" }]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("runtime location", () => {
|
||||
it("lives under the shared data dir, not the plugin cache", () => {
|
||||
assert.equal(boot.runtimeDir(), RT);
|
||||
|
||||
Reference in New Issue
Block a user