From 8a7d9c1040dba60a73cdf4abd2552b279942cc1f Mon Sep 17 00:00:00 2001 From: Dotta Date: Wed, 22 Jul 2026 20:07:23 +0000 Subject: [PATCH] fix(cli): serialize same-instance restarts Co-Authored-By: Paperclip --- .../__tests__/service-health-check.test.ts | 38 ++++++++++++- cli/src/commands/service.ts | 57 ++++++++++++++++--- 2 files changed, 86 insertions(+), 9 deletions(-) diff --git a/cli/src/__tests__/service-health-check.test.ts b/cli/src/__tests__/service-health-check.test.ts index 83f5f8e7658..934633bd4d0 100644 --- a/cli/src/__tests__/service-health-check.test.ts +++ b/cli/src/__tests__/service-health-check.test.ts @@ -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((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"); diff --git a/cli/src/commands/service.ts b/cli/src/commands/service.ts index 5740ad5fed3..d5142bcf7e9 100644 --- a/cli/src/commands/service.ts +++ b/cli/src/commands/service.ts @@ -54,6 +54,47 @@ export function resolveRestartExpectedVersion(expectedVersion: string | null | u return expectedVersion ?? null; } +export async function withHotRestartLock( + instanceId: string, + callback: () => Promise, + options: { timeoutMs?: number; pollMs?: number } = {}, +): Promise { + 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 {