mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-25 17:45:53 +01:00
fix(cli): harden service lifecycle edge cases
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
c18d1d378f
commit
ed4eaf6f46
8 files changed
+72
-22
No files matched your search
@@ -79,8 +79,20 @@ describe("managed install commands", () => {
|
||||
const userData = path.join(process.env.PAPERCLIP_HOME!, "instances", "default", "keep.txt");
|
||||
fs.mkdirSync(path.dirname(userData), { recursive: true });
|
||||
fs.writeFileSync(userData, "keep");
|
||||
await uninstallCommand();
|
||||
const uninstallService = vi.fn(async () => {
|
||||
expect(fs.existsSync(paths.shimPath)).toBe(true);
|
||||
});
|
||||
await uninstallCommand({
|
||||
detectServiceManager: vi.fn(async () => ({
|
||||
supported: true as const,
|
||||
manager: {
|
||||
status: vi.fn(async () => ({ installed: true, active: true })),
|
||||
uninstall: uninstallService,
|
||||
} as never,
|
||||
})),
|
||||
});
|
||||
|
||||
expect(uninstallService).toHaveBeenCalledOnce();
|
||||
expect(fs.existsSync(paths.cliRoot)).toBe(false);
|
||||
expect(fs.existsSync(paths.shimPath)).toBe(false);
|
||||
expect(fs.readFileSync(userData, "utf8")).toBe("keep");
|
||||
|
||||
@@ -5,6 +5,7 @@ import { describe, expect, it, vi } from "vitest";
|
||||
import { serviceHealthChecks } from "../checks/service-health-check.js";
|
||||
import { resolveRestartExpectedVersion } from "../commands/service.js";
|
||||
import type { PaperclipConfig } from "../config/schema.js";
|
||||
import { buildLocalHealthUrl } from "../utils/health-url.js";
|
||||
import { packageVersion } from "../version.js";
|
||||
|
||||
const config = {
|
||||
@@ -40,11 +41,16 @@ function managerFixture(active = true) {
|
||||
}
|
||||
|
||||
describe("service health doctor checks", () => {
|
||||
it("uses the bare package version when restart health has no prior server version", () => {
|
||||
it("uses the current CLI version unless an expected restart version is explicit", () => {
|
||||
expect(resolveRestartExpectedVersion(null)).toBe(packageVersion);
|
||||
expect(resolveRestartExpectedVersion("1.2.3")).toBe("1.2.3");
|
||||
});
|
||||
|
||||
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");
|
||||
});
|
||||
|
||||
it("passes for a current, active, healthy service", async () => {
|
||||
const manager = managerFixture();
|
||||
const results = await serviceHealthChecks(config, {
|
||||
|
||||
@@ -104,6 +104,20 @@ describe("launchd lifecycle", () => {
|
||||
expect(calls).toContain(`launchctl bootout gui/${process.getuid?.() ?? 0}/ing.paperclip.paperclipai.team-a`);
|
||||
expect(calls.some((call) => call.includes("launchctl kill"))).toBe(false);
|
||||
});
|
||||
|
||||
it("disables login startup when uninstalled", async () => {
|
||||
const userHome = await temporaryDirectory();
|
||||
const calls: string[] = [];
|
||||
const runner: CommandRunner = async (command, args) => {
|
||||
calls.push([command, ...args].join(" "));
|
||||
return { stdout: "", stderr: "" };
|
||||
};
|
||||
const manager = new LaunchdServiceManager("team-a", runner, path.join(userHome, ".paperclip"), path.join(userHome, ".local/bin/paperclipai"), userHome);
|
||||
|
||||
await manager.uninstall();
|
||||
|
||||
expect(calls).toContain(`launchctl disable gui/${process.getuid?.() ?? 0}/ing.paperclip.paperclipai.team-a`);
|
||||
});
|
||||
});
|
||||
|
||||
describe("single-writer guard", () => {
|
||||
|
||||
@@ -6,6 +6,7 @@ import {
|
||||
detectServiceManager,
|
||||
type ServiceManagerDetection,
|
||||
} from "../services/service-manager.js";
|
||||
import { buildLocalHealthUrl } from "../utils/health-url.js";
|
||||
import type { CheckResult } from "./index.js";
|
||||
|
||||
type HealthResult = { ok: boolean; version: string | null; error?: string };
|
||||
@@ -14,17 +15,11 @@ type ServiceCheckDependencies = {
|
||||
probe: (config: PaperclipConfig) => Promise<HealthResult>;
|
||||
};
|
||||
|
||||
function healthUrl(config: PaperclipConfig): string {
|
||||
const configuredHost = config.server.host?.trim();
|
||||
const host = !configuredHost || configuredHost === "0.0.0.0" || configuredHost === "::"
|
||||
? "127.0.0.1"
|
||||
: configuredHost;
|
||||
return `http://${host}:${config.server.port}/api/health`;
|
||||
}
|
||||
|
||||
async function probeHealth(config: PaperclipConfig): Promise<HealthResult> {
|
||||
try {
|
||||
const response = await fetch(healthUrl(config), { signal: AbortSignal.timeout(2_000) });
|
||||
const response = await fetch(buildLocalHealthUrl(config.server.host, config.server.port), {
|
||||
signal: AbortSignal.timeout(2_000),
|
||||
});
|
||||
const body = (await response.json()) as {
|
||||
status?: unknown;
|
||||
serverVersion?: unknown;
|
||||
|
||||
@@ -6,6 +6,7 @@ import { packageVersion } from "../version.js";
|
||||
import { readConfig, resolveConfigPath } from "../config/store.js";
|
||||
import { resolvePaperclipHomeDir, resolvePaperclipInstanceId } from "../config/home.js";
|
||||
import { detectServiceManager, type ServiceManager, type ServiceStatus } from "../services/service-manager.js";
|
||||
import { buildLocalHealthUrl } from "../utils/health-url.js";
|
||||
|
||||
type CommonOptions = { instance?: string; json?: boolean };
|
||||
type HealthResult = { ok: boolean; serverVersion: string | null; error?: string };
|
||||
@@ -26,10 +27,7 @@ async function resolveManager(opts: CommonOptions): Promise<ServiceManager | nul
|
||||
function healthUrl(instanceId: string): string {
|
||||
process.env.PAPERCLIP_INSTANCE_ID = instanceId;
|
||||
const config = readConfig(resolveConfigPath());
|
||||
const port = config?.server.port ?? 3100;
|
||||
const configuredHost = config?.server.host?.trim();
|
||||
const host = !configuredHost || configuredHost === "0.0.0.0" || configuredHost === "::" ? "127.0.0.1" : configuredHost;
|
||||
return `http://${host}:${port}/api/health`;
|
||||
return buildLocalHealthUrl(config?.server.host, config?.server.port ?? 3100);
|
||||
}
|
||||
|
||||
async function probeHealth(instanceId: string): Promise<HealthResult> {
|
||||
@@ -53,11 +51,11 @@ async function waitForHealth(instanceId: string, expectedVersion: string | null,
|
||||
throw new Error(`Paperclip service did not become healthy${expectedVersion ? ` at version ${expectedVersion}` : ""}: ${last.error ?? `reported ${last.serverVersion ?? "no version"}`}`);
|
||||
}
|
||||
|
||||
export function resolveRestartExpectedVersion(serverVersion: string | null): string {
|
||||
return serverVersion ?? packageVersion;
|
||||
export function resolveRestartExpectedVersion(expectedVersion: string | null | undefined): string {
|
||||
return expectedVersion ?? packageVersion;
|
||||
}
|
||||
|
||||
async function writeHotRestartIntent(status: ServiceStatus, instanceId: string, drainRequired: boolean): Promise<{ requestedAt: string; previousVersion: string }> {
|
||||
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);
|
||||
const homeDir = resolvePaperclipHomeDir();
|
||||
@@ -72,7 +70,7 @@ async function writeHotRestartIntent(status: ServiceStatus, instanceId: string,
|
||||
drainRequired,
|
||||
requestedByRunId: process.env.PAPERCLIP_RUN_ID?.trim() || null,
|
||||
}, null, 2)}\n`, "utf8");
|
||||
return { requestedAt, previousVersion: resolveRestartExpectedVersion(health.serverVersion) };
|
||||
return { requestedAt };
|
||||
}
|
||||
|
||||
async function waitForRestartReport(requestedAt: string, timeoutMs = 10_000): Promise<unknown | null> {
|
||||
@@ -97,7 +95,7 @@ export async function restartManagedService(input: { instanceId?: string; expect
|
||||
const before = await detection.manager.status();
|
||||
const intent = await writeHotRestartIntent(before, instanceId, input.waitForDrain ?? false);
|
||||
await detection.manager.restart();
|
||||
const health = await waitForHealth(instanceId, input.expectedVersion ?? intent.previousVersion);
|
||||
const health = await waitForHealth(instanceId, resolveRestartExpectedVersion(input.expectedVersion));
|
||||
return { status: await detection.manager.status(), health, report: await waitForRestartReport(intent.requestedAt) };
|
||||
}
|
||||
|
||||
|
||||
@@ -6,8 +6,23 @@ import {
|
||||
removeManagedShim,
|
||||
resolveInstallStorePaths,
|
||||
} from "../install-store.js";
|
||||
import { resolvePaperclipInstanceId } from "../config/home.js";
|
||||
import { detectServiceManager } from "../services/service-manager.js";
|
||||
|
||||
type UninstallDependencies = {
|
||||
detectServiceManager: typeof detectServiceManager;
|
||||
};
|
||||
|
||||
export async function uninstallCommand(
|
||||
dependencies: Partial<UninstallDependencies> = {},
|
||||
): Promise<void> {
|
||||
const detect = dependencies.detectServiceManager ?? detectServiceManager;
|
||||
const detection = await detect({ instanceId: resolvePaperclipInstanceId() });
|
||||
if (detection.supported) {
|
||||
const status = await detection.manager.status();
|
||||
if (status.installed || status.active) await detection.manager.uninstall();
|
||||
}
|
||||
|
||||
export async function uninstallCommand(): Promise<void> {
|
||||
const paths = resolveInstallStorePaths();
|
||||
const shimRemoved = removeManagedShim(paths);
|
||||
fs.rmSync(paths.cliRoot, { recursive: true, force: true });
|
||||
|
||||
@@ -230,7 +230,7 @@ export class LaunchdServiceManager implements ServiceManager {
|
||||
|
||||
async uninstall(): Promise<void> {
|
||||
await this.runner("launchctl", ["bootout", `${this.domain}/${this.serviceName}`]).catch(() => undefined);
|
||||
await this.runner("launchctl", ["enable", `${this.domain}/${this.serviceName}`]).catch(() => undefined);
|
||||
await this.runner("launchctl", ["disable", `${this.domain}/${this.serviceName}`]).catch(() => undefined);
|
||||
await fs.rm(this.definitionPath, { force: true });
|
||||
}
|
||||
async start(): Promise<void> { await this.install({ startNow: true, startOnLogin: true }); }
|
||||
|
||||
@@ -0,0 +1,10 @@
|
||||
export function buildLocalHealthUrl(host: string | undefined, port: number): string {
|
||||
const configuredHost = host?.trim();
|
||||
const reachableHost = !configuredHost || configuredHost === "0.0.0.0" || configuredHost === "::"
|
||||
? "127.0.0.1"
|
||||
: configuredHost;
|
||||
const urlHost = reachableHost.includes(":") && !reachableHost.startsWith("[")
|
||||
? `[${reachableHost}]`
|
||||
: reachableHost;
|
||||
return `http://${urlHost}:${port}/api/health`;
|
||||
}
|
||||
Reference in new issue
Block a user