fix(gateway): bound service reads in status (#117636)

* fix(gateway): bound status service reads

* fix(gateway): preserve service probe errors
This commit is contained in:
Peter Steinberger
2026-08-01 14:38:40 -07:00
committed by GitHub
parent 47bb0378eb
commit f620c19a93
4 changed files with 109 additions and 22 deletions

View File

@@ -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<typeof import("../../infra/ports.js").inspectPortConnections>
@@ -103,10 +105,15 @@ const inspectWindowsGatewayFirewall = vi.fn<(opts?: unknown) => Promise<unknown>
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<boolean>
>(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<GatewayRestartSnapshot>>(
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<typeof import("../../daemon/launchd.js")>()),
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<typeof import("../../daemon/service.js")>()),
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<boolean>(() => {});
}
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);

View File

@@ -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<DaemonStatus> {
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

View File

@@ -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 }));

View File

@@ -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,