diff --git a/src/cli/daemon-cli/lifecycle.test.ts b/src/cli/daemon-cli/lifecycle.test.ts index f24de9b589aa..5cc8cad1aac4 100644 --- a/src/cli/daemon-cli/lifecycle.test.ts +++ b/src/cli/daemon-cli/lifecycle.test.ts @@ -53,6 +53,7 @@ const probeGateway = vi.fn< const callGatewayCli = vi.fn(); const isRestartEnabled = vi.fn<(config?: { commands?: unknown }) => boolean>(() => true); const loadConfig = vi.hoisted(() => vi.fn(() => ({}))); +const readActiveGatewayLockPort = vi.hoisted(() => vi.fn<() => Promise>()); const recoverInstalledLaunchAgent = vi.hoisted(() => vi.fn()); const repairLoadedGatewayServiceForStart = vi.hoisted(() => vi.fn()); const findInstalledSystemdGatewayScope = vi.hoisted(() => @@ -103,6 +104,10 @@ vi.mock("../../infra/gateway-processes.js", () => ({ formatGatewayPidList: (pids: number[]) => formatGatewayPidList(pids), })); +vi.mock("../../infra/gateway-lock.js", () => ({ + readActiveGatewayLockPort: () => readActiveGatewayLockPort(), +})); + vi.mock("../../gateway/probe.js", () => ({ probeGateway: (opts: { url: string; @@ -214,6 +219,7 @@ describe("runDaemonRestart health checks", () => { callGatewayCli.mockReset(); isRestartEnabled.mockReset(); loadConfig.mockReset(); + readActiveGatewayLockPort.mockReset(); recoverInstalledLaunchAgent.mockReset(); repairLoadedGatewayServiceForStart.mockReset(); @@ -224,6 +230,7 @@ describe("runDaemonRestart health checks", () => { service.restart.mockResolvedValue({ outcome: "completed" }); runServiceStart.mockResolvedValue(undefined); recoverInstalledLaunchAgent.mockResolvedValue(null); + readActiveGatewayLockPort.mockResolvedValue(undefined); findInstalledSystemdGatewayScope.mockReset(); findInstalledSystemdGatewayScope.mockResolvedValue(null); restartSystemdService.mockReset(); @@ -575,6 +582,26 @@ describe("runDaemonRestart health checks", () => { expect(service.restart).not.toHaveBeenCalled(); }); + it("signals and verifies the active unmanaged port despite a config edit", async () => { + loadConfig.mockReturnValue({ gateway: { port: 19_001 } }); + readActiveGatewayLockPort.mockResolvedValue(18_789); + findVerifiedGatewayListenerPidsOnPortSync.mockImplementation((port) => + port === 18_789 ? [4200] : [], + ); + mockUnmanagedRestart({ runPostRestartCheck: true }); + + await runDaemonRestart({ json: true }); + + expect(findVerifiedGatewayListenerPidsOnPortSync).toHaveBeenCalledWith(18_789); + expect(probeGateway).toHaveBeenCalledWith( + expect.objectContaining({ url: "ws://127.0.0.1:18789" }), + ); + expect(signalVerifiedGatewayPidSync).toHaveBeenCalledWith(4200, "SIGUSR1"); + expect(waitForGatewayHealthyListener).toHaveBeenCalledWith( + expect.objectContaining({ port: 18_789 }), + ); + }); + it("prefers launchd repair over unmanaged restart when an installed LaunchAgent is unloaded", async () => { vi.spyOn(process, "platform", "get").mockReturnValue("darwin"); recoverInstalledLaunchAgent.mockResolvedValue({ diff --git a/src/cli/daemon-cli/lifecycle.ts b/src/cli/daemon-cli/lifecycle.ts index af4e717db8b0..ee02191bfda0 100644 --- a/src/cli/daemon-cli/lifecycle.ts +++ b/src/cli/daemon-cli/lifecycle.ts @@ -12,6 +12,7 @@ import { } from "../../daemon/systemd.js"; import { callGatewayCli } from "../../gateway/call.js"; import { probeGateway } from "../../gateway/probe.js"; +import { readActiveGatewayLockPort } from "../../infra/gateway-lock.js"; import { findVerifiedGatewayListenerPidsOnPortSync, formatGatewayPidList, @@ -334,9 +335,14 @@ export async function runDaemonRestart(opts: DaemonLifecycleOptions = {}): Promi let restartedWithoutServiceManager = false; const restartIntent = resolveGatewayRestartIntentOptions(opts); const configuredPort = await resolveExplicitGatewayConfigPort(); - const restartPort = + const managedRestartPort = configuredPort ?? (await resolveGatewayLifecyclePort(service).catch(() => resolveGatewayPortFallback())); + // An unmanaged run loop keeps its lock port across in-process restarts, even + // when config changes underneath it. Use that port for both the signal and + // health proof or a valid CLI/env override looks like a failed restart. + const unmanagedPort = + (await readActiveGatewayLockPort().catch(() => undefined)) ?? managedRestartPort; const restartHealthAttempts = postRestartHealthAttempts(); const restartWaitMs = restartHealthAttempts * POST_RESTART_HEALTH_DELAY_MS; const restartWaitSeconds = Math.round(restartWaitMs / 1000); @@ -369,7 +375,7 @@ export async function runDaemonRestart(opts: DaemonLifecycleOptions = {}): Promi return recovered; } } - const handled = await restartGatewayWithoutServiceManager(restartPort, restartIntent); + const handled = await restartGatewayWithoutServiceManager(unmanagedPort, restartIntent); if (handled) { restartedWithoutServiceManager = true; return handled; @@ -380,7 +386,7 @@ export async function runDaemonRestart(opts: DaemonLifecycleOptions = {}): Promi if (restartedWithoutServiceManager) { // SIGUSR1 restarts have no service-manager state to watch; use listener health only. const health = await waitForGatewayHealthyListener({ - port: restartPort, + port: unmanagedPort, attempts: restartHealthAttempts, delayMs: POST_RESTART_HEALTH_DELAY_MS, }); @@ -389,7 +395,7 @@ export async function runDaemonRestart(opts: DaemonLifecycleOptions = {}): Promi } const diagnostics = renderGatewayPortHealthDiagnostics(health); - const timeoutLine = `Timed out after ${restartWaitSeconds}s waiting for gateway port ${restartPort} to become healthy.`; + const timeoutLine = `Timed out after ${restartWaitSeconds}s waiting for gateway port ${unmanagedPort} to become healthy.`; if (!jsonOutput) { defaultRuntime.log(theme.warn(timeoutLine)); for (const line of diagnostics) { @@ -409,7 +415,7 @@ export async function runDaemonRestart(opts: DaemonLifecycleOptions = {}): Promi let health = await waitForGatewayHealthyRestart({ service, - port: restartPort, + port: managedRestartPort, attempts: restartHealthAttempts, delayMs: POST_RESTART_HEALTH_DELAY_MS, includeUnknownListenersAsStale: process.platform === "win32", @@ -432,7 +438,7 @@ export async function runDaemonRestart(opts: DaemonLifecycleOptions = {}): Promi } health = await waitForGatewayHealthyRestart({ service, - port: restartPort, + port: managedRestartPort, attempts: restartHealthAttempts, delayMs: POST_RESTART_HEALTH_DELAY_MS, includeUnknownListenersAsStale: process.platform === "win32", @@ -446,12 +452,12 @@ export async function runDaemonRestart(opts: DaemonLifecycleOptions = {}): Promi const diagnostics = renderRestartDiagnostics(health); const failure = formatRestartFailure({ health, - port: restartPort, + port: managedRestartPort, timeoutSeconds: restartWaitSeconds, }); const runningNoPortLine = health.runtime.status === "running" && health.portUsage.status === "free" - ? `Gateway process is running but port ${restartPort} is still free (startup hang/crash loop or very slow VM startup).` + ? `Gateway process is running but port ${managedRestartPort} is still free (startup hang/crash loop or very slow VM startup).` : null; if (!jsonOutput) { defaultRuntime.log(theme.warn(failure.statusLine)); diff --git a/src/crestodian/operations.test.ts b/src/crestodian/operations.test.ts index 670c5f238f41..f293f970ef15 100644 --- a/src/crestodian/operations.test.ts +++ b/src/crestodian/operations.test.ts @@ -559,6 +559,24 @@ describe("parseCrestodianOperation", () => { expect(runGatewayRestart).not.toHaveBeenCalled(); }); + it("does not report or audit a gateway restart that returned false", async () => { + const tempDir = opTempDirs.make("crestodian-restart-failed-"); + setTestEnvValue("OPENCLAW_STATE_DIR", tempDir); + const { runtime, lines } = createCrestodianTestRuntime(); + const runGatewayRestart = vi.fn(async () => false); + + await expect( + executeCrestodianOperation({ kind: "gateway-restart" }, runtime, { + approved: true, + deps: { runGatewayRestart }, + }), + ).rejects.toThrow("Gateway restart did not complete"); + + expect(lines.join("\n")).toContain("[crestodian] running: gateway.restart"); + expect(lines.join("\n")).not.toContain("[crestodian] done: gateway.restart"); + await expect(fs.access(path.join(tempDir, "audit", "crestodian.jsonl"))).rejects.toThrow(); + }); + it("validates missing config without exiting the process", async () => { mockConfig.missing("/tmp/openclaw.json"); const { runtime, lines } = createCrestodianTestRuntime(); diff --git a/src/crestodian/operations.ts b/src/crestodian/operations.ts index 8306e177ad37..84f6db2c9a60 100644 --- a/src/crestodian/operations.ts +++ b/src/crestodian/operations.ts @@ -114,7 +114,7 @@ export type CrestodianCommandDeps = { cliOptions: ConfigSetOptions; }) => Promise; runDoctor?: (runtime: RuntimeEnv, options: DoctorOptions) => Promise; - runGatewayRestart?: () => Promise; + runGatewayRestart?: () => Promise; runGatewayStart?: () => Promise; runGatewayStop?: () => Promise; runPluginInstall?: (spec: string, runtime: RuntimeEnv) => Promise; @@ -560,7 +560,9 @@ function formatGatewayStatusLine(overview: CrestodianOverview): string { .join("\n"); } -async function runGatewayLifecycle(operation: "start" | "stop" | "restart"): Promise { +async function runGatewayLifecycle( + operation: "start" | "stop" | "restart", +): Promise { const lifecycle = await import("../cli/daemon-cli/lifecycle.js"); if (operation === "start") { await lifecycle.runDaemonStart(); @@ -570,7 +572,7 @@ async function runGatewayLifecycle(operation: "start" | "stop" | "restart"): Pro await lifecycle.runDaemonStop(); return; } - await lifecycle.runDaemonRestart(); + return await lifecycle.runDaemonRestart(); } async function readConfigFileSnapshotLazy(): Promise { @@ -1493,7 +1495,10 @@ export async function executeCrestodianOperation( run: async (ctx) => { const runGatewayRestart = ctx.deps?.runGatewayRestart ?? (() => runGatewayLifecycle("restart")); - await ctx.commit(runGatewayRestart); + const restarted = await ctx.commit(runGatewayRestart); + if (restarted === false) { + throw new Error("Gateway restart did not complete"); + } return { summary: "Restarted Gateway" }; }, });