fix: v2.7.3 — Update now restarts through our own service unit, member panes stay live on a dev server, a start refuses a missing folder
- self-restart: a server in our unit's cgroup (/agenthub.service, system or user) restarts via `systemctl restart --no-block` (sudo -n for the system unit), our launchd job by exiting into KeepAlive. The detached restart-helper replacement held the port while Restart=always started the unit every 3 s (v2.7.2 on the S3 box: 240+ restarts, MainPID 0). - run-as.mayHaveTmux: only ENOENT means "no tmux". A dev server in server mode without CAP_DAC_READ_SEARCH got EACCES on a member's 0700 .agenthub/tmux, dropped the member from the list and ended their runs within seconds — the real cause of `401 not a live pane's token`. - tmux: a member's list is re-asked every 5 s, not 1 s (one sudo + three auth.log lines each). - POST /api/lanes/:id/start: 409 ENOCWD for a folder that no longer exists (the pane started in the account's home otherwise). - Settings: Remote sources only for staff (a developer got a 403). - scripts/test-server.js drops every inherited AGENTHUB_* (pane env). Tests: self-restart.test.js, server-s3-panes (mayHaveTmux), lanes-api (ENOCWD); touched suites 771 + client pages 191, tsc -b.
This commit is contained in:
@@ -427,6 +427,16 @@ describe("lanes changed by another server on the same DB", () => {
|
||||
});
|
||||
|
||||
describe("lane start input", () => {
|
||||
it("refuses to start in a workspace folder that no longer exists (409 ENOCWD)", async () => {
|
||||
const cwd = fs.mkdtempSync(path.join(os.tmpdir(), "lane-gone-"));
|
||||
const id = (await request("POST", "/api/lanes", { cwd })).body.lane.id;
|
||||
fs.rmSync(cwd, { recursive: true, force: true });
|
||||
// Without the check the pane started in the account's home instead.
|
||||
const r = await request("POST", `/api/lanes/${id}/start`, {});
|
||||
assert.equal(r.status, 409);
|
||||
assert.equal(r.body.error.code, "ENOCWD");
|
||||
});
|
||||
|
||||
it("answers 400 for a model that is not a plain model id", async () => {
|
||||
const cwd = fs.mkdtempSync(path.join(os.tmpdir(), "lane-bad-model-"));
|
||||
const id = (await request("POST", "/api/lanes", { cwd })).body.lane.id;
|
||||
|
||||
@@ -0,0 +1,88 @@
|
||||
/**
|
||||
* @file self-restart.test.js
|
||||
* @description After an update, a server our service unit runs is restarted
|
||||
* by the unit, never by a detached replacement: v2.7.2 on the S3 system unit
|
||||
* left the replacement holding the port while `Restart=always` started the
|
||||
* unit every 3 s, each start exiting "already running".
|
||||
* @author Nguyễn Ngọc Trí Vĩ <vinnt@smartgift.vn>
|
||||
*/
|
||||
|
||||
const { describe, it } = require("node:test");
|
||||
const assert = require("node:assert/strict");
|
||||
const { scheduleRestart, owningUnit } = require("../lib/self-restart");
|
||||
|
||||
function deps(unit, restartAnswer = "restarted") {
|
||||
const calls = { restart: [], kill: 0, spawn: 0 };
|
||||
return {
|
||||
calls,
|
||||
unit,
|
||||
svc: () => ({
|
||||
restartUserService: (name) => {
|
||||
calls.restart.push(name);
|
||||
return restartAnswer;
|
||||
},
|
||||
}),
|
||||
kill: () => calls.kill++,
|
||||
spawnHelper: () => {
|
||||
calls.spawn++;
|
||||
return { unref() {} };
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
describe("self-restart", () => {
|
||||
it("tells which unit owns this process, from its cgroup or launchd label", () => {
|
||||
const linux = (text) => owningUnit({ platform: "linux", cgroup: () => text });
|
||||
assert.equal(linux("0::/system.slice/agenthub.service\n"), "systemd");
|
||||
assert.equal(
|
||||
linux("0::/user.slice/user-1001.slice/user@1001.service/app.slice/agenthub.service\n"),
|
||||
"systemd"
|
||||
);
|
||||
assert.equal(linux("0::/user.slice/user-1001.slice/session-3.scope\n"), null);
|
||||
assert.equal(linux("0::/system.slice/agenthub.service.d/x\n"), null);
|
||||
assert.equal(
|
||||
owningUnit({
|
||||
platform: "linux",
|
||||
cgroup: () => {
|
||||
throw new Error("no /proc");
|
||||
},
|
||||
}),
|
||||
null
|
||||
);
|
||||
assert.equal(
|
||||
owningUnit({ platform: "darwin", env: { XPC_SERVICE_NAME: "io.smartgift.agenthub" } }),
|
||||
"launchd"
|
||||
);
|
||||
assert.equal(owningUnit({ platform: "darwin", env: {} }), null);
|
||||
});
|
||||
|
||||
it("asks systemd to restart its unit and starts no replacement", () => {
|
||||
const d = deps("systemd");
|
||||
assert.equal(scheduleRestart("/r", d), "service");
|
||||
assert.deepEqual(d.calls.restart, ["agenthub"]);
|
||||
assert.equal(d.calls.spawn, 0);
|
||||
assert.equal(d.calls.kill, 0, "systemd's restart job stops this process");
|
||||
});
|
||||
|
||||
it("exits for Restart=always when systemd cannot be asked — still no replacement", () => {
|
||||
const d = deps("systemd", "failed");
|
||||
assert.equal(scheduleRestart("/r", d), "exit");
|
||||
assert.equal(d.calls.kill, 1);
|
||||
assert.equal(d.calls.spawn, 0);
|
||||
});
|
||||
|
||||
it("exits under launchd, whose KeepAlive starts the new code", () => {
|
||||
const d = deps("launchd");
|
||||
assert.equal(scheduleRestart("/r", d), "exit");
|
||||
assert.equal(d.calls.kill, 1);
|
||||
assert.equal(d.calls.spawn, 0);
|
||||
assert.deepEqual(d.calls.restart, []);
|
||||
});
|
||||
|
||||
it("hands over to the detached helper when no unit owns the server", () => {
|
||||
const d = deps(null);
|
||||
assert.equal(scheduleRestart("/r", d), "helper");
|
||||
assert.equal(d.calls.spawn, 1);
|
||||
assert.equal(d.calls.kill, 1);
|
||||
});
|
||||
});
|
||||
@@ -248,4 +248,20 @@ describe("run-as: a path under the users root belongs to that user", () => {
|
||||
"/opt/agenthub/libexec/write-as"
|
||||
);
|
||||
});
|
||||
it("counts a member's tmux it cannot stat as maybe there, a missing one as none", () => {
|
||||
const home = path.join(TMP, "users", "zoe");
|
||||
const acct = { name: "zoe", login: "ah-zoe", home, uid: 5002 };
|
||||
assert.equal(runAs.mayHaveTmux(acct), false, "no .agenthub at all");
|
||||
const dir = path.join(home, ".agenthub", "tmux");
|
||||
fs.mkdirSync(dir, { recursive: true });
|
||||
assert.equal(runAs.mayHaveTmux(acct), false, "no socket yet");
|
||||
// The member's own 0700 dir, seen by a dashboard without CAP_DAC_READ_SEARCH.
|
||||
fs.chmodSync(dir, 0o000);
|
||||
try {
|
||||
if (process.getuid() !== 0)
|
||||
assert.equal(runAs.mayHaveTmux(acct), true, "EACCES proves nothing");
|
||||
} finally {
|
||||
fs.chmodSync(dir, 0o700);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
+24
-12
@@ -133,13 +133,22 @@ function memberHomes() {
|
||||
function membersWithTmux() {
|
||||
return memberHomes()
|
||||
.map((home) => accountForPath(home))
|
||||
.filter((acct) => {
|
||||
try {
|
||||
return fs.statSync(tmuxSocket(acct)).isSocket();
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
});
|
||||
.filter(mayHaveTmux);
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether the member may have a tmux server. Only a missing socket says no:
|
||||
* a member's `.agenthub/tmux` is theirs (0700), and a dashboard without
|
||||
* CAP_DAC_READ_SEARCH (a dev server on the same database) gets EACCES there
|
||||
* — that proves nothing, and reading it as "no server" ended every run of
|
||||
* theirs within seconds. Their list is asked through sudo, which can tell.
|
||||
*/
|
||||
function mayHaveTmux(acct) {
|
||||
try {
|
||||
return fs.statSync(tmuxSocket(acct)).isSocket();
|
||||
} catch (err) {
|
||||
return err.code !== "ENOENT" && err.code !== "ENOTDIR";
|
||||
}
|
||||
}
|
||||
|
||||
// uid by login, from /etc/passwd (world-readable; node has no getpwnam).
|
||||
@@ -357,8 +366,7 @@ function openSafe(file) {
|
||||
// at no cost): MEMBER_READ_MAX unless the caller passes its own `max`, and
|
||||
// never past the size `fstat` gave — a file growing under the read is cut.
|
||||
const MEMBER_READ_MAX = 64 * 1024 * 1024;
|
||||
const tooBig = (file) =>
|
||||
Object.assign(new Error(`${file}: too big to read`), { code: "ETOOBIG" });
|
||||
const tooBig = (file) => Object.assign(new Error(`${file}: too big to read`), { code: "ETOOBIG" });
|
||||
|
||||
/** `fs.readFileSync(file, enc)`, guarded (a member's file: ≤ `max` bytes). */
|
||||
function readFileSafe(file, enc, { max = MEMBER_READ_MAX } = {}) {
|
||||
@@ -369,7 +377,7 @@ function readFileSafe(file, enc, { max = MEMBER_READ_MAX } = {}) {
|
||||
if (size > max) throw tooBig(file);
|
||||
const buf = Buffer.alloc(size);
|
||||
let off = 0;
|
||||
for (let n; off < size && (n = fs.readSync(fd, buf, off, size - off, off)) > 0; ) off += n;
|
||||
for (let n; off < size && (n = fs.readSync(fd, buf, off, size - off, off)) > 0;) off += n;
|
||||
return enc ? buf.toString(enc, 0, off) : buf.subarray(0, off);
|
||||
} finally {
|
||||
fs.closeSync(fd);
|
||||
@@ -398,7 +406,7 @@ async function readFileSafeAsync(file, enc, { max = MEMBER_READ_MAX } = {}) {
|
||||
if (size > max) throw tooBig(file);
|
||||
const buf = Buffer.alloc(size);
|
||||
let off = 0;
|
||||
for (let n; off < size && (n = (await fh.read(buf, off, size - off, off)).bytesRead) > 0; )
|
||||
for (let n; off < size && (n = (await fh.read(buf, off, size - off, off)).bytesRead) > 0;)
|
||||
off += n;
|
||||
return enc ? buf.toString(enc, 0, off) : buf.subarray(0, off);
|
||||
} finally {
|
||||
@@ -438,7 +446,10 @@ async function* readLines(file, { maxLine = 64 * 1024 * 1024 } = {}) {
|
||||
let start = 0;
|
||||
for (let i = view.indexOf(0x0a); i !== -1; i = view.indexOf(0x0a, start)) {
|
||||
if (skipping || partLen + (i - start) > maxLine) yield "";
|
||||
else yield text(partLen ? Buffer.concat([...parts, view.subarray(start, i)]) : view.subarray(start, i));
|
||||
else
|
||||
yield text(
|
||||
partLen ? Buffer.concat([...parts, view.subarray(start, i)]) : view.subarray(start, i)
|
||||
);
|
||||
parts = [];
|
||||
partLen = 0;
|
||||
skipping = false;
|
||||
@@ -502,6 +513,7 @@ module.exports = {
|
||||
assertOwnerFits,
|
||||
hasLinuxAccount,
|
||||
membersWithTmux,
|
||||
mayHaveTmux,
|
||||
memberHomes,
|
||||
ownerForRead,
|
||||
tmuxSocket,
|
||||
|
||||
@@ -7,26 +7,85 @@
|
||||
* it, and hands the "wait for the old PID to actually die, then spawn the
|
||||
* replacement" job to a detached helper script — the dying process can't do
|
||||
* that itself without racing its own replacement for the port.
|
||||
*
|
||||
* A server our service unit runs (systemd `agenthub.service`, system or user;
|
||||
* the launchd job) is restarted BY that manager instead: a detached
|
||||
* replacement would hold the port while `Restart=always` starts the unit
|
||||
* again every few seconds, each start exiting "already running" — forever.
|
||||
* @author Nguyễn Ngọc Trí Vĩ <vinnt@smartgift.vn>
|
||||
*/
|
||||
|
||||
const fs = require("fs");
|
||||
const path = require("path");
|
||||
const { spawn } = require("child_process");
|
||||
|
||||
const UNIT = "agenthub";
|
||||
|
||||
/**
|
||||
* "systemd" when this process lives in our unit's cgroup, "launchd" when it
|
||||
* is our LaunchAgent job, else null. Pure reads: no process is spawned.
|
||||
*/
|
||||
function owningUnit({
|
||||
platform = process.platform,
|
||||
cgroup = () => fs.readFileSync("/proc/self/cgroup", "utf8"),
|
||||
env = process.env,
|
||||
} = {}) {
|
||||
if (platform === "darwin") {
|
||||
return env.XPC_SERVICE_NAME === `io.smartgift.${UNIT}` ? "launchd" : null;
|
||||
}
|
||||
try {
|
||||
return cgroup()
|
||||
.split("\n")
|
||||
.some((l) => l.trim().endsWith(`/${UNIT}.service`))
|
||||
? "systemd"
|
||||
: null;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* @param {string} [root] repo root the helper should run from.
|
||||
* @returns {"service"|"exit"|"helper"} how the restart was handed over.
|
||||
*/
|
||||
function scheduleRestart(root = path.join(__dirname, "..", "..")) {
|
||||
function scheduleRestart(root = path.join(__dirname, "..", ".."), deps = {}) {
|
||||
const {
|
||||
unit = owningUnit(),
|
||||
svc = () => require("../../scripts/service-manager"),
|
||||
kill = () => process.kill(process.pid, "SIGTERM"),
|
||||
spawnHelper = spawn,
|
||||
} = deps;
|
||||
// systemd: one non-blocking restart job (sudo -n for the system unit). If
|
||||
// that cannot be asked, a plain exit still comes back — the unit is
|
||||
// Restart=always — but never with a second server beside it.
|
||||
if (unit === "systemd") {
|
||||
let restarted = false;
|
||||
try {
|
||||
restarted = svc().restartUserService(UNIT) === "restarted";
|
||||
} catch {
|
||||
/* fall through to the exit */
|
||||
}
|
||||
if (restarted) return "service";
|
||||
kill();
|
||||
return "exit";
|
||||
}
|
||||
// launchd: KeepAlive restarts the job on exit; a `bootout` from inside the
|
||||
// job would wait for this very process.
|
||||
if (unit === "launchd") {
|
||||
kill();
|
||||
return "exit";
|
||||
}
|
||||
const helperPath = path.join(root, "scripts", "restart-helper.js");
|
||||
const entry = process.argv[1] || path.join(root, "server", "index.js");
|
||||
const helper = spawn(process.execPath, [helperPath, String(process.pid), entry], {
|
||||
const helper = spawnHelper(process.execPath, [helperPath, String(process.pid), entry], {
|
||||
cwd: root,
|
||||
detached: true,
|
||||
stdio: "ignore",
|
||||
env: process.env,
|
||||
});
|
||||
helper.unref();
|
||||
process.kill(process.pid, "SIGTERM");
|
||||
kill();
|
||||
return "helper";
|
||||
}
|
||||
|
||||
module.exports = { scheduleRestart };
|
||||
module.exports = { scheduleRestart, owningUnit };
|
||||
|
||||
+7
-2
@@ -226,16 +226,21 @@ function listOutput(acct = null) {
|
||||
}
|
||||
|
||||
// A member's list is never waited for: a read answers from `listed` and,
|
||||
// once that is LIST_TTL_MS old, starts one async re-list. Until a first
|
||||
// once that is MEMBER_LIST_TTL_MS old, starts one async re-list. Until a first
|
||||
// answer — or once the last is MEMBER_LIST_MAX_AGE_MS old (nobody polled) —
|
||||
// or while their server fails, it throws: their panes are UNKNOWN. A session
|
||||
// this process creates or kills is written into the answer at once (`noteSession`).
|
||||
const MEMBER_LIST_MAX_AGE_MS = 15_000; // > the 10 s heal beat
|
||||
// A member's list is a `sudo` (fork + PAM session + 3 auth.log lines) — at a
|
||||
// 1 s TTL every open dashboard re-asked each member ~30 times a minute. A pane
|
||||
// this process starts or ends is corrected at once (`noteSession`); only one
|
||||
// that exits on its own reads live for up to these 5 s.
|
||||
const MEMBER_LIST_TTL_MS = 5_000;
|
||||
const refreshing = new Set(); // member logins with a list in flight
|
||||
function memberListOutput(acct) {
|
||||
const hit = listed.get(acct.login);
|
||||
const age = hit ? Date.now() - hit.at : Infinity;
|
||||
if (age >= LIST_TTL_MS) refreshList(acct);
|
||||
if (age >= MEMBER_LIST_TTL_MS) refreshList(acct);
|
||||
if (age >= MEMBER_LIST_MAX_AGE_MS) {
|
||||
throw Object.assign(new Error(`${acct.login}: tmux list pending`), { code: "ETMUXPENDING" });
|
||||
}
|
||||
|
||||
@@ -2060,6 +2060,22 @@ router.post("/:id/:action", sameOriginGuard, async (req, res) => {
|
||||
try {
|
||||
switch (action) {
|
||||
case "start": {
|
||||
// A folder that is gone (or never was) would start the pane in the
|
||||
// account's home instead, under this workspace's name. EACCES is not
|
||||
// "gone": a member's folder may be unreadable to this process.
|
||||
try {
|
||||
if (!fs.statSync(lane.cwd).isDirectory())
|
||||
throw Object.assign(new Error(), { code: "ENOTDIR" });
|
||||
} catch (err) {
|
||||
if (err.code === "ENOENT" || err.code === "ENOTDIR") {
|
||||
return res.status(409).json({
|
||||
error: {
|
||||
code: "ENOCWD",
|
||||
message: `the workspace folder does not exist: ${lane.cwd}`,
|
||||
},
|
||||
});
|
||||
}
|
||||
}
|
||||
// `"auto"`: the first free tab, picked under the lane lock below — a
|
||||
// client's own pick comes from a list that may already be stale.
|
||||
const auto = body.slot === "auto";
|
||||
|
||||
Reference in New Issue
Block a user