diff --git a/src/daemon/launchd-restart-handoff.test.ts b/src/daemon/launchd-restart-handoff.test.ts index 70ef5c67b8c9..f107677a2c46 100644 --- a/src/daemon/launchd-restart-handoff.test.ts +++ b/src/daemon/launchd-restart-handoff.test.ts @@ -1,4 +1,9 @@ // Launchd restart handoff tests cover restart coordination on macOS. +import { execFile } from "node:child_process"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { promisify } from "node:util"; import { afterEach, describe, expect, it, vi } from "vitest"; const spawnMock = vi.hoisted(() => vi.fn()); @@ -16,6 +21,8 @@ import { scheduleDetachedLaunchdRestartHandoff } from "./launchd-restart-handoff type SpawnCall = [string, string[], { env: Record }]; +const execFileAsync = promisify(execFile); + function requireSpawnCall(callIndex = 0): SpawnCall { const call = spawnMock.mock.calls[callIndex]; if (!call) { @@ -33,6 +40,78 @@ function requireSpawnCall(callIndex = 0): SpawnCall { return [command, args as string[], options as SpawnCall[2]]; } +async function executeReloadHandoff(launchctlStub: string): Promise<{ + calls: string[]; + exitCode: number; + log: string; +}> { + const noWaitPid = 0; + const stubDir = fs.mkdtempSync(path.join(os.tmpdir(), "launchd-stub-")); + try { + const home = path.join(stubDir, "home"); + const callsPath = path.join(stubDir, "launchctl.calls"); + fs.mkdirSync(path.join(home, ".openclaw", "logs"), { recursive: true }); + fs.writeFileSync( + path.join(stubDir, "launchctl"), + `#!/bin/sh\nprintf '%s\\n' "$*" >> "$LAUNCHCTL_CALLS_PATH"\n${launchctlStub}\n`, + ); + fs.chmodSync(path.join(stubDir, "launchctl"), 0o755); + fs.writeFileSync(path.join(stubDir, "sleep"), "#!/bin/sh\nexit 0\n"); + fs.chmodSync(path.join(stubDir, "sleep"), 0o755); + + spawnMock.mockReturnValue({ pid: 4242, unref: unrefMock }); + scheduleDetachedLaunchdRestartHandoff({ + env: { HOME: home, OPENCLAW_PROFILE: "default" }, + mode: "reload", + waitForPid: noWaitPid, + }); + const [, args] = requireSpawnCall(); + const script = args[1]; + if (!script) { + throw new Error("expected generated restart script"); + } + + let exitCode = 0; + try { + await execFileAsync( + "/bin/sh", + [ + "-c", + script, + "handoff-test", + "gui/501/test.label", + "gui/501", + "/tmp/test.plist", + String(noWaitPid), + ], + { + env: { + ...process.env, + LAUNCHCTL_CALLS_PATH: callsPath, + LAUNCHCTL_STUB_DIR: stubDir, + PATH: `${stubDir}:${process.env.PATH}`, + }, + }, + ); + } catch (error) { + const code = (error as { code?: unknown }).code; + if (typeof code !== "number") { + throw error; + } + exitCode = code; + } + + const calls = fs.readFileSync(callsPath, "utf8").trim().split("\n"); + const log = fs.readFileSync( + path.join(home, ".openclaw", "logs", "gateway-restart.log"), + "utf8", + ); + return { calls, exitCode, log }; + } finally { + fs.rmSync(stubDir, { recursive: true, force: true }); + } +} + afterEach(() => { spawnMock.mockReset(); unrefMock.mockReset(); @@ -95,7 +174,7 @@ describe("scheduleDetachedLaunchdRestartHandoff", () => { expect(args[1]).not.toContain('basename "$service_target"'); }); - it("polls after bootout and falls back to kickstart on bootstrap failure for reload mode", () => { + it("outwaits launchd's stop window after bootout and retries bootstrap for reload mode", () => { spawnMock.mockReturnValue({ pid: 4242, unref: unrefMock }); scheduleDetachedLaunchdRestartHandoff({ @@ -111,12 +190,89 @@ describe("scheduleDetachedLaunchdRestartHandoff", () => { expect(args[1]).toContain("openclaw restart attempt source=launchd-handoff mode=reload"); expect(args[1]).toContain('launchctl enable "$service_target"'); expect(args[1]).toContain('launchctl bootout "$service_target"'); - // polls until launchd finishes the async unload before re-bootstrapping - expect(args[1]).toContain("bootout_wait_count="); + // The unload poll must outlast launchd's ExitTimeOut SIGKILL ceiling plus + // margin (#110137): 35 × 1s vs the old 15 × 0.2s stop window. + expect(args[1]).toContain('bootout_wait_count="35"'); expect(args[1]).toContain('if ! launchctl print "$service_target" >/dev/null 2>&1; then'); + expect(args[1]).toContain("sleep 1"); + // Bootstrap failures retry; kickstart -k only fires while the label is + // registered, because it cannot succeed on a booted-out label. + expect(args[1]).toContain('bootstrap_retry_count="15"'); expect(args[1]).toContain('if launchctl bootstrap "$domain" "$plist_path"; then'); - // fallback: kickstart -k on bootstrap failure so service isn't left deregistered - expect(args[1]).toContain('launchctl kickstart -k "$service_target"'); + expect(args[1]).toContain( + 'if launchctl print "$service_target" >/dev/null 2>&1; then\n if launchctl kickstart -k "$service_target"; then', + ); + expect(args[1]).toContain("bootstrap_retry_count=$((bootstrap_retry_count - 1))"); + }); + + it("executes the generated reload handoff through a delayed launchd stop", async () => { + const result = await executeReloadHandoff(` +case "$1" in + print) + count_file="$LAUNCHCTL_STUB_DIR/print-count" + count=0 + [ -f "$count_file" ] && count=$(sed -n '1p' "$count_file") + count=$((count + 1)) + printf '%s\n' "$count" > "$count_file" + [ "$count" -le 20 ] && exit 0 + exit 113 + ;; + bootstrap) exit 0 ;; + *) exit 0 ;; +esac`); + + expect(result.exitCode).toBe(0); + expect(result.calls.filter((call) => call.startsWith("print "))).toHaveLength(21); + expect(result.calls.filter((call) => call.startsWith("bootstrap "))).toHaveLength(1); + expect(result.log).toContain("restart done"); + expect(result.log).not.toContain("restart failed"); + }); + + it("retries bootstrap when the label disappears between print and kickstart", async () => { + const result = await executeReloadHandoff(` +case "$1" in + print) + count_file="$LAUNCHCTL_STUB_DIR/print-count" + count=0 + [ -f "$count_file" ] && count=$(sed -n '1p' "$count_file") + count=$((count + 1)) + printf '%s\n' "$count" > "$count_file" + [ "$count" -eq 2 ] && exit 0 + exit 113 + ;; + bootstrap) + count_file="$LAUNCHCTL_STUB_DIR/bootstrap-count" + count=0 + [ -f "$count_file" ] && count=$(sed -n '1p' "$count_file") + count=$((count + 1)) + printf '%s\n' "$count" > "$count_file" + [ "$count" -eq 1 ] && exit 5 + exit 0 + ;; + kickstart) exit 113 ;; + *) exit 0 ;; +esac`); + + expect(result.exitCode).toBe(0); + expect(result.calls.filter((call) => call.startsWith("bootstrap "))).toHaveLength(2); + expect(result.calls.filter((call) => call.startsWith("kickstart "))).toHaveLength(1); + expect(result.log).toContain("restart done"); + expect(result.log).not.toContain("restart failed"); + }); + + it("reload retry exhaustion exits nonzero instead of reporting a successful restart", async () => { + // A completed if with a false condition leaves $? at 0. Keep this + // execution-level check so exhausted retries cannot report success while + // the LaunchAgent remains deregistered. + const result = await executeReloadHandoff( + 'case "$1" in bootstrap) exit 5 ;; print) exit 113 ;; *) exit 0 ;; esac', + ); + + expect(result.exitCode).toBe(5); + expect(result.calls.filter((call) => call.startsWith("bootstrap "))).toHaveLength(15); + const { log } = result; + expect(log).toContain("restart failed"); + expect(log).not.toContain("restart done"); }); it("sanitizes restart helper environment overrides before spawning", () => { diff --git a/src/daemon/launchd-restart-handoff.ts b/src/daemon/launchd-restart-handoff.ts index 483c7e470b1f..40e89f021e1f 100644 --- a/src/daemon/launchd-restart-handoff.ts +++ b/src/daemon/launchd-restart-handoff.ts @@ -8,6 +8,7 @@ import { sanitizeForLog } from "../../packages/terminal-core/src/ansi.js"; import { formatErrorMessage } from "../infra/errors.js"; import { sanitizeHostExecEnv } from "../infra/host-env-security.js"; import { resolveGatewayLaunchAgentLabel } from "./constants.js"; +import { LAUNCH_AGENT_EXIT_TIMEOUT_SECONDS } from "./launchd-plist.js"; import { renderPosixRestartLogSetup } from "./restart-logs.js"; type LaunchdRestartHandoffMode = "kickstart" | "reload" | "start-after-exit"; @@ -23,6 +24,13 @@ type LaunchdRestartTarget = { const START_AFTER_EXIT_PRINT_RETRY_COUNT = 15; const START_AFTER_EXIT_PRINT_RETRY_DELAY_SECONDS = 0.2; +// The booted-out label stays registered until launchd finishes stopping the +// old process. ExitTimeOut bounds that stop with SIGKILL, so the reload wait is +// that ceiling plus teardown margin. A 3s poll could advance mid-stop and +// strand the LaunchAgent (#110137). +const RELOAD_BOOTOUT_WAIT_DELAY_SECONDS = 1; +const RELOAD_BOOTOUT_WAIT_COUNT = LAUNCH_AGENT_EXIT_TIMEOUT_SECONDS + 15; +const RELOAD_BOOTSTRAP_RETRY_COUNT = 15; type LaunchdRestartLogEnv = { HOME?: string; @@ -138,17 +146,47 @@ exit "$status" if (mode === "reload") { // Reloading is required after plist content changes; kickstart alone keeps // launchd's already-loaded stdout/stderr/stdin paths. - // After bootout we poll until launchd finishes the async unload before - // re-bootstrapping to avoid EIO (Bootstrap failed: 5) from the race. - // If bootstrap still fails, kickstart -k as a fallback to keep the service - // alive rather than leaving it deregistered. - const bootoutWaitLoop = `bootout_wait_count="${START_AFTER_EXIT_PRINT_RETRY_COUNT}" + // After bootout the label stays registered until launchd finishes its + // ExitTimeOut-bounded stop, so this poll must outlast that stop window. + // Bootstrapping early fails with EIO (Bootstrap failed: 5) and can leave + // the LaunchAgent deregistered (#110137). + const bootoutWaitLoop = `bootout_wait_count="${RELOAD_BOOTOUT_WAIT_COUNT}" while [ "$bootout_wait_count" -gt 0 ]; do if ! launchctl print "$service_target" >/dev/null 2>&1; then break fi bootout_wait_count=$((bootout_wait_count - 1)) - sleep ${START_AFTER_EXIT_PRINT_RETRY_DELAY_SECONDS} + sleep ${RELOAD_BOOTOUT_WAIT_DELAY_SECONDS} +done +`; + // kickstart -k cannot succeed on a booted-out label, so it is only a valid + // fallback while the label is registered; otherwise retry bootstrap so the + // handoff never exits with the service deregistered (#110137). + const bootstrapRetryLoop = `bootstrap_retry_count="${RELOAD_BOOTSTRAP_RETRY_COUNT}" +while :; do + if launchctl bootstrap "$domain" "$plist_path"; then + status=0 + break + else + # Capture inside the else: after a completed if with a false condition, + # $? is 0, which would let exhausted retries report a successful restart. + status=$? + fi + if launchctl print "$service_target" >/dev/null 2>&1; then + if launchctl kickstart -k "$service_target"; then + status=0 + break + else + # The pending bootout can finish between print and kickstart. Keep + # retrying bootstrap if that check-then-act race deregisters the label. + status=$? + fi + fi + bootstrap_retry_count=$((bootstrap_retry_count - 1)) + if [ "$bootstrap_retry_count" -le 0 ]; then + break + fi + sleep ${RELOAD_BOOTOUT_WAIT_DELAY_SECONDS} done `; return `service_target="$1" @@ -159,13 +197,7 @@ status=0 launchctl enable "$service_target" launchctl bootout "$service_target" >/dev/null 2>&1 || true ${bootoutWaitLoop} -if launchctl bootstrap "$domain" "$plist_path"; then - status=0 -else - status=$? - launchctl kickstart -k "$service_target" - status=$? -fi +${bootstrapRetryLoop} if [ "$status" -eq 0 ]; then printf '[%s] openclaw restart done source=launchd-handoff mode=${mode}\\n' "$(date -u +%FT%TZ)" >&2 else