fix(doctor): bound legacy launchd cleanup (#109116)

* fix(doctor): bound legacy launchd cleanup

Bound both legacy launchctl cleanup attempts to five seconds while keeping the existing bootout-then-unload order. Only move the legacy plist after a successful command or an explicit already-unloaded result; leave it in place when timeout or another failure prevents confirmation so a later doctor run can retry. Report filesystem cleanup failures as skipped instead of claiming removal.

* fix(doctor): verify launchd cleanup state before moving plist

Poll a bounded launchctl print postcondition after legacy bootout and unload. Treat loaded, unknown, and timed-out probes as unconfirmed so doctor leaves the plist available for retry.

* test(doctor): clarify launchd cleanup postcondition

* fix(doctor): reject timed-out launchd probes

Preserve timeout evidence from the command runner, including sanitized timeout messages, so partial not-loaded output cannot authorize plist removal.

* fix(doctor): classify no-output launchd timeouts

* test(doctor): trim launchd cleanup cases

Co-authored-by: Alix-007 <li.long15@xydigit.com>

---------

Co-authored-by: Peter Steinberger <steipete@gmail.com>

src/commands/doctor-gateway-services.test.ts

@@ -48,6 +48,7 @@ const mocks = vi.hoisted(() => ({
   uninstallLegacySystemdUnits: vi.fn().mockResolvedValue([]),
   readWindowsProcessArgsSync: vi.fn(),
   readWindowsStartupFallbackRuntimeForUpdate: vi.fn(),
+  runExec: vi.fn(),
   note: vi.fn(),
 }));
 
@@ -111,6 +112,10 @@ vi.mock("../infra/windows-port-pids.js", () => ({
   readWindowsProcessArgsSync: mocks.readWindowsProcessArgsSync,
 }));
 
+vi.mock("../process/exec.js", () => ({
+  runExec: mocks.runExec,
+}));
+
 vi.mock("../../packages/terminal-core/src/note.js", () => ({
   note: mocks.note,
 }));
@@ -174,6 +179,70 @@ function mockProcessPlatform(platform: NodeJS.Platform) {
   });
 }
 
+const LEGACY_MAC_LABEL = "com.openclaw.gateway";
+const LEGACY_MAC_PLIST = "/Users/test/Library/LaunchAgents/com.openclaw.gateway.plist";
+
+function setupLegacyMacService() {
+  mockProcessPlatform("darwin");
+  mocks.findExtraGatewayServices.mockResolvedValue([
+    {
+      platform: "darwin",
+      label: LEGACY_MAC_LABEL,
+      detail: `plist: ${LEGACY_MAC_PLIST}`,
+      scope: "user",
+      legacy: true,
+    },
+  ]);
+}
+
+function launchctlFailure(
+  params: {
+    message?: string;
+    stderr?: string;
+    stdout?: string;
+    timedOut?: boolean;
+  } = {},
+) {
+  return Object.assign(new Error(params.message ?? "launchctl failed"), {
+    stderr: params.stderr ?? "",
+    stdout: params.stdout ?? "",
+    ...(params.timedOut ? { timedOut: true } : {}),
+  });
+}
+
+function expectBoundedLaunchctlCleanup() {
+  const domain = typeof process.getuid === "function" ? `gui/${process.getuid()}` : "gui/501";
+  expect(mocks.runExec).toHaveBeenNthCalledWith(
+    1,
+    "launchctl",
+    ["bootout", domain, LEGACY_MAC_PLIST],
+    { logOutput: false, timeoutMs: 5_000 },
+  );
+  expect(mocks.runExec).toHaveBeenNthCalledWith(2, "launchctl", ["unload", LEGACY_MAC_PLIST], {
+    logOutput: false,
+    timeoutMs: 5_000,
+  });
+  expect(mocks.runExec).toHaveBeenNthCalledWith(
+    3,
+    "launchctl",
+    ["print", `${domain}/${LEGACY_MAC_LABEL}`],
+    {
+      logOutput: false,
+      timeoutMs: expect.any(Number),
+    },
+  );
+  const probeTimeout = mocks.runExec.mock.calls[2]?.[2]?.timeoutMs;
+  expect(probeTimeout).toBeGreaterThan(0);
+  expect(probeTimeout).toBeLessThanOrEqual(5_000);
+}
+
+function mockConfirmedUnloaded(stderr = "Could not find service") {
+  mocks.runExec
+    .mockResolvedValueOnce({ stdout: "", stderr: "" })
+    .mockResolvedValueOnce({ stdout: "", stderr: "" })
+    .mockRejectedValueOnce(launchctlFailure({ stderr }));
+}
+
 async function runRepair(cfg: OpenClawConfig, options: { allowExecSecretRefs?: boolean } = {}) {
   await maybeRepairGatewayServiceConfig(cfg, "local", makeDoctorIo(), makeDoctorPrompts(), options);
 }
