From f620c19a936ed49417470597c421047067247d8a Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 1 Aug 2026 14:38:40 -0700 Subject: [PATCH] fix(gateway): bound service reads in status (#117636) * fix(gateway): bound status service reads * fix(gateway): preserve service probe errors --- src/cli/daemon-cli/status.gather.test.ts | 83 ++++++++++++++++++++++-- src/cli/daemon-cli/status.gather.ts | 23 ++----- src/daemon/service.test.ts | 17 +++++ src/daemon/service.ts | 8 ++- 4 files changed, 109 insertions(+), 22 deletions(-) diff --git a/src/cli/daemon-cli/status.gather.test.ts b/src/cli/daemon-cli/status.gather.test.ts index 91474f315563..4d89d8f7d214 100644 --- a/src/cli/daemon-cli/status.gather.test.ts +++ b/src/cli/daemon-cli/status.gather.test.ts @@ -7,10 +7,12 @@ import type { StaleOpenClawUpdateLaunchdJob } from "../../daemon/launchd.js"; import { createMockGatewayService } from "../../daemon/service.test-helpers.js"; import type { PortListener, PortUsageStatus } from "../../infra/ports.js"; import type { GatewayRestartHandoff } from "../../infra/restart-handoff.js"; +import { defaultRuntime } from "../../runtime.js"; import { captureEnv, deleteTestEnvValue, setTestEnvValue } from "../../test-utils/env.js"; import { VERSION } from "../../version.js"; import type { GatewayRestartSnapshot } from "./restart-health.js"; import { gatherDaemonStatus } from "./status.gather.js"; +import { printDaemonStatus } from "./status.print.js"; type PortConnections = Awaited< ReturnType @@ -103,10 +105,15 @@ const inspectWindowsGatewayFirewall = vi.fn<(opts?: unknown) => Promise details: [], })); const auditGatewayServiceConfig = vi.fn(async (_opts?: unknown) => undefined); -const serviceIsLoaded = vi.fn(async (_opts?: unknown) => true); +const serviceIsLoaded = vi.fn< + (opts?: { env?: NodeJS.ProcessEnv; timeoutMs?: number }) => Promise +>(async (_opts?: { env?: NodeJS.ProcessEnv; timeoutMs?: number }) => true); const serviceReadRuntime = vi.fn< - (_env?: NodeJS.ProcessEnv) => Promise<{ status: string; detail?: string }> ->(async (_env?: NodeJS.ProcessEnv) => ({ status: "running" })); + ( + _env?: NodeJS.ProcessEnv, + _opts?: { timeoutMs?: number }, + ) => Promise<{ status: string; detail?: string }> +>(async (_env?: NodeJS.ProcessEnv, _opts?: { timeoutMs?: number }) => ({ status: "running" })); const inspectGatewayRestart = vi.fn<(opts?: unknown) => Promise>( async (_opts?: unknown) => ({ runtime: { status: "running", pid: 1234 }, @@ -210,7 +217,8 @@ vi.mock("../../daemon/inspect.js", () => ({ findExtraGatewayServices: (env: unknown, opts?: unknown) => findExtraGatewayServices(env, opts), })); -vi.mock("../../daemon/launchd.js", () => ({ +vi.mock("../../daemon/launchd.js", async (importOriginal) => ({ + ...(await importOriginal()), findStaleOpenClawUpdateLaunchdJobs: (env?: NodeJS.ProcessEnv) => findStaleOpenClawUpdateLaunchdJobs(env), })); @@ -219,7 +227,8 @@ vi.mock("../../daemon/service-audit.js", () => ({ auditGatewayServiceConfig: (opts: unknown) => auditGatewayServiceConfig(opts), })); -vi.mock("../../daemon/service.js", () => ({ +vi.mock("../../daemon/service.js", async (importOriginal) => ({ + ...(await importOriginal()), resolveGatewayService: () => createMockGatewayService({ isLoaded: serviceIsLoaded, @@ -374,6 +383,9 @@ describe("gatherDaemonStatus", () => { readLastGatewayErrorLine.mockReset(); readLastGatewayErrorLine.mockResolvedValue(null); readGatewayRestartHandoffSync.mockClear(); + serviceIsLoaded.mockClear(); + serviceReadCommand.mockClear(); + serviceReadRuntime.mockClear(); readConfigFileSnapshotCalls.mockClear(); loadConfigCalls.mockClear(); daemonConfigWarnings = []; @@ -640,6 +652,67 @@ describe("gatherDaemonStatus", () => { expect((status.service.runtime as { detail?: string }).detail).toBe("19001"); }); + it("bounds both service-manager reads and still emits JSON after they time out", async () => { + serviceIsLoaded.mockImplementationOnce(async (args?: { timeoutMs?: number }) => { + if (args?.timeoutMs === undefined) { + return await new Promise(() => {}); + } + throw new Error("systemctl is-enabled timed out"); + }); + serviceReadRuntime.mockImplementationOnce(async (_env, opts) => { + if (opts?.timeoutMs === undefined) { + return await new Promise<{ status: string }>(() => {}); + } + throw new Error("systemctl show timed out"); + }); + + const status = await gatherDaemonStatus({ + rpc: { timeout: "100", json: true }, + probe: false, + deep: true, + }); + + expect(serviceIsLoaded).toHaveBeenCalledWith(expect.objectContaining({ timeoutMs: 100 })); + expect(serviceReadRuntime).toHaveBeenCalledWith(expect.any(Object), { timeoutMs: 100 }); + expect(status.service.loaded).toBe(false); + expect(status.service.runtime).toEqual({ + status: "unknown", + detail: "Error: systemctl show timed out", + }); + + const writeJson = vi.spyOn(defaultRuntime, "writeJson").mockImplementation(() => {}); + try { + printDaemonStatus(status, { json: true, deep: true }); + expect(writeJson).toHaveBeenCalledOnce(); + const serialized = JSON.stringify(writeJson.mock.calls[0]?.[0]); + if (!serialized) { + throw new Error("expected terminal JSON output"); + } + expect(JSON.parse(serialized)).toMatchObject({ + service: { + loaded: false, + runtime: { + status: "unknown", + detail: "Error: systemctl show timed out", + }, + }, + }); + } finally { + writeJson.mockRestore(); + } + + const log = vi.spyOn(defaultRuntime, "log").mockImplementation(() => {}); + const error = vi.spyOn(defaultRuntime, "error").mockImplementation(() => {}); + try { + printDaemonStatus(status, { json: false, deep: true }); + const output = log.mock.calls.flat().join("\n"); + expect(output).toContain("Runtime: unknown (Error: systemctl show timed out)"); + } finally { + log.mockRestore(); + error.mockRestore(); + } + }, 1_000); + it("keeps gateway status read-only when service management is unsupported", async () => { serviceReadCommand.mockResolvedValueOnce(null); serviceIsLoaded.mockResolvedValueOnce(false); diff --git a/src/cli/daemon-cli/status.gather.ts b/src/cli/daemon-cli/status.gather.ts index 23b3f68b9fc6..7e54bf0c4ae9 100644 --- a/src/cli/daemon-cli/status.gather.ts +++ b/src/cli/daemon-cli/status.gather.ts @@ -21,7 +21,7 @@ import type { ExtraGatewayService, FindExtraGatewayServicesOptions } from "../.. import type { StaleOpenClawUpdateLaunchdJob } from "../../daemon/launchd.js"; import type { ServiceConfigAudit } from "../../daemon/service-audit.js"; import type { GatewayServiceRuntime } from "../../daemon/service-runtime.js"; -import { resolveGatewayService } from "../../daemon/service.js"; +import { readGatewayServiceState, resolveGatewayService } from "../../daemon/service.js"; import { resolveAdvertisedControlUiLinks } from "../../gateway/control-ui-links.js"; import { gatewaySecretInputPathCanWin } from "../../gateway/credentials-secret-inputs.js"; import { trimToUndefined } from "../../gateway/credentials.js"; @@ -578,20 +578,13 @@ export async function gatherDaemonStatus( allowExecSecretRefs?: boolean; } & FindExtraGatewayServicesOptions, ): Promise { + const timeoutMs = parseStrictPositiveInteger(opts.rpc.timeout ?? undefined) ?? 10_000; const service = resolveGatewayService(); - 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((err: unknown) => ({ status: "unknown", detail: String(err) })), - ]); + const serviceState = await readGatewayServiceState(service, { + env: process.env, + timeoutMs, + }); + const { command, env: serviceEnv, loaded, runtime } = serviceState; const restartHandoff = opts.deep ? readGatewayRestartHandoffSync(serviceEnv) : null; const configAudit: ServiceConfigAudit = command ? await loadServiceAuditModule().then(({ auditGatewayServiceConfig }) => @@ -655,8 +648,6 @@ export async function gatherDaemonStatus( .catch(() => []) : []; - const timeoutMs = parseStrictPositiveInteger(opts.rpc.timeout ?? undefined) ?? 10_000; - const tlsEnabled = daemonCfg.gateway?.tls?.enabled === true; const shouldUseLocalTlsRuntime = opts.probe && !probeUrlOverride && tlsEnabled; const tlsRuntime = shouldUseLocalTlsRuntime diff --git a/src/daemon/service.test.ts b/src/daemon/service.test.ts index ae9c36bdbb56..de0a99771b2c 100644 --- a/src/daemon/service.test.ts +++ b/src/daemon/service.test.ts @@ -186,6 +186,23 @@ describe("readGatewayServiceState", () => { ); }); + it("preserves runtime probe failures as an explicit unknown state", async () => { + const service = createService({ + isLoaded: vi.fn(async () => true), + readRuntime: vi.fn(async () => { + throw new Error("systemctl show timed out"); + }), + }); + + const state = await readGatewayServiceState(service, { timeoutMs: 100 }); + + expect(state.running).toBe(false); + expect(state.runtime).toEqual({ + status: "unknown", + detail: "Error: systemctl show timed out", + }); + }); + it("validates merged service env before native status probes", async () => { const isLoaded = vi.fn(async () => true); const readRuntime = vi.fn(async () => ({ status: "running" as const })); diff --git a/src/daemon/service.ts b/src/daemon/service.ts index 4ea791652ee2..e33b4e6030e7 100644 --- a/src/daemon/service.ts +++ b/src/daemon/service.ts @@ -196,7 +196,13 @@ export async function readGatewayServiceState( // bound; isLoaded/readRuntime can spawn service-manager subprocesses. const [loaded, runtime] = await Promise.all([ service.isLoaded({ env, timeoutMs: args.timeoutMs }).catch(() => false), - service.readRuntime(env, { timeoutMs: args.timeoutMs }).catch(() => undefined), + service.readRuntime(env, { timeoutMs: args.timeoutMs }).catch( + (error: unknown) => + ({ + status: "unknown", + detail: String(error), + }) satisfies GatewayServiceRuntime, + ), ]); return { installed: command !== null,