Skip to content

Commit eebad7a

Browse files
ngutmansteipete
ngutman
authored and
steipete
committed
refactor(daemon): simplify launchd stop lifecycle
1 parent affffdd commit eebad7a

5 files changed

Lines changed: 55 additions & 204 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, forceEnable: true });
71+
const repair = await repairLaunchAgentBootstrap({ env: params.env });
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: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -38,34 +38,31 @@ 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("0");
42-
expect(args[7]).toBe("9876");
41+
expect(args[6]).toBe("9876");
42+
expect(args[7]).toBe("ai.openclaw.gateway");
4343
expect(args[1]).toContain('while kill -0 "$wait_pid" >/dev/null 2>&1; do');
44-
expect(args[1]).not.toContain('launchctl enable "$service_target" >/dev/null 2>&1\nif !');
44+
expect(args[1]).toContain('launchctl enable "$service_target" >/dev/null 2>&1');
4545
expect(args[1]).toContain(
4646
'if ! launchctl kickstart -k "$service_target" >/dev/null 2>&1; then',
4747
);
4848
expect(args[1]).not.toContain("sleep 1");
4949
expect(unrefMock).toHaveBeenCalledTimes(1);
5050
});
5151

52-
it("only injects launchctl enable when the caller requested re-enable", () => {
52+
it("passes the plain label separately for start-after-exit mode", () => {
5353
spawnMock.mockReturnValue({ pid: 4242, unref: unrefMock });
5454

5555
scheduleDetachedLaunchdRestartHandoff({
5656
env: {
5757
HOME: "/Users/test",
5858
OPENCLAW_PROFILE: "default",
5959
},
60-
mode: "kickstart",
61-
shouldEnable: true,
62-
enableMarkerPath: "/Users/test/.openclaw/service/marker",
60+
mode: "start-after-exit",
6361
});
6462

6563
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');
64+
expect(args[7]).toBe("ai.openclaw.gateway");
65+
expect(args[1]).toContain('launchctl start "$label" >/dev/null 2>&1');
66+
expect(args[1]).not.toContain('basename "$service_target"');
7067
});
7168
});

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

Lines changed: 15 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,14 @@ export type LaunchdRestartTarget = {
2020
serviceTarget: string;
2121
};
2222

