From 258a214bcbffa2fa043ade4c00befd204be053a6 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 24 Mar 2026 22:48:26 -0700 Subject: [PATCH] refactor: centralize daemon service start state flow --- src/cli/daemon-cli/lifecycle-core.test.ts | 31 ++++- src/cli/daemon-cli/lifecycle-core.ts | 115 ++++++++--------- .../test-helpers/lifecycle-core-harness.ts | 2 + src/commands/status.service-summary.ts | 24 ++-- src/daemon/service-types.ts | 16 +++ src/daemon/service.test.ts | 117 +++++++++++++++++- src/daemon/service.ts | 69 +++++++++++ 7 files changed, 290 insertions(+), 84 deletions(-) diff --git a/src/cli/daemon-cli/lifecycle-core.test.ts b/src/cli/daemon-cli/lifecycle-core.test.ts index 0fd4fd81f82..ddf7e32c67f 100644 --- a/src/cli/daemon-cli/lifecycle-core.test.ts +++ b/src/cli/daemon-cli/lifecycle-core.test.ts @@ -205,9 +205,38 @@ describe("runServiceRestart token drift", () => { opts: { json: true }, }); - expect(service.isLoaded).toHaveBeenCalledTimes(1); + expect(service.isLoaded).toHaveBeenCalled(); const payload = readJsonLog<{ result?: string; message?: string }>(); expect(payload.result).toBe("scheduled"); expect(payload.message).toBe("restart scheduled, gateway will restart momentarily"); }); + + it("fails start when restarting a stopped installed service errors", async () => { + service.isLoaded.mockResolvedValue(false); + service.restart.mockRejectedValue(new Error("launchctl kickstart failed: permission denied")); + + await expect(runServiceStart(createServiceRunArgs())).rejects.toThrow("__exit__:1"); + + const payload = readJsonLog<{ ok?: boolean; error?: string }>(); + expect(payload.ok).toBe(false); + expect(payload.error).toContain("launchctl kickstart failed: permission denied"); + }); + + it("falls back to not-loaded hints when start finds no install artifacts", async () => { + service.isLoaded.mockResolvedValue(false); + service.readCommand.mockResolvedValue(null); + + await runServiceStart({ + serviceNoun: "Gateway", + service, + renderStartHints: () => ["openclaw gateway install"], + opts: { json: true }, + }); + + const payload = readJsonLog<{ ok?: boolean; result?: string; hints?: string[] }>(); + expect(payload.ok).toBe(true); + expect(payload.result).toBe("not-loaded"); + expect(payload.hints).toEqual(["openclaw gateway install"]); + expect(service.restart).not.toHaveBeenCalled(); + }); }); diff --git a/src/cli/daemon-cli/lifecycle-core.ts b/src/cli/daemon-cli/lifecycle-core.ts index 10cfc0240c8..004d6ba705c 100644 --- a/src/cli/daemon-cli/lifecycle-core.ts +++ b/src/cli/daemon-cli/lifecycle-core.ts @@ -4,7 +4,7 @@ import { formatConfigIssueLines } from "../../config/issue-format.js"; import { resolveIsNixMode } from "../../config/paths.js"; import { checkTokenDrift } from "../../daemon/service-audit.js"; import type { GatewayServiceRestartResult } from "../../daemon/service-types.js"; -import { describeGatewayServiceRestart } from "../../daemon/service.js"; +import { describeGatewayServiceRestart, startGatewayService } from "../../daemon/service.js"; import type { GatewayService } from "../../daemon/service.js"; import { renderSystemdUnavailableHints } from "../../daemon/systemd-hints.js"; import { isSystemdUserServiceAvailable } from "../../daemon/systemd.js"; @@ -77,6 +77,17 @@ function createActionIO(params: { action: DaemonAction; json: boolean }) { return { stdout, emit, fail }; } +function emitActionMessage(params: { + json: boolean; + emit: ReturnType["emit"]; + payload: Omit; +}) { + params.emit(params.payload); + if (!params.json && params.payload.message) { + defaultRuntime.log(params.payload.message); + } +} + async function handleServiceNotLoaded(params: { serviceNoun: string; service: GatewayService; @@ -200,12 +211,13 @@ export async function runServiceStart(params: { const json = Boolean(params.opts?.json); const { stdout, emit, fail } = createActionIO({ action: "start", json }); - const loaded = await resolveServiceLoadedOrFail({ - serviceNoun: params.serviceNoun, - service: params.service, - fail, - }); - if (loaded === null) { + if ( + (await resolveServiceLoadedOrFail({ + serviceNoun: params.serviceNoun, + service: params.service, + fail, + })) === null + ) { return; } // Pre-flight config validation (#35862) — run for both loaded and not-loaded @@ -219,71 +231,45 @@ export async function runServiceStart(params: { return; } } - if (!loaded) { - // Service was stopped (e.g. `gateway stop` booted out the LaunchAgent). - // Attempt a restart, which handles re-bootstrapping the service. Without - // this, `start` after `stop` just prints hints and does nothing (#53878). - try { - const restartResult = await params.service.restart({ env: process.env, stdout }); - const restartStatus = describeGatewayServiceRestart(params.serviceNoun, restartResult); - const postLoaded = await params.service.isLoaded({ env: process.env }).catch(() => true); - emit({ - ok: true, - result: restartStatus.daemonActionResult, - message: restartStatus.message, - service: buildDaemonServiceSnapshot(params.service, postLoaded), - }); - if (!json) { - defaultRuntime.log(restartStatus.message); - } - return; - } catch { - // Bootstrap failed (e.g. plist was deleted, not just booted out). - // Fall through to the not-loaded hints. + try { + const startResult = await startGatewayService(params.service, { env: process.env, stdout }); + if (startResult.outcome === "missing-install") { await handleServiceNotLoaded({ serviceNoun: params.serviceNoun, service: params.service, - loaded, + loaded: startResult.state.loaded, renderStartHints: params.renderStartHints, json, emit, }); return; } - } - - try { - const restartResult = await params.service.restart({ env: process.env, stdout }); - const restartStatus = describeGatewayServiceRestart(params.serviceNoun, restartResult); - if (restartStatus.scheduled) { - emit({ - ok: true, - result: restartStatus.daemonActionResult, - message: restartStatus.message, - service: buildDaemonServiceSnapshot(params.service, loaded), + if (startResult.outcome === "scheduled") { + const restartStatus = describeGatewayServiceRestart(params.serviceNoun, { + outcome: "scheduled", + }); + emitActionMessage({ + json, + emit, + payload: { + ok: true, + result: "scheduled", + message: restartStatus.message, + service: buildDaemonServiceSnapshot(params.service, startResult.state.loaded), + }, }); - if (!json) { - defaultRuntime.log(restartStatus.message); - } return; } + emit({ + ok: true, + result: "started", + service: buildDaemonServiceSnapshot(params.service, startResult.state.loaded), + }); } catch (err) { const hints = params.renderStartHints(); fail(`${params.serviceNoun} start failed: ${String(err)}`, hints); return; } - - let started = true; - try { - started = await params.service.isLoaded({ env: process.env }); - } catch { - started = true; - } - emit({ - ok: true, - result: "started", - service: buildDaemonServiceSnapshot(params.service, started), - }); } export async function runServiceStop(params: { @@ -371,16 +357,17 @@ export async function runServiceRestart(params: { restartStatus: ReturnType, serviceLoaded: boolean, ) => { - emit({ - ok: true, - result: restartStatus.daemonActionResult, - message: restartStatus.message, - service: buildDaemonServiceSnapshot(params.service, serviceLoaded), - warnings: warnings.length ? warnings : undefined, + emitActionMessage({ + json, + emit, + payload: { + ok: true, + result: restartStatus.daemonActionResult, + message: restartStatus.message, + service: buildDaemonServiceSnapshot(params.service, serviceLoaded), + warnings: warnings.length ? warnings : undefined, + }, }); - if (!json) { - defaultRuntime.log(restartStatus.message); - } return true; }; diff --git a/src/cli/daemon-cli/test-helpers/lifecycle-core-harness.ts b/src/cli/daemon-cli/test-helpers/lifecycle-core-harness.ts index 180751d5055..b7d4fdc7a62 100644 --- a/src/cli/daemon-cli/test-helpers/lifecycle-core-harness.ts +++ b/src/cli/daemon-cli/test-helpers/lifecycle-core-harness.ts @@ -42,9 +42,11 @@ export function resetLifecycleServiceMocks() { service.stage.mockClear(); service.isLoaded.mockClear(); service.readCommand.mockClear(); + service.readRuntime.mockClear(); service.restart.mockClear(); service.isLoaded.mockResolvedValue(true); service.readCommand.mockResolvedValue({ programArguments: [], environment: {} }); + service.readRuntime.mockResolvedValue({ status: "running" }); service.restart.mockResolvedValue({ outcome: "completed" }); } diff --git a/src/commands/status.service-summary.ts b/src/commands/status.service-summary.ts index cc366c2c7ba..d202c40fa41 100644 --- a/src/commands/status.service-summary.ts +++ b/src/commands/status.service-summary.ts @@ -1,5 +1,5 @@ import type { GatewayServiceRuntime } from "../daemon/service-runtime.js"; -import type { GatewayService } from "../daemon/service.js"; +import { readGatewayServiceState, type GatewayService } from "../daemon/service.js"; export type ServiceStatusSummary = { label: string; @@ -16,33 +16,23 @@ export async function readServiceStatusSummary( fallbackLabel: string, ): Promise { try { - const command = await service.readCommand(process.env).catch(() => null); - const serviceEnv = command?.environment - ? ({ - ...process.env, - ...command.environment, - } satisfies NodeJS.ProcessEnv) - : process.env; - const [loaded, runtime] = await Promise.all([ - service.isLoaded({ env: serviceEnv }).catch(() => false), - service.readRuntime(serviceEnv).catch(() => undefined), - ]); - const managedByOpenClaw = command != null; - const externallyManaged = !managedByOpenClaw && runtime?.status === "running"; + const state = await readGatewayServiceState(service, { env: process.env }); + const managedByOpenClaw = state.installed; + const externallyManaged = !managedByOpenClaw && state.running; const installed = managedByOpenClaw || externallyManaged; const loadedText = externallyManaged ? "running (externally managed)" - : loaded + : state.loaded ? service.loadedText : service.notLoadedText; return { label: service.label, installed, - loaded, + loaded: state.loaded, managedByOpenClaw, externallyManaged, loadedText, - runtime, + runtime: state.runtime, }; } catch { return { diff --git a/src/daemon/service-types.ts b/src/daemon/service-types.ts index 93ac69ddfe8..28d684d67ec 100644 --- a/src/daemon/service-types.ts +++ b/src/daemon/service-types.ts @@ -1,3 +1,5 @@ +import type { GatewayServiceRuntime } from "./service-runtime.js"; + export type GatewayServiceEnv = Record; export type GatewayServiceInstallArgs = { @@ -35,6 +37,20 @@ export type GatewayServiceCommandConfig = { sourcePath?: string; }; +export type GatewayServiceState = { + installed: boolean; + loaded: boolean; + running: boolean; + env: GatewayServiceEnv; + command: GatewayServiceCommandConfig | null; + runtime?: GatewayServiceRuntime; +}; + +export type GatewayServiceStartResult = + | { outcome: "started"; state: GatewayServiceState } + | { outcome: "scheduled"; state: GatewayServiceState } + | { outcome: "missing-install"; state: GatewayServiceState }; + export type GatewayServiceRenderArgs = { description?: string; programArguments: string[]; diff --git a/src/daemon/service.test.ts b/src/daemon/service.test.ts index ea2c53e8e1a..efbb62c0723 100644 --- a/src/daemon/service.test.ts +++ b/src/daemon/service.test.ts @@ -1,5 +1,11 @@ -import { afterEach, describe, expect, it } from "vitest"; -import { describeGatewayServiceRestart, resolveGatewayService } from "./service.js"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import type { GatewayService } from "./service.js"; +import { + describeGatewayServiceRestart, + readGatewayServiceState, + resolveGatewayService, + startGatewayService, +} from "./service.js"; const originalPlatformDescriptor = Object.getOwnPropertyDescriptor(process, "platform"); @@ -21,6 +27,23 @@ afterEach(() => { Object.defineProperty(process, "platform", originalPlatformDescriptor); }); +function createService(overrides: Partial = {}): GatewayService { + return { + label: "LaunchAgent", + loadedText: "loaded", + notLoadedText: "not loaded", + stage: vi.fn(async () => {}), + install: vi.fn(async () => {}), + uninstall: vi.fn(async () => {}), + stop: vi.fn(async () => {}), + restart: vi.fn(async () => ({ outcome: "completed" as const })), + isLoaded: vi.fn(async () => false), + readCommand: vi.fn(async () => null), + readRuntime: vi.fn(async () => ({ status: "stopped" as const })), + ...overrides, + }; +} + describe("resolveGatewayService", () => { it.each([ { platform: "darwin" as const, label: "LaunchAgent", loadedText: "loaded" }, @@ -47,3 +70,93 @@ describe("resolveGatewayService", () => { }); }); }); + +describe("readGatewayServiceState", () => { + it("tracks installed, loaded, and running separately", async () => { + const service = createService({ + isLoaded: vi.fn(async () => true), + readCommand: vi.fn(async () => ({ + programArguments: ["openclaw", "gateway", "run"], + environment: { OPENCLAW_GATEWAY_PORT: "18789" }, + })), + readRuntime: vi.fn(async () => ({ status: "running" })), + }); + + const state = await readGatewayServiceState(service, { + env: { OPENCLAW_GATEWAY_PORT: "1" }, + }); + + expect(state.installed).toBe(true); + expect(state.loaded).toBe(true); + expect(state.running).toBe(true); + expect(state.env.OPENCLAW_GATEWAY_PORT).toBe("18789"); + }); +}); + +describe("startGatewayService", () => { + it("returns missing-install without attempting restart", async () => { + const service = createService(); + + const result = await startGatewayService(service, { + env: {}, + stdout: process.stdout, + }); + + expect(result.outcome).toBe("missing-install"); + expect(service.restart).not.toHaveBeenCalled(); + }); + + it("restarts stopped installed services and returns post-start state", async () => { + const readCommand = vi.fn(async () => ({ + programArguments: ["openclaw", "gateway", "run"], + environment: { OPENCLAW_GATEWAY_PORT: "18789" }, + })); + const isLoaded = vi + .fn() + .mockResolvedValueOnce(false) + .mockResolvedValueOnce(true); + const readRuntime = vi + .fn() + .mockResolvedValueOnce({ status: "stopped" }) + .mockResolvedValueOnce({ status: "running" }); + const service = createService({ + readCommand, + isLoaded, + readRuntime, + }); + + const result = await startGatewayService(service, { + env: {}, + stdout: process.stdout, + }); + + expect(result.outcome).toBe("started"); + expect(service.restart).toHaveBeenCalledTimes(1); + expect(result.state.installed).toBe(true); + expect(result.state.loaded).toBe(true); + expect(result.state.running).toBe(true); + }); + + it("falls back to missing-install when restart fails and install artifacts are gone", async () => { + const readCommand = vi + .fn() + .mockResolvedValueOnce({ + programArguments: ["openclaw", "gateway", "run"], + }) + .mockResolvedValueOnce(null); + const service = createService({ + readCommand, + restart: vi.fn(async () => { + throw new Error("launchctl bootstrap failed"); + }), + }); + + const result = await startGatewayService(service, { + env: {}, + stdout: process.stdout, + }); + + expect(result.outcome).toBe("missing-install"); + expect(result.state.installed).toBe(false); + }); +}); diff --git a/src/daemon/service.ts b/src/daemon/service.ts index f6577d38d75..3df405e7add 100644 --- a/src/daemon/service.ts +++ b/src/daemon/service.ts @@ -27,7 +27,9 @@ import type { GatewayServiceInstallArgs, GatewayServiceManageArgs, GatewayServiceRestartResult, + GatewayServiceStartResult, GatewayServiceStageArgs, + GatewayServiceState, } from "./service-types.js"; import { installSystemdService, @@ -47,7 +49,9 @@ export type { GatewayServiceInstallArgs, GatewayServiceManageArgs, GatewayServiceRestartResult, + GatewayServiceStartResult, GatewayServiceStageArgs, + GatewayServiceState, } from "./service-types.js"; function ignoreServiceWriteResult( @@ -72,6 +76,71 @@ export type GatewayService = { readRuntime: (env: GatewayServiceEnv) => Promise; }; +function mergeGatewayServiceEnv( + baseEnv: GatewayServiceEnv, + command: GatewayServiceCommandConfig | null, +): GatewayServiceEnv { + if (!command?.environment) { + return baseEnv; + } + return { + ...baseEnv, + ...command.environment, + }; +} + +export async function readGatewayServiceState( + service: GatewayService, + args: GatewayServiceEnvArgs = {}, +): Promise { + const baseEnv = args.env ?? (process.env as GatewayServiceEnv); + const command = await service.readCommand(baseEnv).catch(() => null); + const env = mergeGatewayServiceEnv(baseEnv, command); + const [loaded, runtime] = await Promise.all([ + service.isLoaded({ env }).catch(() => false), + service.readRuntime(env).catch(() => undefined), + ]); + return { + installed: command !== null, + loaded, + running: runtime?.status === "running", + env, + command, + runtime, + }; +} + +export async function startGatewayService( + service: GatewayService, + args: GatewayServiceControlArgs, +): Promise { + const state = await readGatewayServiceState(service, { env: args.env }); + if (!state.loaded && !state.installed) { + return { + outcome: "missing-install", + state, + }; + } + + try { + const restartResult = await service.restart({ ...args, env: state.env }); + const nextState = await readGatewayServiceState(service, { env: state.env }); + return { + outcome: restartResult.outcome === "scheduled" ? "scheduled" : "started", + state: nextState, + }; + } catch (err) { + const nextState = await readGatewayServiceState(service, { env: state.env }); + if (!nextState.installed) { + return { + outcome: "missing-install", + state: nextState, + }; + } + throw err; + } +} + export function describeGatewayServiceRestart( serviceNoun: string, result: GatewayServiceRestartResult,