From 1ace6a0d6a0dc764834253aca9220ea8507a0634 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 3 May 2026 21:39:41 +0100 Subject: [PATCH] fix: avoid launchd kickstart after fresh bootstrap --- src/cli/update-cli/restart-helper.test.ts | 4 ++- src/cli/update-cli/restart-helper.ts | 13 ++++++--- src/daemon/launchd-restart-handoff.test.ts | 5 +++- src/daemon/launchd-restart-handoff.ts | 9 ++---- src/daemon/launchd.test.ts | 2 +- src/daemon/launchd.ts | 15 ++++------ src/infra/restart.test.ts | 32 ++++++++++++++++++++++ src/infra/restart.ts | 5 +++- 8 files changed, 62 insertions(+), 23 deletions(-) diff --git a/src/cli/update-cli/restart-helper.test.ts b/src/cli/update-cli/restart-helper.test.ts index 77f6b6b605d..79b3e7d1153 100644 --- a/src/cli/update-cli/restart-helper.test.ts +++ b/src/cli/update-cli/restart-helper.test.ts @@ -194,6 +194,7 @@ exit 1 // Should clear disabled state and fall back to bootstrap when kickstart fails. expect(content).toContain("launchctl enable 'gui/501/ai.openclaw.gateway'"); expect(content).toContain("launchctl bootstrap 'gui/501'"); + expect(content).toContain("Bootstrap loads RunAtLoad agents"); expect(content).toContain('rm -f "$0"'); await cleanupScript(scriptPath); }); @@ -250,7 +251,8 @@ exit 1 echo "launchctl $*" >&2 case "$1" in kickstart) exit 42 ;; - enable|bootstrap) exit 0 ;; + enable) exit 0 ;; + bootstrap) exit 1 ;; esac exit 0 `, diff --git a/src/cli/update-cli/restart-helper.ts b/src/cli/update-cli/restart-helper.ts index c6370122214..7e76228f627 100644 --- a/src/cli/update-cli/restart-helper.ts +++ b/src/cli/update-cli/restart-helper.ts @@ -137,14 +137,19 @@ ${logSetup} printf '[%s] openclaw restart attempt source=update target=%s\\n' "$(date -u +%FT%TZ)" '${shellEscapeRestartLogValue(label)}' >&2 # Try kickstart first (works when the service is still registered). # If it fails (e.g. after bootout), clear any persisted disabled state, -# then re-register via bootstrap and kickstart. The final status is captured +# then re-register via bootstrap. Bootstrap loads RunAtLoad agents, so the +# fallback must not immediately kickstart -k the freshly spawned gateway. +# The final status is captured # before self-cleanup so a genuine failure remains observable. status=0 if ! launchctl kickstart -k 'gui/${uid}/${escaped}'; then launchctl enable 'gui/${uid}/${escaped}' - launchctl bootstrap 'gui/${uid}' '${escapedPlistPath}' - launchctl kickstart -k 'gui/${uid}/${escaped}' - status=$? + if launchctl bootstrap 'gui/${uid}' '${escapedPlistPath}'; then + status=0 + else + launchctl kickstart -k 'gui/${uid}/${escaped}' + status=$? + fi fi if [ "$status" -eq 0 ]; then printf '[%s] openclaw restart done source=update\\n' "$(date -u +%FT%TZ)" >&2 diff --git a/src/daemon/launchd-restart-handoff.test.ts b/src/daemon/launchd-restart-handoff.test.ts index b1356c72af8..ac30996875e 100644 --- a/src/daemon/launchd-restart-handoff.test.ts +++ b/src/daemon/launchd-restart-handoff.test.ts @@ -45,6 +45,9 @@ describe("scheduleDetachedLaunchdRestartHandoff", () => { expect(args[1]).toContain("openclaw restart attempt source=launchd-handoff mode=kickstart"); expect(args[1]).toContain('launchctl enable "$service_target"'); expect(args[1]).toContain('if launchctl kickstart -k "$service_target"; then'); + expect(args[1]).toContain( + 'if launchctl bootstrap "$domain" "$plist_path"; then\n status=0\n else\n launchctl kickstart -k "$service_target"', + ); expect(args[1]).not.toMatch(/launchctl[^\n]*\/dev\/null/); expect(args[1]).not.toContain("sleep 1"); expect(unrefMock).toHaveBeenCalledTimes(1); @@ -68,7 +71,7 @@ describe("scheduleDetachedLaunchdRestartHandoff", () => { expect(args[1]).toContain("print_retry_count=$((print_retry_count - 1))"); expect(args[1]).toContain("sleep 0.2"); expect(args[1]).toContain('if launchctl bootstrap "$domain" "$plist_path"; then'); - expect(args[1]).toContain('if launchctl start "$label"; then'); + expect(args[1]).not.toContain('if launchctl start "$label"; then'); expect(args[1]).not.toContain('basename "$service_target"'); }); diff --git a/src/daemon/launchd-restart-handoff.ts b/src/daemon/launchd-restart-handoff.ts index b33988fb3e9..fb105cae5e5 100644 --- a/src/daemon/launchd-restart-handoff.ts +++ b/src/daemon/launchd-restart-handoff.ts @@ -135,6 +135,8 @@ if launchctl kickstart -k "$service_target"; then else status=$? if launchctl bootstrap "$domain" "$plist_path"; then + status=0 + else launchctl kickstart -k "$service_target" status=$? fi @@ -168,12 +170,7 @@ ${verifyLaunchdReload} status=0 launchctl enable "$service_target" if launchctl bootstrap "$domain" "$plist_path"; then - if launchctl start "$label"; then - status=0 - else - launchctl kickstart -k "$service_target" - status=$? - fi + status=0 else status=$? launchctl kickstart -k "$service_target" diff --git a/src/daemon/launchd.test.ts b/src/daemon/launchd.test.ts index d9bd69c6025..78fcf92510b 100644 --- a/src/daemon/launchd.test.ts +++ b/src/daemon/launchd.test.ts @@ -830,7 +830,7 @@ describe("launchd install", () => { expect(result).toEqual({ outcome: "completed" }); expect(state.launchctlCalls.some((call) => call[0] === "enable")).toBe(true); expect(state.launchctlCalls.some((call) => call[0] === "bootstrap")).toBe(true); - expect(kickstartCalls).toHaveLength(2); + expect(kickstartCalls).toHaveLength(1); expect(state.launchctlCalls.some((call) => call[0] === "bootout")).toBe(false); }); diff --git a/src/daemon/launchd.ts b/src/daemon/launchd.ts index 0d864222e45..a79f748a4ff 100644 --- a/src/daemon/launchd.ts +++ b/src/daemon/launchd.ts @@ -437,6 +437,11 @@ type LaunchAgentBootstrapRepairResult = | { ok: true; status: "repaired" | "already-loaded" } | { ok: false; status: "bootstrap-failed" | "kickstart-failed"; detail?: string }; +function isLaunchctlAlreadyLoaded(res: { stdout: string; stderr: string; code: number }): boolean { + const detail = normalizeLowercaseStringOrEmpty(res.stderr || res.stdout); + return res.code === 130 || detail.includes("already exists in domain"); +} + export async function repairLaunchAgentBootstrap(args: { env?: Record; }): Promise { @@ -450,9 +455,7 @@ export async function repairLaunchAgentBootstrap(args: { let repairStatus: "repaired" | "already-loaded" = "repaired"; if (boot.code !== 0) { const detail = (boot.stderr || boot.stdout).trim(); - const normalized = normalizeLowercaseStringOrEmpty(detail); - const alreadyLoaded = boot.code === 130 || normalized.includes("already exists in domain"); - if (!alreadyLoaded) { + if (!isLaunchctlAlreadyLoaded(boot)) { return { ok: false, status: "bootstrap-failed", detail: detail || undefined }; } repairStatus = "already-loaded"; @@ -850,12 +853,6 @@ export async function restartLaunchAgent({ plistPath, actionHint: "openclaw gateway restart", }); - - const retry = await execLaunchctl(["kickstart", "-k", serviceTarget]); - if (retry.code !== 0) { - await ensureLaunchAgentLoadedAfterFailure({ domain, serviceTarget, plistPath }); - throw new Error(`launchctl kickstart failed: ${retry.stderr || retry.stdout}`.trim()); - } writeLaunchAgentActionLine(stdout, "Restarted LaunchAgent", serviceTarget); return { outcome: "completed" }; } diff --git a/src/infra/restart.test.ts b/src/infra/restart.test.ts index bee3936117f..02b02a9a7cb 100644 --- a/src/infra/restart.test.ts +++ b/src/infra/restart.test.ts @@ -184,6 +184,38 @@ describe.runIf(process.platform !== "win32")("cleanStaleGatewayProcessesSync", ( }); describe("triggerOpenClawRestart", () => { + it("does not kickstart after bootstrap registers an unloaded LaunchAgent", () => { + setPlatform("darwin"); + delete process.env.VITEST; + delete process.env.NODE_ENV; + process.env.HOME = "/Users/test"; + process.env.OPENCLAW_PROFILE = "default"; + const uid = typeof process.getuid === "function" ? process.getuid() : 501; + spawnSyncMock.mockImplementation((command: string, args: string[]) => { + if (command === "/usr/sbin/lsof") { + return { error: undefined, status: 1, stdout: "" }; + } + if (command === "launchctl" && args[0] === "kickstart" && args[1] === "-k") { + return { error: undefined, status: 113, stderr: "service not loaded" }; + } + if (command === "launchctl" && args[0] === "bootstrap") { + return { error: undefined, status: 0, stderr: "" }; + } + return { error: undefined, status: 1, stdout: "" }; + }); + + const result = triggerOpenClawRestart(); + + expect(result).toEqual({ + ok: true, + method: "launchctl", + tried: [ + `launchctl kickstart -k gui/${uid}/ai.openclaw.gateway`, + `launchctl bootstrap gui/${uid} /Users/test/Library/LaunchAgents/ai.openclaw.gateway.plist`, + ], + }); + }); + it("continues when launchctl bootstrap reports the service is already loaded", () => { setPlatform("darwin"); delete process.env.VITEST; diff --git a/src/infra/restart.ts b/src/infra/restart.ts index 6fdcbd37580..bf8b7fbd7c3 100644 --- a/src/infra/restart.ts +++ b/src/infra/restart.ts @@ -640,7 +640,7 @@ export function triggerOpenClawRestart(): RestartAttempt { } // kickstart fails when the service was previously booted out (deregistered from launchd). - // Fall back to bootstrap (re-register from plist) + kickstart. + // Fall back to bootstrap, which loads RunAtLoad agents without a follow-up kickstart. // Use env HOME to match how launchd.ts resolves the plist install path. const home = process.env.HOME?.trim() || os.homedir(); const plistPath = path.join(home, "Library", "LaunchAgents", `${label}.plist`); @@ -663,6 +663,9 @@ export function triggerOpenClawRestart(): RestartAttempt { tried, }; } + if (boot.status === 0) { + return { ok: true, method: "launchctl", tried }; + } const retryArgs = ["kickstart", target]; tried.push(`launchctl ${retryArgs.join(" ")}`); const retry = spawnSync("launchctl", retryArgs, {