Skip to content

Commit affffdd

Browse files
ngutmansteipete
ngutman
authored and
steipete
committed
fix(daemon): keep launchd enable scoped to owned stops
1 parent c0ddcf6 commit affffdd

5 files changed

Lines changed: 282 additions & 46 deletions

File tree

‎src/commands/doctor-gateway-daemon-flow.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ async function maybeRepairLaunchAgentBootstrap(params: {
6868
}
6969

7070
params.runtime.log(`Bootstrapping ${params.title} LaunchAgent...`);
71-
const repair = await repairLaunchAgentBootstrap({ env: params.env });
71+
const repair = await repairLaunchAgentBootstrap({ env: params.env, forceEnable: true });
7272
if (!repair.ok) {
7373
params.runtime.error(
7474
`${params.title} LaunchAgent bootstrap failed: ${repair.detail ?? "unknown error"}`,

‎src/daemon/launchd-restart-handoff.test.ts‎

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,34 @@ describe("scheduleDetachedLaunchdRestartHandoff", () => {
3838
const [, args] = spawnMock.mock.calls[0] as [string, string[]];
3939
expect(args[0]).toBe("-c");
4040
expect(args[2]).toBe("openclaw-launchd-restart-handoff");
41-
expect(args[6]).toBe("9876");
41+
expect(args[6]).toBe("0");
42+
expect(args[7]).toBe("9876");
4243
expect(args[1]).toContain('while kill -0 "$wait_pid" >/dev/null 2>&1; do');
43-
expect(args[1]).toContain('launchctl enable "$service_target" >/dev/null 2>&1');
44-
expect(args[1]).toContain('launchctl kickstart -k "$service_target" >/dev/null 2>&1');
44+
expect(args[1]).not.toContain('launchctl enable "$service_target" >/dev/null 2>&1\nif !');
45+
expect(args[1]).toContain(
46+
'if ! launchctl kickstart -k "$service_target" >/dev/null 2>&1; then',
47+
);
4548
expect(args[1]).not.toContain("sleep 1");
4649
expect(unrefMock).toHaveBeenCalledTimes(1);
4750
});
51+
52+
it("only injects launchctl enable when the caller requested re-enable", () => {
53+
spawnMock.mockReturnValue({ pid: 4242, unref: unrefMock });
54+
55+
scheduleDetachedLaunchdRestartHandoff({
56+
env: {
57+
HOME: "/Users/test",
58+
OPENCLAW_PROFILE: "default",
59+
},
60+
mode: "kickstart",
61+
shouldEnable: true,
62+
enableMarkerPath: "/Users/test/.openclaw/service/marker",
63+
});
64+
65+
const [, args] = spawnMock.mock.calls[0] as [string, string[]];
66+
expect(args[6]).toBe("1");
67+
expect(args[8]).toBe("/Users/test/.openclaw/service/marker");
68+
expect(args[1]).toContain('launchctl enable "$service_target" >/dev/null 2>&1');
69+
expect(args[1]).toContain('rm -f "$enable_marker_path" >/dev/null 2>&1 || true');
70+
});
4871
});

‎src/daemon/launchd-restart-handoff.ts‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,9 @@ export function isCurrentProcessLaunchdServiceLabel(
6666
}
6767

6868
function buildLaunchdRestartScript(mode: LaunchdRestartHandoffMode): string {
69-
const waitForCallerPid = `wait_pid="$4"
69+
const waitForCallerPid = `should_enable="$4"
70+
wait_pid="$5"
71+
enable_marker_path="$6"
7072
if [ -n "$wait_pid" ] && [ "$wait_pid" -gt 1 ] 2>/dev/null; then
7173
while kill -0 "$wait_pid" >/dev/null 2>&1; do
7274
sleep 0.1
@@ -79,7 +81,12 @@ fi
7981
domain="$2"
8082
plist_path="$3"
8183
${waitForCallerPid}
82-
launchctl enable "$service_target" >/dev/null 2>&1
84+
if [ "$should_enable" = "1" ]; then
85+
launchctl enable "$service_target" >/dev/null 2>&1
86+
if [ -n "$enable_marker_path" ]; then
87+
rm -f "$enable_marker_path" >/dev/null 2>&1 || true
88+
fi
89+
fi
8390
if ! launchctl kickstart -k "$service_target" >/dev/null 2>&1; then
8491
if launchctl bootstrap "$domain" "$plist_path" >/dev/null 2>&1; then
8592
launchctl kickstart -k "$service_target" >/dev/null 2>&1 || true
@@ -93,7 +100,12 @@ domain="$2"
93100
plist_path="$3"
94101
label="$(basename "$service_target")"
95102
${waitForCallerPid}
96-
launchctl enable "$service_target" >/dev/null 2>&1
103+
if [ "$should_enable" = "1" ]; then
104+
launchctl enable "$service_target" >/dev/null 2>&1
105+
if [ -n "$enable_marker_path" ]; then
106+
rm -f "$enable_marker_path" >/dev/null 2>&1 || true
107+
fi
108+
fi
97109
if ! launchctl start "$label" >/dev/null 2>&1; then
98110
if launchctl bootstrap "$domain" "$plist_path" >/dev/null 2>&1; then
99111
launchctl start "$label" >/dev/null 2>&1 || launchctl kickstart -k "$service_target" >/dev/null 2>&1 || true
@@ -107,7 +119,9 @@ fi
107119
export function scheduleDetachedLaunchdRestartHandoff(params: {
108120
env?: Record<string, string | undefined>;
109121
mode: LaunchdRestartHandoffMode;
122+
shouldEnable?: boolean;
110123
waitForPid?: number;
124+
enableMarkerPath?: string;
111125
}): LaunchdRestartHandoffResult {
112126
const target = resolveLaunchdRestartTarget(params.env);
113127
const waitForPid =
@@ -124,7 +138,9 @@ export function scheduleDetachedLaunchdRestartHandoff(params: {
124138
target.serviceTarget,
125139
target.domain,
126140
target.plistPath,
141+
params.shouldEnable ? "1" : "0",
127142
String(waitForPid),
143+
params.enableMarkerPath ?? "",
128144
],
129145
{
130146
detached: true,

‎src/daemon/launchd.test.ts‎

Lines changed: 127 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import path from "node:path";
12
import { PassThrough } from "node:stream";
23
import { beforeEach, describe, expect, it, vi } from "vitest";
34
import {
@@ -49,6 +50,19 @@ const cleanStaleGatewayProcessesSync = vi.hoisted(() =>
4950
);
5051
const defaultProgramArguments = ["node", "-e", "process.exit(0)"];
5152

53+
function resolveDisableMarkerPath(
54+
env: Record<string, string | undefined>,
55+
label = "ai.openclaw.gateway",
56+
) {
57+
const profile = env.OPENCLAW_PROFILE?.trim();
58+
const suffix = !profile || profile.toLowerCase() === "default" ? "" : `-${profile}`;
59+
return path.join(
60+
env.OPENCLAW_STATE_DIR ?? path.join(env.HOME ?? "/Users/test", `.openclaw${suffix}`),
61+
"service",
62+
`${encodeURIComponent(label)}.launchd-disabled-by-openclaw`,
63+
);
64+
}
65+
5266
function expectLaunchctlEnableBootstrapOrder(env: Record<string, string | undefined>) {
5367
const domain = typeof process.getuid === "function" ? `gui/${process.getuid()}` : "gui/501";
5468
const label = "ai.openclaw.gateway";
@@ -318,19 +332,24 @@ describe("launchctl list detection", () => {
318332
});
319333

320334
describe("launchd bootstrap repair", () => {
321-
it("enables, bootstraps, and kickstarts the resolved label", async () => {
335+
it("bootstraps and kickstarts the resolved label without enabling unrelated disabled state", async () => {
322336
const env: Record<string, string | undefined> = {
323337
HOME: "/Users/test",
324338
OPENCLAW_PROFILE: "default",
325339
};
326340
const repair = await repairLaunchAgentBootstrap({ env });
327341
expect(repair).toEqual({ ok: true, status: "repaired" });
328342

329-
const { serviceId, bootstrapIndex } = expectLaunchctlEnableBootstrapOrder(env);
343+
const domain = typeof process.getuid === "function" ? `gui/${process.getuid()}` : "gui/501";
344+
const serviceId = `${domain}/ai.openclaw.gateway`;
345+
const bootstrapIndex = state.launchctlCalls.findIndex(
346+
(c) => c[0] === "bootstrap" && c[1] === domain,
347+
);
330348
const kickstartIndex = state.launchctlCalls.findIndex(
331349
(c) => c[0] === "kickstart" && c[1] === "-k" && c[2] === serviceId,
332350
);
333351

352+
expect(state.launchctlCalls.some((call) => call[0] === "enable")).toBe(false);
334353
expect(kickstartIndex).toBeGreaterThanOrEqual(0);
335354
expect(bootstrapIndex).toBeLessThan(kickstartIndex);
336355
});
@@ -396,6 +415,30 @@ describe("launchd bootstrap repair", () => {
396415
detail: "launchctl kickstart failed: permission denied",
397416
});
398417
});
418+
419+
it("re-enables when the disabled marker shows OpenClaw owns the stop state", async () => {
420+
const env: Record<string, string | undefined> = {
421+
HOME: "/Users/test",
422+
OPENCLAW_PROFILE: "default",
423+
};
424+
state.files.set(resolveDisableMarkerPath(env), "disabled_by_openclaw\n");
425+
426+
await repairLaunchAgentBootstrap({ env });
427+
428+
expect(state.launchctlCalls.some((call) => call[0] === "enable")).toBe(true);
429+
expect(state.files.has(resolveDisableMarkerPath(env))).toBe(false);
430+
});
431+
432+
it("allows explicit repairs to force re-enable disabled services", async () => {
433+
const env: Record<string, string | undefined> = {
434+
HOME: "/Users/test",
435+
OPENCLAW_PROFILE: "default",
436+
};
437+
438+
await repairLaunchAgentBootstrap({ env, forceEnable: true });
439+
440+
expect(state.launchctlCalls.some((call) => call[0] === "enable")).toBe(true);
441+
});
399442
});
400443

401444
describe("launchd install", () => {
@@ -492,6 +535,7 @@ describe("launchd install", () => {
492535
expect(state.launchctlCalls).toContainEqual(["disable", serviceId]);
493536
expect(state.launchctlCalls).toContainEqual(["stop", "ai.openclaw.gateway"]);
494537
expect(state.launchctlCalls.some((call) => call[0] === "bootout")).toBe(false);
538+
expect(state.files.has(resolveDisableMarkerPath(env))).toBe(true);
495539
expect(output).toContain("Stopped LaunchAgent");
496540
});
497541

@@ -508,6 +552,7 @@ describe("launchd install", () => {
508552

509553
expect(state.launchctlCalls.some((call) => call[0] === "stop")).toBe(false);
510554
expect(state.launchctlCalls.some((call) => call[0] === "bootout")).toBe(true);
555+
expect(state.files.has(resolveDisableMarkerPath(env))).toBe(false);
511556
expect(output).toContain("Stopped LaunchAgent (degraded)");
512557
expect(output).toContain("used bootout fallback");
513558
});
@@ -529,6 +574,22 @@ describe("launchd install", () => {
529574
expect(output).toContain("did not fully stop the service");
530575
});
531576

577+
it("falls back to bootout when launchctl stop itself errors", async () => {
578+
const env = createDefaultLaunchdEnv();
579+
const stdout = new PassThrough();
580+
let output = "";
581+
state.stopError = "stop failed due to transient launchd error";
582+
stdout.on("data", (chunk: Buffer) => {
583+
output += chunk.toString();
584+
});
585+
586+
await stopLaunchAgent({ env, stdout });
587+
588+
expect(state.launchctlCalls.some((call) => call[0] === "bootout")).toBe(true);
589+
expect(output).toContain("Stopped LaunchAgent (degraded)");
590+
expect(output).toContain("launchctl stop failed; used bootout fallback");
591+
});
592+
532593
it("falls back to bootout when launchctl print cannot confirm the stop state", async () => {
533594
const env = createDefaultLaunchdEnv();
534595
const stdout = new PassThrough();
@@ -553,10 +614,26 @@ describe("launchd install", () => {
553614
state.bootoutError = "launchctl bootout permission denied";
554615

555616
await expect(stopLaunchAgent({ env, stdout: new PassThrough() })).rejects.toThrow(
556-
"launchctl print could not confirm stop: launchctl print permission denied; launchctl bootout failed: launchctl bootout permission denied",
617+
"launchctl print could not confirm stop; used bootout fallback and left service unloaded: launchctl print permission denied; launchctl bootout failed: launchctl bootout permission denied",
557618
);
558619
});
559620

621+
it("sanitizes launchctl details before writing warnings", async () => {
622+
const env = createDefaultLaunchdEnv();
623+
const stdout = new PassThrough();
624+
let output = "";
625+
state.disableError = "boom\n\u001b[31mred\u001b[0m\tmsg";
626+
stdout.on("data", (chunk: Buffer) => {
627+
output += chunk.toString();
628+
});
629+
630+
await stopLaunchAgent({ env, stdout });
631+
632+
expect(output).not.toContain("\u001b[31m");
633+
expect(output).not.toContain("\nred\n");
634+
expect(output).toContain("boom red msg");
635+
});
636+
560637
it("restarts LaunchAgent with kickstart and no bootout", async () => {
561638
const env = {
562639
...createDefaultLaunchdEnv(),
@@ -572,12 +649,30 @@ describe("launchd install", () => {
572649
const serviceId = `${domain}/${label}`;
573650
expect(result).toEqual({ outcome: "completed" });
574651
expect(cleanStaleGatewayProcessesSync).toHaveBeenCalledWith(18789);
575-
expect(state.launchctlCalls).toContainEqual(["enable", serviceId]);
652+
expect(state.launchctlCalls.some((call) => call[0] === "enable")).toBe(false);
576653
expect(state.launchctlCalls).toContainEqual(["kickstart", "-k", serviceId]);
577654
expect(state.launchctlCalls.some((call) => call[0] === "bootout")).toBe(false);
578655
expect(state.launchctlCalls.some((call) => call[0] === "bootstrap")).toBe(false);
579656
});
580657

658+
it("re-enables before restart when OpenClaw owns the persisted disabled state", async () => {
659+
const env = {
660+
...createDefaultLaunchdEnv(),
661+
OPENCLAW_GATEWAY_PORT: "18789",
662+
};
663+
const domain = typeof process.getuid === "function" ? `gui/${process.getuid()}` : "gui/501";
664+
const serviceId = `${domain}/ai.openclaw.gateway`;
665+
state.files.set(resolveDisableMarkerPath(env), "disabled_by_openclaw\n");
666+
667+
await restartLaunchAgent({
668+
env,
669+
stdout: new PassThrough(),
670+
});
671+
672+
expect(state.launchctlCalls).toContainEqual(["enable", serviceId]);
673+
expect(state.files.has(resolveDisableMarkerPath(env))).toBe(false);
674+
});
675+
581676
it("uses the configured gateway port for stale cleanup", async () => {
582677
const env = {
583678
...createDefaultLaunchdEnv(),
@@ -614,12 +709,15 @@ describe("launchd install", () => {
614709
stdout: new PassThrough(),
615710
});
616711

617-
const { serviceId } = expectLaunchctlEnableBootstrapOrder(env);
712+
const domain = typeof process.getuid === "function" ? `gui/${process.getuid()}` : "gui/501";
713+
const serviceId = `${domain}/ai.openclaw.gateway`;
618714
const kickstartCalls = state.launchctlCalls.filter(
619715
(c) => c[0] === "kickstart" && c[1] === "-k" && c[2] === serviceId,
620716
);
621717

622718
expect(result).toEqual({ outcome: "completed" });
719+
expect(state.launchctlCalls.some((call) => call[0] === "enable")).toBe(false);
720+
expect(state.launchctlCalls.some((call) => call[0] === "bootstrap")).toBe(true);
623721
expect(kickstartCalls).toHaveLength(2);
624722
expect(state.launchctlCalls.some((call) => call[0] === "bootout")).toBe(false);
625723
});
@@ -636,7 +734,7 @@ describe("launchd install", () => {
636734
}),
637735
).rejects.toThrow("launchctl kickstart failed: Input/output error");
638736

639-
expect(state.launchctlCalls.some((call) => call[0] === "enable")).toBe(true);
737+
expect(state.launchctlCalls.some((call) => call[0] === "enable")).toBe(false);
640738
expect(state.launchctlCalls.some((call) => call[0] === "bootstrap")).toBe(false);
641739
});
642740

@@ -653,7 +751,7 @@ describe("launchd install", () => {
653751
}),
654752
).rejects.toThrow("launchctl kickstart failed: Input/output error");
655753

656-
expect(state.launchctlCalls.some((call) => call[0] === "enable")).toBe(true);
754+
expect(state.launchctlCalls.some((call) => call[0] === "enable")).toBe(false);
657755
expect(state.launchctlCalls.some((call) => call[0] === "bootstrap")).toBe(true);
658756
});
659757

@@ -669,7 +767,7 @@ describe("launchd install", () => {
669767
}),
670768
).rejects.toThrow("launchctl kickstart failed: Input/output error");
671769

672-
expect(state.launchctlCalls.some((call) => call[0] === "enable")).toBe(true);
770+
expect(state.launchctlCalls.some((call) => call[0] === "enable")).toBe(false);
673771
expect(state.launchctlCalls.some((call) => call[0] === "bootstrap")).toBe(false);
674772
});
675773

@@ -686,11 +784,32 @@ describe("launchd install", () => {
686784
expect(launchdRestartHandoffState.scheduleDetachedLaunchdRestartHandoff).toHaveBeenCalledWith({
687785
env,
688786
mode: "kickstart",
787+
shouldEnable: false,
689788
waitForPid: process.pid,
789+
enableMarkerPath: undefined,
690790
});
691791
expect(state.launchctlCalls).toEqual([]);
692792
});
693793

794+
it("passes marker-owned re-enable intent to the detached handoff", async () => {
795+
const env = createDefaultLaunchdEnv();
796+
state.files.set(resolveDisableMarkerPath(env), "disabled_by_openclaw\n");
797+
launchdRestartHandoffState.isCurrentProcessLaunchdServiceLabel.mockReturnValue(true);
798+
799+
await restartLaunchAgent({
800+
env,
801+
stdout: new PassThrough(),
802+
});
803+
804+
expect(launchdRestartHandoffState.scheduleDetachedLaunchdRestartHandoff).toHaveBeenCalledWith({
805+
env,
806+
mode: "kickstart",
807+
shouldEnable: true,
808+
waitForPid: process.pid,
809+
enableMarkerPath: resolveDisableMarkerPath(env),
810+
});
811+
});
812+
694813
it("shows actionable guidance when launchctl gui domain does not support bootstrap", async () => {
695814
state.bootstrapError = "Bootstrap failed: 125: Domain does not support specified action";
696815
const env = createDefaultLaunchdEnv();

0 commit comments

Comments
 (0)