@@ -1593,9 +1662,11 @@ describe("maybeScanExtraGatewayServices", () => {
     mocks.renderGatewayServiceCleanupHints.mockReturnValue([]);
     mocks.isSystemdUnitActive.mockResolvedValue(false);
     mocks.uninstallLegacySystemdUnits.mockResolvedValue([]);
+    mocks.runExec.mockResolvedValue({ stdout: "", stderr: "" });
   });
 
   afterEach(() => {
+    vi.restoreAllMocks();
     mockProcessPlatform(originalPlatform);
   });
 
@@ -1773,6 +1844,184 @@ describe("maybeScanExtraGatewayServices", () => {
     );
   });
 
+  it.each(["Could not find service", "No such process"])(
+    "moves a legacy macOS plist only after print reports '%s'",
+    async (stderr) => {
+      setupLegacyMacService();
+      mockConfirmedUnloaded(stderr);
+      const runtime = makeDoctorIo();
+      const rename = vi.spyOn(fs, "rename").mockResolvedValue(undefined);
+      vi.spyOn(fs, "mkdir").mockResolvedValue(undefined);
+      vi.spyOn(fs, "access").mockResolvedValue(undefined);
+
+      await maybeScanExtraGatewayServices({ deep: false }, runtime, makeDoctorPrompts());
+
+      expectBoundedLaunchctlCleanup();
+      expect(rename).toHaveBeenCalledTimes(1);
+      expectNoteContaining(LEGACY_MAC_LABEL, "Legacy gateway removed");
+      expectNoNoteContaining(LEGACY_MAC_LABEL, "Legacy gateway cleanup skipped");
+      expect(runtime.log).toHaveBeenCalledWith(
+        "Legacy gateway services removed. Installing OpenClaw gateway next.",
+      );
+    },
+  );
+
+  it.each([
+    ["timeouts", launchctlFailure({ timedOut: true })],
+    ["unknown failures", launchctlFailure({ stderr: "Permission denied" })],
+  ])("keeps the plist when both launchctl calls end in %s", async (_, failure) => {
+    setupLegacyMacService();
+    mocks.runExec.mockRejectedValue(failure);
+    const mkdir = vi.spyOn(fs, "mkdir").mockResolvedValue(undefined);
+    const access = vi.spyOn(fs, "access").mockResolvedValue(undefined);
+    const rename = vi.spyOn(fs, "rename").mockResolvedValue(undefined);
+    const runtime = makeDoctorIo();
+
+    await maybeScanExtraGatewayServices({ deep: false }, runtime, makeDoctorPrompts());
+
+    expectBoundedLaunchctlCleanup();
+    expect(mkdir).not.toHaveBeenCalled();
+    expect(access).not.toHaveBeenCalled();
+    expect(rename).not.toHaveBeenCalled();
+    expectNoteContaining(
+      `${LEGACY_MAC_LABEL} (launchctl could not confirm unload)`,
+      "Legacy gateway cleanup skipped",
+    );
+    expectNoNoteContaining(LEGACY_MAC_LABEL, "Legacy gateway removed");
+    expect(runtime.log).not.toHaveBeenCalledWith(
+      "Legacy gateway services removed. Installing OpenClaw gateway next.",
+    );
+  });
+
+  it("keeps the plist when a successful cleanup command is followed by a loaded probe", async () => {
+    setupLegacyMacService();
+    mocks.runExec
+      .mockRejectedValueOnce(launchctlFailure({ timedOut: true }))
+      .mockResolvedValueOnce({ stdout: "", stderr: "" })
+      .mockResolvedValueOnce({ stdout: "state = waiting\npid = 0\n", stderr: "" })
+      .mockRejectedValueOnce(launchctlFailure({ stderr: "Permission denied" }));
+    const mkdir = vi.spyOn(fs, "mkdir").mockResolvedValue(undefined);
+    const access = vi.spyOn(fs, "access").mockResolvedValue(undefined);
+    const rename = vi.spyOn(fs, "rename").mockResolvedValue(undefined);
+    const runtime = makeDoctorIo();
+
+    await maybeScanExtraGatewayServices({ deep: false }, runtime, makeDoctorPrompts());
+
+    expect(mocks.runExec).toHaveBeenCalledTimes(4);
+    expect(mkdir).not.toHaveBeenCalled();
+    expect(access).not.toHaveBeenCalled();
+    expect(rename).not.toHaveBeenCalled();
+    expectNoteContaining(
+      `${LEGACY_MAC_LABEL} (launchctl could not confirm unload)`,
+      "Legacy gateway cleanup skipped",
+    );
+    expectNoNoteContaining(LEGACY_MAC_LABEL, "Legacy gateway removed");
+  });
+
+  it("keeps the plist when the postcondition probe times out", async () => {
+    setupLegacyMacService();
+    mocks.runExec
+      .mockResolvedValueOnce({ stdout: "", stderr: "" })
+      .mockResolvedValueOnce({ stdout: "", stderr: "" })
+      .mockRejectedValueOnce(
+        launchctlFailure({
+          message: "Command timed out after 5000 milliseconds",
+          stderr: "Could not find service",
+        }),
+      );
+    const mkdir = vi.spyOn(fs, "mkdir").mockResolvedValue(undefined);
+    const access = vi.spyOn(fs, "access").mockResolvedValue(undefined);
+    const rename = vi.spyOn(fs, "rename").mockResolvedValue(undefined);
+    const runtime = makeDoctorIo();
+
+    await maybeScanExtraGatewayServices({ deep: false }, runtime, makeDoctorPrompts());
+
+    expectBoundedLaunchctlCleanup();
+    expect(mkdir).not.toHaveBeenCalled();
+    expect(access).not.toHaveBeenCalled();
+    expect(rename).not.toHaveBeenCalled();
+    expectNoteContaining(
+      `${LEGACY_MAC_LABEL} (launchctl could not confirm unload)`,
+      "Legacy gateway cleanup skipped",
+    );
+    expectNoNoteContaining(LEGACY_MAC_LABEL, "Legacy gateway removed");
+  });
+
+  it("polls a still-registered stopped label until launchd reports it gone", async () => {
+    setupLegacyMacService();
+    mocks.runExec
+      .mockResolvedValueOnce({ stdout: "", stderr: "" })
+      .mockResolvedValueOnce({ stdout: "", stderr: "" })
+      .mockResolvedValueOnce({ stdout: "state = waiting\npid = 0\n", stderr: "" })
+      .mockRejectedValueOnce(launchctlFailure({ stderr: "Could not find service" }));
+    vi.spyOn(fs, "mkdir").mockResolvedValue(undefined);
+    vi.spyOn(fs, "access").mockResolvedValue(undefined);
+    const rename = vi.spyOn(fs, "rename").mockResolvedValue(undefined);
+
+    await maybeScanExtraGatewayServices({ deep: false }, makeDoctorIo(), makeDoctorPrompts());
+
+    expect(mocks.runExec).toHaveBeenCalledTimes(4);
+    expect(rename).toHaveBeenCalledTimes(1);
+    expectNoteContaining(LEGACY_MAC_LABEL, "Legacy gateway removed");
+  });
+
+  it("reports removal when launchctl confirms unload and the plist is already absent", async () => {
+    setupLegacyMacService();
+    mockConfirmedUnloaded();
+    vi.spyOn(fs, "mkdir").mockResolvedValue(undefined);
+    const missing = Object.assign(new Error("missing"), { code: "ENOENT" });
+    vi.spyOn(fs, "access").mockRejectedValue(missing);
+    const rename = vi.spyOn(fs, "rename").mockResolvedValue(undefined);
+
+    await maybeScanExtraGatewayServices({ deep: false }, makeDoctorIo(), makeDoctorPrompts());
+
+    expectBoundedLaunchctlCleanup();
+    expect(rename).not.toHaveBeenCalled();
+    expectNoteContaining(LEGACY_MAC_LABEL, "Legacy gateway removed");
+    expectNoNoteContaining(LEGACY_MAC_LABEL, "Legacy gateway cleanup skipped");
+  });
+
+  it("does not report removal when the plist cannot be inspected", async () => {
+    setupLegacyMacService();
+    mockConfirmedUnloaded();
+    vi.spyOn(fs, "mkdir").mockResolvedValue(undefined);
+    vi.spyOn(fs, "access").mockRejectedValue(
+      Object.assign(new Error("permission denied"), { code: "EACCES" }),
+    );
+    const rename = vi.spyOn(fs, "rename").mockResolvedValue(undefined);
+
+    await maybeScanExtraGatewayServices({ deep: false }, makeDoctorIo(), makeDoctorPrompts());
+
+    expectBoundedLaunchctlCleanup();
+    expect(rename).not.toHaveBeenCalled();
+    expectNoteContaining(
+      `${LEGACY_MAC_LABEL} (could not inspect plist)`,
+      "Legacy gateway cleanup skipped",
+    );
+    expectNoNoteContaining(LEGACY_MAC_LABEL, "Legacy gateway removed");
+  });
+
+  it("does not report removal when the confirmed-unloaded plist cannot be moved", async () => {
+    setupLegacyMacService();
+    mockConfirmedUnloaded();
+    vi.spyOn(fs, "mkdir").mockResolvedValue(undefined);
+    vi.spyOn(fs, "access").mockResolvedValue(undefined);
+    vi.spyOn(fs, "rename").mockRejectedValue(new Error("permission denied"));
+    const runtime = makeDoctorIo();
+
+    await maybeScanExtraGatewayServices({ deep: false }, runtime, makeDoctorPrompts());
+
+    expectBoundedLaunchctlCleanup();
+    expectNoteContaining(
+      `${LEGACY_MAC_LABEL} (could not move plist)`,
+      "Legacy gateway cleanup skipped",
+    );
+    expectNoNoteContaining(LEGACY_MAC_LABEL, "Legacy gateway removed");
+    expect(runtime.log).not.toHaveBeenCalledWith(
+      "Legacy gateway services removed. Installing OpenClaw gateway next.",
+    );
+  });
+
   it("reports legacy services but skips cleanup when service repair policy is external", async () => {
     await withEnvAsync({ OPENCLAW_SERVICE_REPAIR_POLICY: "external" }, async () => {
       mocks.findExtraGatewayServices.mockResolvedValue([

src/commands/doctor-gateway-services.ts

@@ -16,6 +16,7 @@ import {
   renderGatewayServiceCleanupHints,
   type ExtraGatewayService,
 } from "../daemon/inspect.js";
+import { isLaunchctlNotLoaded } from "../daemon/launchd.js";
 import { OPENCLAW_WRAPPER_ENV_KEY } from "../daemon/program-args.js";
 import { renderSystemNodeWarning, resolveSystemNodeInfo } from "../daemon/runtime-paths.js";
 import { readWindowsStartupFallbackRuntimeForUpdate } from "../daemon/schtasks.js";
@@ -108,8 +109,60 @@ const EXECSTART_REPAIR_CODES = new Set<string>([
   SERVICE_AUDIT_CODES.gatewayCommandMissing,
   SERVICE_AUDIT_CODES.gatewayEntrypointMismatch,
 ]);
-const runLaunchctlQuietly = (args: string[]) =>
-  runExec("launchctl", args, { logOutput: false }).catch(() => undefined);
+const DOCTOR_LAUNCHCTL_TIMEOUT_MS = 5_000;
+const DOCTOR_LAUNCHCTL_CONFIRM_POLL_MS = 100;
+type LaunchctlCleanupAttempt =
+  | { status: "succeeded"; stdout: string; stderr: string }
+  | { status: "failed"; stdout: string; stderr: string; timedOut: boolean };
+
+const runLaunchctlQuietly = async (
+  args: string[],
+  timeoutMs = DOCTOR_LAUNCHCTL_TIMEOUT_MS,
+): Promise<LaunchctlCleanupAttempt> => {
+  try {
+    const output = await runExec("launchctl", args, {
+      logOutput: false,
+      timeoutMs,
+    });
+    return { status: "succeeded", ...output };
+  } catch (error) {
+    const record = error && typeof error === "object" ? (error as Record<string, unknown>) : {};
+    const message = typeof record.message === "string" ? record.message : "";
+    return {
+      status: "failed",
+      stdout: typeof record.stdout === "string" ? record.stdout : "",
+      stderr: typeof record.stderr === "string" ? record.stderr : "",
+      timedOut:
+        record.timedOut === true ||
+        record.noOutputTimedOut === true ||
+        /\bcommand timed out\b/i.test(message),
+    };
+  }
+};
+
+async function confirmLegacyLaunchdServiceUnloaded(serviceTarget: string): Promise<boolean> {
+  const deadline = Date.now() + DOCTOR_LAUNCHCTL_TIMEOUT_MS;
+  while (Date.now() < deadline) {
+    const remainingMs = Math.max(1, deadline - Date.now());
+    const probe = await runLaunchctlQuietly(
+      ["print", serviceTarget],
+      Math.min(DOCTOR_LAUNCHCTL_TIMEOUT_MS, remainingMs),
+    );
+    if (probe.status === "failed") {
+      // A successful print (including a stopped job) means launchd still owns
+      // the label. Unknown errors and probe timeouts stay fail-closed.
+      return !probe.timedOut && isLaunchctlNotLoaded(probe);
+    }
+    const delayMs = Math.min(DOCTOR_LAUNCHCTL_CONFIRM_POLL_MS, deadline - Date.now());
+    if (delayMs <= 0) {
+      break;
+    }
+    await new Promise<void>((resolve) => {
+      setTimeout(resolve, delayMs);
+    });
+  }
+  return false;
+}
 const GATEWAY_SERVICES_EXTRA_CHECK_ID = "core/doctor/gateway-services/extra";
 
 function detectGatewayRuntime(programArguments: string[] | undefined): GatewayDaemonRuntime {
@@ -343,11 +396,17 @@ export function extraGatewayServiceToRepairEffects(
 async function cleanupLegacyLaunchdService(params: {
   label: string;
   plistPath: string;
-}): Promise<string | null> {
+}): Promise<{ status: "removed"; destination?: string } | { status: "failed"; reason: string }> {
   const domain = typeof process.getuid === "function" ? `gui/${process.getuid()}` : "gui/501";
   await runLaunchctlQuietly(["bootout", domain, params.plistPath]);
   await runLaunchctlQuietly(["unload", params.plistPath]);
 
+  // bootout/unload can return before launchd finishes stopping the job. A plist
+  // must stay in place unless a bounded print probe observes the label gone.
+  if (!(await confirmLegacyLaunchdServiceUnloaded(`${domain}/${params.label}`))) {
+    return { status: "failed", reason: "launchctl could not confirm unload" };
+  }
+
   const trashDir = path.join(os.homedir(), ".Trash");
   try {
     await fs.mkdir(trashDir, { recursive: true });
@@ -357,16 +416,19 @@ async function cleanupLegacyLaunchdService(params: {
 
   try {
     await fs.access(params.plistPath);
-  } catch {
-    return null;
+  } catch (error) {
+    if ((error as NodeJS.ErrnoException).code === "ENOENT") {
+      return { status: "removed" };
+    }
+    return { status: "failed", reason: "could not inspect plist" };
   }
 
   const dest = path.join(trashDir, `${params.label}-${Date.now()}.plist`);
   try {
     await fs.rename(params.plistPath, dest);
-    return dest;
+    return { status: "removed", destination: dest };
   } catch {
-    return null;
+    return { status: "failed", reason: "could not move plist" };
   }
 }
 
@@ -416,11 +478,15 @@ async function cleanupLegacyDarwinServices(
       failed.push(`${svc.label} (missing plist path)`);
       continue;
     }
-    const dest = await cleanupLegacyLaunchdService({
+    const result = await cleanupLegacyLaunchdService({
       label: svc.label,
       plistPath,
     });
-    removed.push(dest ? `${svc.label} -> ${dest}` : svc.label);
+    if (result.status === "removed") {
+      removed.push(result.destination ? `${svc.label} -> ${result.destination}` : svc.label);
+    } else {
+      failed.push(`${svc.label} (${result.reason})`);
+    }
   }
 
   return { removed, failed };

src/daemon/launchd.ts

@@ -854,7 +854,7 @@ export async function uninstallLaunchAgent({
   }
 }
 
-function isLaunchctlNotLoaded(res: { stdout: string; stderr: string; code: number }): boolean {
+export function isLaunchctlNotLoaded(res: { stdout: string; stderr: string }): boolean {
   const detail = normalizeLowercaseStringOrEmpty(res.stderr || res.stdout);
   return (
     detail.includes("no such process") ||