Skip to content

Commit 1ace6a0

Browse files
steipetesteipete
committed
fix: avoid launchd kickstart after fresh bootstrap
1 parent d0ad5c3 commit 1ace6a0

8 files changed

Lines changed: 62 additions & 23 deletions

File tree

‎src/cli/update-cli/restart-helper.test.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,7 @@ exit 1
194194
// Should clear disabled state and fall back to bootstrap when kickstart fails.
195195
expect(content).toContain("launchctl enable 'gui/501/ai.openclaw.gateway'");
196196
expect(content).toContain("launchctl bootstrap 'gui/501'");
197+
expect(content).toContain("Bootstrap loads RunAtLoad agents");
197198
expect(content).toContain('rm -f "$0"');
198199
await cleanupScript(scriptPath);
199200
});
@@ -250,7 +251,8 @@ exit 1
250251
echo "launchctl $*" >&2
251252
case "$1" in
252253
kickstart) exit 42 ;;
253-
enable|bootstrap) exit 0 ;;
254+
enable) exit 0 ;;
255+
bootstrap) exit 1 ;;
254256
esac
255257
exit 0
256258
`,

‎src/cli/update-cli/restart-helper.ts‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -137,14 +137,19 @@ ${logSetup}
137137
printf '[%s] openclaw restart attempt source=update target=%s\\n' "$(date -u +%FT%TZ)" '${shellEscapeRestartLogValue(label)}' >&2
138138
# Try kickstart first (works when the service is still registered).
139139
# If it fails (e.g. after bootout), clear any persisted disabled state,
140-
# then re-register via bootstrap and kickstart. The final status is captured
140+
# then re-register via bootstrap. Bootstrap loads RunAtLoad agents, so the
141+
# fallback must not immediately kickstart -k the freshly spawned gateway.
142+
# The final status is captured
141143
# before self-cleanup so a genuine failure remains observable.
142144
status=0
143145
if ! launchctl kickstart -k 'gui/${uid}/${escaped}'; then
144146
launchctl enable 'gui/${uid}/${escaped}'
145-
launchctl bootstrap 'gui/${uid}' '${escapedPlistPath}'
146-
launchctl kickstart -k 'gui/${uid}/${escaped}'
147-
status=$?
147+
if launchctl bootstrap 'gui/${uid}' '${escapedPlistPath}'; then
148+
status=0
149+
else
150+
launchctl kickstart -k 'gui/${uid}/${escaped}'
151+
status=$?
152+
fi
148153
fi
149154
if [ "$status" -eq 0 ]; then
150155
printf '[%s] openclaw restart done source=update\\n' "$(date -u +%FT%TZ)" >&2

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ describe("scheduleDetachedLaunchdRestartHandoff", () => {
4545
expect(args[1]).toContain("openclaw restart attempt source=launchd-handoff mode=kickstart");
4646
expect(args[1]).toContain('launchctl enable "$service_target"');
4747
expect(args[1]).toContain('if launchctl kickstart -k "$service_target"; then');
48+
expect(args[1]).toContain(
49+
'if launchctl bootstrap "$domain" "$plist_path"; then\n status=0\n else\n launchctl kickstart -k "$service_target"',
50+
);
4851
expect(args[1]).not.toMatch(/launchctl[^\n]*\/dev\/null/);
4952
expect(args[1]).not.toContain("sleep 1");
5053
expect(unrefMock).toHaveBeenCalledTimes(1);
@@ -68,7 +71,7 @@ describe("scheduleDetachedLaunchdRestartHandoff", () => {
6871
expect(args[1]).toContain("print_retry_count=$((print_retry_count - 1))");
6972
expect(args[1]).toContain("sleep 0.2");
7073
expect(args[1]).toContain('if launchctl bootstrap "$domain" "$plist_path"; then');
71-
expect(args[1]).toContain('if launchctl start "$label"; then');
74+
expect(args[1]).not.toContain('if launchctl start "$label"; then');
7275
expect(args[1]).not.toContain('basename "$service_target"');
7376
});
7477

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

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,8 @@ if launchctl kickstart -k "$service_target"; then
135135
else
136136
status=$?
137137
if launchctl bootstrap "$domain" "$plist_path"; then
138+
status=0
139+
else
138140
launchctl kickstart -k "$service_target"
139141
status=$?
140142
fi
@@ -168,12 +170,7 @@ ${verifyLaunchdReload}
168170
status=0
169171
launchctl enable "$service_target"
170172
if launchctl bootstrap "$domain" "$plist_path"; then
171-
if launchctl start "$label"; then
172-
status=0
173-
else
174-
launchctl kickstart -k "$service_target"
175-
status=$?
176-
fi
173+
status=0
177174
else
178175
status=$?
179176
launchctl kickstart -k "$service_target"

‎src/daemon/launchd.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -830,7 +830,7 @@ describe("launchd install", () => {
830830
expect(result).toEqual({ outcome: "completed" });
831831
expect(state.launchctlCalls.some((call) => call[0] === "enable")).toBe(true);
832832
expect(state.launchctlCalls.some((call) => call[0] === "bootstrap")).toBe(true);
833-
expect(kickstartCalls).toHaveLength(2);
833+
expect(kickstartCalls).toHaveLength(1);
834834
expect(state.launchctlCalls.some((call) => call[0] === "bootout")).toBe(false);
835835
});
836836

‎src/daemon/launchd.ts‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,11 @@ type LaunchAgentBootstrapRepairResult =
437437
| { ok: true; status: "repaired" | "already-loaded" }
438438
| { ok: false; status: "bootstrap-failed" | "kickstart-failed"; detail?: string };
439439

440+
function isLaunchctlAlreadyLoaded(res: { stdout: string; stderr: string; code: number }): boolean {
441+
const detail = normalizeLowercaseStringOrEmpty(res.stderr || res.stdout);
442+
return res.code === 130 || detail.includes("already exists in domain");
443+
}
444+
440445
export async function repairLaunchAgentBootstrap(args: {
441446
env?: Record<string, string | undefined>;
442447
}): Promise<LaunchAgentBootstrapRepairResult> {
@@ -450,9 +455,7 @@ export async function repairLaunchAgentBootstrap(args: {
450455
let repairStatus: "repaired" | "already-loaded" = "repaired";
451456
if (boot.code !== 0) {
452457
const detail = (boot.stderr || boot.stdout).trim();
453-
const normalized = normalizeLowercaseStringOrEmpty(detail);
454-
const alreadyLoaded = boot.code === 130 || normalized.includes("already exists in domain");
455-
if (!alreadyLoaded) {
458+
if (!isLaunchctlAlreadyLoaded(boot)) {
456459
return { ok: false, status: "bootstrap-failed", detail: detail || undefined };
457460
}
458461
repairStatus = "already-loaded";
@@ -850,12 +853,6 @@ export async function restartLaunchAgent({
850853
plistPath,
851854
actionHint: "openclaw gateway restart",
852855
});
853-
854-
const retry = await execLaunchctl(["kickstart", "-k", serviceTarget]);
855-
if (retry.code !== 0) {
856-
await ensureLaunchAgentLoadedAfterFailure({ domain, serviceTarget, plistPath });
857-
throw new Error(`launchctl kickstart failed: ${retry.stderr || retry.stdout}`.trim());
858-
}
859856
writeLaunchAgentActionLine(stdout, "Restarted LaunchAgent", serviceTarget);
860857
return { outcome: "completed" };
861858
}

‎src/infra/restart.test.ts‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,38 @@ describe.runIf(process.platform !== "win32")("cleanStaleGatewayProcessesSync", (
184184
});
185185

186186
describe("triggerOpenClawRestart", () => {
187+
it("does not kickstart after bootstrap registers an unloaded LaunchAgent", () => {
188+
setPlatform("darwin");
189+
delete process.env.VITEST;
190+
delete process.env.NODE_ENV;
191+
process.env.HOME = "/Users/test";
192+
process.env.OPENCLAW_PROFILE = "default";
193+
const uid = typeof process.getuid === "function" ? process.getuid() : 501;
194+
spawnSyncMock.mockImplementation((command: string, args: string[]) => {
195+
if (command === "/usr/sbin/lsof") {
196+
return { error: undefined, status: 1, stdout: "" };
197+
}
198+
if (command === "launchctl" && args[0] === "kickstart" && args[1] === "-k") {
199+
return { error: undefined, status: 113, stderr: "service not loaded" };
200+
}
201+
if (command === "launchctl" && args[0] === "bootstrap") {
202+
return { error: undefined, status: 0, stderr: "" };
203+
}
204+
return { error: undefined, status: 1, stdout: "" };
205+
});
206+
207+
const result = triggerOpenClawRestart();
208+
209+
expect(result).toEqual({
210+
ok: true,
211+
method: "launchctl",
212+
tried: [
213+
`launchctl kickstart -k gui/${uid}/ai.openclaw.gateway`,
214+
`launchctl bootstrap gui/${uid} /Users/test/Library/LaunchAgents/ai.openclaw.gateway.plist`,
215+
],
216+
});
217+
});
218+
187219
it("continues when launchctl bootstrap reports the service is already loaded", () => {
188220
setPlatform("darwin");
189221
delete process.env.VITEST;

‎src/infra/restart.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -640,7 +640,7 @@ export function triggerOpenClawRestart(): RestartAttempt {
640640
}
641641

642642
// kickstart fails when the service was previously booted out (deregistered from launchd).
643-
// Fall back to bootstrap (re-register from plist) + kickstart.
643+
// Fall back to bootstrap, which loads RunAtLoad agents without a follow-up kickstart.
644644
// Use env HOME to match how launchd.ts resolves the plist install path.
645645
const home = process.env.HOME?.trim() || os.homedir();
646646
const plistPath = path.join(home, "Library", "LaunchAgents", `${label}.plist`);
@@ -663,6 +663,9 @@ export function triggerOpenClawRestart(): RestartAttempt {
663663
tried,
664664
};
665665
}
666+
if (boot.status === 0) {
667+
return { ok: true, method: "launchctl", tried };
668+
}
666669
const retryArgs = ["kickstart", target];
667670
tried.push(`launchctl ${retryArgs.join(" ")}`);
668671
const retry = spawnSync("launchctl", retryArgs, {

0 commit comments

Comments
 (0)