23+
function assertValidLaunchAgentLabel(label: string): string {
24+
const trimmed = label.trim();
25+
if (!/^[A-Za-z0-9._-]+$/.test(trimmed)) {
26+
throw new Error(`Invalid launchd label: ${trimmed}`);
27+
}
28+
return trimmed;
29+
}
30+
2331
function resolveGuiDomain(): string {
2432
if (typeof process.getuid !== "function") {
2533
return "gui/501";
@@ -30,9 +38,9 @@ function resolveGuiDomain(): string {
3038
function resolveLaunchAgentLabel(env?: Record<string, string | undefined>): string {
3139
const envLabel = normalizeOptionalString(env?.OPENCLAW_LAUNCHD_LABEL);
3240
if (envLabel) {
33-
return envLabel;
41+
return assertValidLaunchAgentLabel(envLabel);
3442
}
35-
return resolveGatewayLaunchAgentLabel(env?.OPENCLAW_PROFILE);
43+
return assertValidLaunchAgentLabel(resolveGatewayLaunchAgentLabel(env?.OPENCLAW_PROFILE));
3644
}
3745

3846
export function resolveLaunchdRestartTarget(
@@ -66,9 +74,8 @@ export function isCurrentProcessLaunchdServiceLabel(
6674
}
6775

6876
function buildLaunchdRestartScript(mode: LaunchdRestartHandoffMode): string {
69-
const waitForCallerPid = `should_enable="$4"
70-
wait_pid="$5"
71-
enable_marker_path="$6"
77+
const waitForCallerPid = `wait_pid="$4"
78+
label="$5"
7279
if [ -n "$wait_pid" ] && [ "$wait_pid" -gt 1 ] 2>/dev/null; then
7380
while kill -0 "$wait_pid" >/dev/null 2>&1; do
7481
sleep 0.1
@@ -81,12 +88,7 @@ fi
8188
domain="$2"
8289
plist_path="$3"
8390
${waitForCallerPid}
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
91+
launchctl enable "$service_target" >/dev/null 2>&1
9092
if ! launchctl kickstart -k "$service_target" >/dev/null 2>&1; then
9193
if launchctl bootstrap "$domain" "$plist_path" >/dev/null 2>&1; then
9294
launchctl kickstart -k "$service_target" >/dev/null 2>&1 || true
@@ -98,14 +100,8 @@ fi
98100
return `service_target="$1"
99101
domain="$2"
100102
plist_path="$3"
101-
label="$(basename "$service_target")"
102103
${waitForCallerPid}
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
104+
launchctl enable "$service_target" >/dev/null 2>&1
109105
if ! launchctl start "$label" >/dev/null 2>&1; then
110106
if launchctl bootstrap "$domain" "$plist_path" >/dev/null 2>&1; then
111107
launchctl start "$label" >/dev/null 2>&1 || launchctl kickstart -k "$service_target" >/dev/null 2>&1 || true
@@ -119,9 +115,7 @@ fi
119115
export function scheduleDetachedLaunchdRestartHandoff(params: {
120116
env?: Record<string, string | undefined>;
121117
mode: LaunchdRestartHandoffMode;
122-
shouldEnable?: boolean;
123118
waitForPid?: number;
124-
enableMarkerPath?: string;
125119
}): LaunchdRestartHandoffResult {
126120
const target = resolveLaunchdRestartTarget(params.env);
127121
const waitForPid =
@@ -138,9 +132,8 @@ export function scheduleDetachedLaunchdRestartHandoff(params: {
138132
target.serviceTarget,
139133
target.domain,
140134
target.plistPath,
141-
params.shouldEnable ? "1" : "0",
142135
String(waitForPid),
143-
params.enableMarkerPath ?? "",
136+
target.label,
144137
],
145138
{
146139
detached: true,

‎src/daemon/launchd.test.ts‎

Lines changed: 16 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import path from "node:path";
21
import { PassThrough } from "node:stream";
32
import { beforeEach, describe, expect, it, vi } from "vitest";
43
import {
@@ -50,19 +49,6 @@ const cleanStaleGatewayProcessesSync = vi.hoisted(() =>
5049
);
5150
const defaultProgramArguments = ["node", "-e", "process.exit(0)"];
5251

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-
6652
function expectLaunchctlEnableBootstrapOrder(env: Record<string, string | undefined>) {
6753
const domain = typeof process.getuid === "function" ? `gui/${process.getuid()}` : "gui/501";
6854
const label = "ai.openclaw.gateway";
@@ -332,24 +318,19 @@ describe("launchctl list detection", () => {
332318
});
333319

334320
describe("launchd bootstrap repair", () => {
335-
it("bootstraps and kickstarts the resolved label without enabling unrelated disabled state", async () => {
321+
it("enables, bootstraps, and kickstarts the resolved label", async () => {
336322
const env: Record<string, string | undefined> = {
337323
HOME: "/Users/test",
338324
OPENCLAW_PROFILE: "default",
339325
};
340326
const repair = await repairLaunchAgentBootstrap({ env });
341327
expect(repair).toEqual({ ok: true, status: "repaired" });
342328

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-
);
329+
const { serviceId, bootstrapIndex } = expectLaunchctlEnableBootstrapOrder(env);
348330
const kickstartIndex = state.launchctlCalls.findIndex(
349331
(c) => c[0] === "kickstart" && c[1] === "-k" && c[2] === serviceId,
350332
);
351333

352-
expect(state.launchctlCalls.some((call) => call[0] === "enable")).toBe(false);
353334
expect(kickstartIndex).toBeGreaterThanOrEqual(0);
354335
expect(bootstrapIndex).toBeLessThan(kickstartIndex);
355336
});
@@ -415,30 +396,6 @@ describe("launchd bootstrap repair", () => {
415396
detail: "launchctl kickstart failed: permission denied",
416397
});
417398
});
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-
});
442399
});
443400

444401
describe("launchd install", () => {
@@ -535,7 +492,6 @@ describe("launchd install", () => {
535492
expect(state.launchctlCalls).toContainEqual(["disable", serviceId]);
536493
expect(state.launchctlCalls).toContainEqual(["stop", "ai.openclaw.gateway"]);
537494
expect(state.launchctlCalls.some((call) => call[0] === "bootout")).toBe(false);
538-
expect(state.files.has(resolveDisableMarkerPath(env))).toBe(true);
539495
expect(output).toContain("Stopped LaunchAgent");
540496
});
541497

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

553509
expect(state.launchctlCalls.some((call) => call[0] === "stop")).toBe(false);
554510
expect(state.launchctlCalls.some((call) => call[0] === "bootout")).toBe(true);
555-
expect(state.files.has(resolveDisableMarkerPath(env))).toBe(false);
556511
expect(output).toContain("Stopped LaunchAgent (degraded)");
557512
expect(output).toContain("used bootout fallback");
558513
});
@@ -649,30 +604,12 @@ describe("launchd install", () => {
649604
const serviceId = `${domain}/${label}`;
650605
expect(result).toEqual({ outcome: "completed" });
651606
expect(cleanStaleGatewayProcessesSync).toHaveBeenCalledWith(18789);
652-
expect(state.launchctlCalls.some((call) => call[0] === "enable")).toBe(false);
607+
expect(state.launchctlCalls).toContainEqual(["enable", serviceId]);
653608
expect(state.launchctlCalls).toContainEqual(["kickstart", "-k", serviceId]);
654609
expect(state.launchctlCalls.some((call) => call[0] === "bootout")).toBe(false);
655610
expect(state.launchctlCalls.some((call) => call[0] === "bootstrap")).toBe(false);
656611
});
657612

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-
676613
it("uses the configured gateway port for stale cleanup", async () => {
677614
const env = {
678615
...createDefaultLaunchdEnv(),
@@ -716,7 +653,7 @@ describe("launchd install", () => {
716653
);
717654

718655
expect(result).toEqual({ outcome: "completed" });
719-
expect(state.launchctlCalls.some((call) => call[0] === "enable")).toBe(false);
656+
expect(state.launchctlCalls.some((call) => call[0] === "enable")).toBe(true);
720657
expect(state.launchctlCalls.some((call) => call[0] === "bootstrap")).toBe(true);
721658
expect(kickstartCalls).toHaveLength(2);
722659
expect(state.launchctlCalls.some((call) => call[0] === "bootout")).toBe(false);
@@ -734,7 +671,7 @@ describe("launchd install", () => {
734671
}),
735672
).rejects.toThrow("launchctl kickstart failed: Input/output error");
736673

737-
expect(state.launchctlCalls.some((call) => call[0] === "enable")).toBe(false);
674+
expect(state.launchctlCalls.some((call) => call[0] === "enable")).toBe(true);
738675
expect(state.launchctlCalls.some((call) => call[0] === "bootstrap")).toBe(false);
739676
});
740677

@@ -751,7 +688,7 @@ describe("launchd install", () => {
751688
}),
752689
).rejects.toThrow("launchctl kickstart failed: Input/output error");
753690

754-
expect(state.launchctlCalls.some((call) => call[0] === "enable")).toBe(false);
691+
expect(state.launchctlCalls.some((call) => call[0] === "enable")).toBe(true);
755692
expect(state.launchctlCalls.some((call) => call[0] === "bootstrap")).toBe(true);
756693
});
757694

@@ -767,7 +704,7 @@ describe("launchd install", () => {
767704
}),
768705
).rejects.toThrow("launchctl kickstart failed: Input/output error");
769706

770-
expect(state.launchctlCalls.some((call) => call[0] === "enable")).toBe(false);
707+
expect(state.launchctlCalls.some((call) => call[0] === "enable")).toBe(true);
771708
expect(state.launchctlCalls.some((call) => call[0] === "bootstrap")).toBe(false);
772709
});
773710

@@ -784,32 +721,11 @@ describe("launchd install", () => {
784721
expect(launchdRestartHandoffState.scheduleDetachedLaunchdRestartHandoff).toHaveBeenCalledWith({
785722
env,
786723
mode: "kickstart",
787-
shouldEnable: false,
788724
waitForPid: process.pid,
789-
enableMarkerPath: undefined,
790725
});
791726
expect(state.launchctlCalls).toEqual([]);
792727
});
793728

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-
813729
it("shows actionable guidance when launchctl gui domain does not support bootstrap", async () => {
814730
state.bootstrapError = "Bootstrap failed: 125: Domain does not support specified action";
815731
const env = createDefaultLaunchdEnv();
@@ -883,4 +799,13 @@ describe("resolveLaunchAgentPlistPath", () => {
883799
])("$name", ({ env, expected }) => {
884800
expect(resolveLaunchAgentPlistPath(env)).toBe(expected);
885801
});
802+
803+
it("rejects invalid launchd labels that contain path separators", () => {
804+
expect(() =>
805+
resolveLaunchAgentPlistPath({
806+
HOME: "/Users/test",
807+
OPENCLAW_LAUNCHD_LABEL: "../evil/label",
808+
}),
809+
).toThrow("Invalid launchd label");
810+
});
886811
});

0 commit comments

Comments
 (0)