fix(cli): serialize same-instance restarts

Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
DottaandPaperclip committed 2026-07-27 15:32:13 +00:00
1 parent 38dadbf5ba
commit 8a7d9c1040
2 files changed
+86 -9

No files matched your search

+36 -2
View File
@@ -1,9 +1,9 @@
import fs from "node:fs";
import os from "node:os";
import path from "node:path";
import { describe, expect, it, vi } from "vitest";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { serviceHealthChecks } from "../checks/service-health-check.js";
import { resolveRestartExpectedVersion } from "../commands/service.js";
import { resolveRestartExpectedVersion, withHotRestartLock } from "../commands/service.js";
import type { PaperclipConfig } from "../config/schema.js";
import { buildLocalHealthUrl } from "../utils/health-url.js";
@@ -11,6 +11,18 @@ const config = {
server: { host: "127.0.0.1", port: 3100 },
} as PaperclipConfig;
let previousPaperclipHome: string | undefined;
beforeEach(() => {
previousPaperclipHome = process.env.PAPERCLIP_HOME;
process.env.PAPERCLIP_HOME = fs.mkdtempSync(path.join(os.tmpdir(), "paperclip-service-restart-"));
});
afterEach(() => {
if (previousPaperclipHome === undefined) delete process.env.PAPERCLIP_HOME;
else process.env.PAPERCLIP_HOME = previousPaperclipHome;
});
function managerFixture(active = true) {
const root = fs.mkdtempSync(path.join(os.tmpdir(), "paperclip-service-doctor-"));
const definitionPath = path.join(root, "paperclipai.service");
@@ -46,6 +58,28 @@ describe("service health doctor checks", () => {
expect(resolveRestartExpectedVersion("1.2.3")).toBe("1.2.3");
});
it("serializes concurrent restarts for the same instance", async () => {
const order: string[] = [];
let releaseFirst!: () => void;
const firstBlocked = new Promise<void>((resolve) => { releaseFirst = resolve; });
const first = withHotRestartLock("default", async () => {
order.push("first-start");
await firstBlocked;
order.push("first-end");
}, { pollMs: 5 });
await vi.waitFor(() => expect(order).toEqual(["first-start"]));
const second = withHotRestartLock("default", async () => {
order.push("second-start");
}, { pollMs: 5 });
await new Promise((resolve) => setTimeout(resolve, 20));
expect(order).toEqual(["first-start"]);
releaseFirst();
await Promise.all([first, second]);
expect(order).toEqual(["first-start", "first-end", "second-start"]);
});
it("brackets configured IPv6 hosts in health URLs", () => {
expect(buildLocalHealthUrl("::1", 3100)).toBe("http://[::1]:3100/api/health");
expect(buildLocalHealthUrl("::", 3100)).toBe("http://127.0.0.1:3100/api/health");
+50 -7
View File
@@ -54,6 +54,47 @@ export function resolveRestartExpectedVersion(expectedVersion: string | null | u
return expectedVersion ?? null;
}
export async function withHotRestartLock<T>(
instanceId: string,
callback: () => Promise<T>,
options: { timeoutMs?: number; pollMs?: number } = {},
): Promise<T> {
const instanceRoot = resolvePaperclipInstanceRoot(instanceId);
const lockPath = path.join(instanceRoot, "hot-restart.lock");
const token = `${process.pid}:${Date.now()}:${Math.random().toString(16).slice(2)}`;
const deadline = Date.now() + (options.timeoutMs ?? 120_000);
const pollMs = options.pollMs ?? 100;
await fs.mkdir(instanceRoot, { recursive: true });
while (true) {
try {
await fs.writeFile(lockPath, `${token}\n`, { encoding: "utf8", mode: 0o600, flag: "wx" });
break;
} catch (error) {
if ((error as NodeJS.ErrnoException).code !== "EEXIST") throw error;
if (Date.now() >= deadline) {
throw new Error(
`Another restart for instance ${instanceId} is still running. ` +
`If no restart process is active, remove the stale lock at ${lockPath} and retry.`,
);
}
await new Promise((resolve) => setTimeout(resolve, pollMs));
}
}
try {
return await callback();
} finally {
try {
if ((await fs.readFile(lockPath, "utf8")).trim() === token) {
await fs.rm(lockPath, { force: true });
}
} catch (error) {
if ((error as NodeJS.ErrnoException).code !== "ENOENT") throw error;
}
}
}
async function writeHotRestartIntent(status: ServiceStatus, instanceId: string, drainRequired: boolean): Promise<{ requestedAt: string }> {
if (!status.pid) throw new Error(`Cannot restart ${status.serviceName}: supervisor did not report a server pid.`);
const health = await probeHealth(instanceId);
@@ -89,13 +130,15 @@ async function waitForRestartReport(instanceId: string, requestedAt: string, tim
export async function restartManagedService(input: { instanceId?: string; expectedVersion?: string | null; waitForDrain?: boolean } = {}): Promise<{ status: ServiceStatus; health: HealthResult; report: unknown | null }> {
const instanceId = resolvePaperclipInstanceId(input.instanceId);
const detection = await detectServiceManager({ instanceId });
if (!detection.supported) throw new Error(detection.reason);
const before = await detection.manager.status();
const intent = await writeHotRestartIntent(before, instanceId, input.waitForDrain ?? false);
await detection.manager.restart();
const health = await waitForHealth(instanceId, resolveRestartExpectedVersion(input.expectedVersion));
return { status: await detection.manager.status(), health, report: await waitForRestartReport(instanceId, intent.requestedAt) };
return withHotRestartLock(instanceId, async () => {
const detection = await detectServiceManager({ instanceId });
if (!detection.supported) throw new Error(detection.reason);
const before = await detection.manager.status();
const intent = await writeHotRestartIntent(before, instanceId, input.waitForDrain ?? false);
await detection.manager.restart();
const health = await waitForHealth(instanceId, resolveRestartExpectedVersion(input.expectedVersion));
return { status: await detection.manager.status(), health, report: await waitForRestartReport(instanceId, intent.requestedAt) };
});
}
export function registerServiceCommands(program: Command): void {