From cdcb9464fb76c0b0282232ece745439bfd4e03a0 Mon Sep 17 00:00:00 2001 From: Chance Robinson Date: Wed, 11 Mar 2026 22:59:39 -0400 Subject: [PATCH 1/3] fix(gateway): auto-repair unloaded LaunchAgent on start/restart MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a macOS LaunchAgent silently unloads after sleep or extended idle, `gateway start` and `gateway restart` would bail with 'not loaded' hints instead of attempting recovery — even though the plist still exists on disk and `repairLaunchAgentBootstrap()` can re-register it. Changes: - Add optional `repairNotLoaded` method to `GatewayService` interface - Wire darwin implementation: checks plist exists, calls `repairLaunchAgentBootstrap()` (enable + bootstrap + kickstart) - `runServiceStart()`: when service is not loaded but repair succeeds, proceed with normal start flow instead of printing install hints - `runServiceRestart()`: when `onNotLoaded` returns null (no running process to signal) but repair succeeds, proceed with restart flow - Both paths are best-effort: if repair fails or throws, falls through to existing not-loaded behavior (no regression) Fixes #43602 --- src/cli/daemon-cli/lifecycle-core.test.ts | 137 ++++++++++++++++++++++ src/cli/daemon-cli/lifecycle-core.ts | 43 ++++++- src/daemon/service.ts | 20 ++++ 3 files changed, 198 insertions(+), 2 deletions(-) diff --git a/src/cli/daemon-cli/lifecycle-core.test.ts b/src/cli/daemon-cli/lifecycle-core.test.ts index 2f17269eb6c..9cbcc163193 100644 --- a/src/cli/daemon-cli/lifecycle-core.test.ts +++ b/src/cli/daemon-cli/lifecycle-core.test.ts @@ -190,4 +190,141 @@ describe("runServiceRestart token drift", () => { expect(payload.result).toBe("scheduled"); expect(payload.message).toBe("restart scheduled, gateway will restart momentarily"); }); + + describe("repairNotLoaded (#43602)", () => { + it("start: repairs unloaded service when repairNotLoaded succeeds", async () => { + service.isLoaded.mockResolvedValue(false); + const repairNotLoaded = vi.fn().mockResolvedValue({ ok: true }); + const serviceWithRepair = { ...service, repairNotLoaded }; + + await runServiceStart({ + serviceNoun: "Gateway", + service: serviceWithRepair, + renderStartHints: () => [], + opts: { json: true }, + }); + + expect(repairNotLoaded).toHaveBeenCalledTimes(1); + // After successful repair, start should proceed to restart the service. + expect(service.restart).toHaveBeenCalledTimes(1); + const jsonLine = runtimeLogs.find((line) => line.trim().startsWith("{")); + const payload = JSON.parse(jsonLine ?? "{}") as { result?: string }; + expect(payload.result).toBe("started"); + }); + + it("start: falls through to hints when repairNotLoaded returns ok:false", async () => { + service.isLoaded.mockResolvedValue(false); + const repairNotLoaded = vi.fn().mockResolvedValue({ ok: false }); + const serviceWithRepair = { ...service, repairNotLoaded }; + + await runServiceStart({ + serviceNoun: "Gateway", + service: serviceWithRepair, + renderStartHints: () => ["openclaw gateway install"], + opts: { json: true }, + }); + + expect(repairNotLoaded).toHaveBeenCalledTimes(1); + expect(service.restart).not.toHaveBeenCalled(); + const jsonLine = runtimeLogs.find((line) => line.trim().startsWith("{")); + const payload = JSON.parse(jsonLine ?? "{}") as { result?: string; hints?: string[] }; + expect(payload.result).toBe("not-loaded"); + expect(payload.hints).toContain("openclaw gateway install"); + }); + + it("start: falls through to hints when repairNotLoaded throws", async () => { + service.isLoaded.mockResolvedValue(false); + const repairNotLoaded = vi.fn().mockRejectedValue(new Error("launchctl failed")); + const serviceWithRepair = { ...service, repairNotLoaded }; + + await runServiceStart({ + serviceNoun: "Gateway", + service: serviceWithRepair, + renderStartHints: () => ["openclaw gateway install"], + opts: { json: true }, + }); + + expect(repairNotLoaded).toHaveBeenCalledTimes(1); + expect(service.restart).not.toHaveBeenCalled(); + const jsonLine = runtimeLogs.find((line) => line.trim().startsWith("{")); + const payload = JSON.parse(jsonLine ?? "{}") as { result?: string }; + expect(payload.result).toBe("not-loaded"); + }); + + it("start: does not call repairNotLoaded when service is already loaded", async () => { + service.isLoaded.mockResolvedValue(true); + const repairNotLoaded = vi.fn().mockResolvedValue({ ok: true }); + const serviceWithRepair = { ...service, repairNotLoaded }; + + await runServiceStart({ + serviceNoun: "Gateway", + service: serviceWithRepair, + renderStartHints: () => [], + opts: { json: true }, + }); + + expect(repairNotLoaded).not.toHaveBeenCalled(); + }); + + it("restart: repairs unloaded service when onNotLoaded returns null", async () => { + service.isLoaded.mockResolvedValue(false); + const repairNotLoaded = vi.fn().mockResolvedValue({ ok: true }); + const serviceWithRepair = { ...service, repairNotLoaded }; + + const result = await runServiceRestart({ + serviceNoun: "Gateway", + service: serviceWithRepair, + renderStartHints: () => [], + opts: { json: true }, + onNotLoaded: async () => null, + }); + + expect(result).toBe(true); + expect(repairNotLoaded).toHaveBeenCalledTimes(1); + const jsonLine = runtimeLogs.find((line) => line.trim().startsWith("{")); + const payload = JSON.parse(jsonLine ?? "{}") as { result?: string; message?: string }; + expect(payload.result).toBe("restarted"); + expect(payload.message).toContain("re-registered"); + }); + + it("restart: skips repair when onNotLoaded handles it", async () => { + service.isLoaded.mockResolvedValue(false); + const repairNotLoaded = vi.fn().mockResolvedValue({ ok: true }); + const serviceWithRepair = { ...service, repairNotLoaded }; + + const result = await runServiceRestart({ + serviceNoun: "Gateway", + service: serviceWithRepair, + renderStartHints: () => [], + opts: { json: true }, + onNotLoaded: async () => ({ + result: "restarted" as const, + message: "handled by SIGUSR1", + }), + }); + + expect(result).toBe(true); + expect(repairNotLoaded).not.toHaveBeenCalled(); + }); + + it("restart: falls through to hints when repair returns ok:false", async () => { + service.isLoaded.mockResolvedValue(false); + const repairNotLoaded = vi.fn().mockResolvedValue({ ok: false }); + const serviceWithRepair = { ...service, repairNotLoaded }; + + const result = await runServiceRestart({ + serviceNoun: "Gateway", + service: serviceWithRepair, + renderStartHints: () => ["openclaw gateway install"], + opts: { json: true }, + onNotLoaded: async () => null, + }); + + expect(result).toBe(false); + expect(repairNotLoaded).toHaveBeenCalledTimes(1); + const jsonLine = runtimeLogs.find((line) => line.trim().startsWith("{")); + const payload = JSON.parse(jsonLine ?? "{}") as { result?: string }; + expect(payload.result).toBe("not-loaded"); + }); + }); }); diff --git a/src/cli/daemon-cli/lifecycle-core.ts b/src/cli/daemon-cli/lifecycle-core.ts index 8def6aeefe6..38bd47d85f8 100644 --- a/src/cli/daemon-cli/lifecycle-core.ts +++ b/src/cli/daemon-cli/lifecycle-core.ts @@ -194,7 +194,7 @@ export async function runServiceStart(params: { const json = Boolean(params.opts?.json); const { stdout, emit, fail } = createActionIO({ action: "start", json }); - const loaded = await resolveServiceLoadedOrFail({ + let loaded = await resolveServiceLoadedOrFail({ serviceNoun: params.serviceNoun, service: params.service, fail, @@ -202,6 +202,25 @@ export async function runServiceStart(params: { if (loaded === null) { return; } + if (!loaded && params.service.repairNotLoaded) { + // The service was previously installed but is no longer loaded (e.g. + // macOS LaunchAgent silently unloaded after sleep/idle). Attempt to + // re-register the existing service definition before falling through + // to the "not loaded" install hints. See #43602. + try { + const repair = await params.service.repairNotLoaded({ env: process.env }); + if (repair.ok) { + loaded = true; + if (!json) { + defaultRuntime.log( + `${params.serviceNoun} was not loaded — re-registered from existing service definition.`, + ); + } + } + } catch { + // Best-effort repair; fall through to normal not-loaded handling. + } + } if (!loaded) { await handleServiceNotLoaded({ serviceNoun: params.serviceNoun, @@ -356,7 +375,7 @@ export async function runServiceRestart(params: { return true; }; - const loaded = await resolveServiceLoadedOrFail({ + let loaded = await resolveServiceLoadedOrFail({ serviceNoun: params.serviceNoun, service: params.service, fail, @@ -384,6 +403,26 @@ export async function runServiceRestart(params: { fail(`${params.serviceNoun} restart failed: ${String(err)}`); return false; } + if (!handledNotLoaded && params.service.repairNotLoaded) { + // No running process to signal, but the service definition may still + // exist on disk (e.g. macOS LaunchAgent unloaded after sleep/idle). + // Re-register it so `restart` can proceed normally. See #43602. + try { + const repair = await params.service.repairNotLoaded({ env: process.env }); + if (repair.ok) { + loaded = true; + handledNotLoaded = { + result: "restarted", + message: `${params.serviceNoun} was not loaded — re-registered from existing service definition.`, + }; + if (!json) { + defaultRuntime.log(handledNotLoaded.message); + } + } + } catch { + // Best-effort repair; fall through to normal not-loaded handling. + } + } if (!handledNotLoaded) { await handleServiceNotLoaded({ serviceNoun: params.serviceNoun, diff --git a/src/daemon/service.ts b/src/daemon/service.ts index 8083ce4b5e1..58dac26af20 100644 --- a/src/daemon/service.ts +++ b/src/daemon/service.ts @@ -1,8 +1,10 @@ import { installLaunchAgent, isLaunchAgentLoaded, + launchAgentPlistExists, readLaunchAgentProgramArguments, readLaunchAgentRuntime, + repairLaunchAgentBootstrap, restartLaunchAgent, stopLaunchAgent, uninstallLaunchAgent, @@ -64,6 +66,16 @@ export type GatewayService = { isLoaded: (args: GatewayServiceEnvArgs) => Promise; readCommand: (env: GatewayServiceEnv) => Promise; readRuntime: (env: GatewayServiceEnv) => Promise; + /** + * Attempt to re-register and start a service that was previously installed + * but is no longer loaded (e.g. after macOS sleep/idle unloads the + * LaunchAgent). Returns `{ ok: true }` when the service was successfully + * re-bootstrapped, `{ ok: false }` when the service definition does not + * exist on disk (caller should fall through to install hints). + * + * Optional — platforms that do not experience silent unloads can omit this. + */ + repairNotLoaded?: (args: GatewayServiceEnvArgs) => Promise<{ ok: boolean; detail?: string }>; }; export function describeGatewayServiceRestart( @@ -105,6 +117,14 @@ const GATEWAY_SERVICE_REGISTRY: Record { + const env = args.env ?? (process.env as Record); + const plistExists = await launchAgentPlistExists(env); + if (!plistExists) { + return { ok: false, detail: "plist not found on disk" }; + } + return await repairLaunchAgentBootstrap({ env }); + }, }, linux: { label: "systemd", From 16d1462f8b3af2185fbb1839a90a1f3b58ba6f66 Mon Sep 17 00:00:00 2001 From: Chance Robinson Date: Fri, 13 Mar 2026 11:56:56 -0400 Subject: [PATCH 2/3] fix(gateway): race condition handling + config-guard for repairNotLoaded - Handle bootstrap exit 130 and 'already exists in domain' as race condition success in repairLaunchAgentBootstrap (inspired by #26088) - Gate repairNotLoaded behind config validation in runServiceStart to prevent re-registering a service with invalid config (#35862) - Adapt config-guard tests to upstream test harness refactor - Add 3 race condition tests for launchd bootstrap repair - Add 4 config-guard integration tests for repairNotLoaded (start+restart) - All 50 tests pass --- .../lifecycle-core.config-guard.test.ts | 95 +++++++++++++++++++ src/cli/daemon-cli/lifecycle-core.ts | 9 ++ src/daemon/launchd.test.ts | 47 ++++++++- src/daemon/launchd.ts | 10 +- 4 files changed, 159 insertions(+), 2 deletions(-) diff --git a/src/cli/daemon-cli/lifecycle-core.config-guard.test.ts b/src/cli/daemon-cli/lifecycle-core.config-guard.test.ts index 59a2926e993..fa1264544df 100644 --- a/src/cli/daemon-cli/lifecycle-core.config-guard.test.ts +++ b/src/cli/daemon-cli/lifecycle-core.config-guard.test.ts @@ -140,3 +140,98 @@ describe("runServiceStart config pre-flight (#35862)", () => { expect(service.restart).toHaveBeenCalledTimes(1); }); }); + +describe("config-guard gates repairNotLoaded (#43602 + #35862)", () => { + let runServiceStart: typeof import("./lifecycle-core.js").runServiceStart; + let runServiceRestart: typeof import("./lifecycle-core.js").runServiceRestart; + + beforeAll(async () => { + ({ runServiceStart, runServiceRestart } = await import("./lifecycle-core.js")); + }); + + beforeEach(() => { + resetLifecycleRuntimeLogs(); + readConfigFileSnapshotMock.mockReset(); + setConfigSnapshot({ exists: true, valid: true }); + loadConfig.mockReset(); + loadConfig.mockReturnValue({}); + resetLifecycleServiceMocks(); + service.isLoaded.mockResolvedValue(false); + }); + + it("start: aborts before repairNotLoaded when config is invalid", async () => { + const repairNotLoaded = vi.fn().mockResolvedValue({ ok: true }); + const serviceWithRepair = { ...service, repairNotLoaded }; + setConfigSnapshot({ + exists: true, + valid: false, + issues: [{ path: "agents.defaults.model", message: "Unrecognized key" }], + }); + + await expect( + runServiceStart({ + serviceNoun: "Gateway", + service: serviceWithRepair, + renderStartHints: () => [], + opts: { json: true }, + }), + ).rejects.toThrow("__exit__:1"); + + expect(repairNotLoaded).not.toHaveBeenCalled(); + expect(service.restart).not.toHaveBeenCalled(); + }); + + it("start: proceeds with repair when config is valid", async () => { + const repairNotLoaded = vi.fn().mockResolvedValue({ ok: true }); + const serviceWithRepair = { ...service, repairNotLoaded }; + + await runServiceStart({ + serviceNoun: "Gateway", + service: serviceWithRepair, + renderStartHints: () => [], + opts: { json: true }, + }); + + expect(repairNotLoaded).toHaveBeenCalledTimes(1); + expect(service.restart).toHaveBeenCalledTimes(1); + }); + + it("restart: aborts before repairNotLoaded when config is invalid", async () => { + const repairNotLoaded = vi.fn().mockResolvedValue({ ok: true }); + const serviceWithRepair = { ...service, repairNotLoaded }; + setConfigSnapshot({ + exists: true, + valid: false, + issues: [{ path: "agents.defaults.model", message: "Unrecognized key" }], + }); + + await expect( + runServiceRestart({ + serviceNoun: "Gateway", + service: serviceWithRepair, + renderStartHints: () => [], + opts: { json: true }, + onNotLoaded: async () => null, + }), + ).rejects.toThrow("__exit__:1"); + + expect(repairNotLoaded).not.toHaveBeenCalled(); + expect(service.restart).not.toHaveBeenCalled(); + }); + + it("restart: proceeds with repair when config is valid", async () => { + const repairNotLoaded = vi.fn().mockResolvedValue({ ok: true }); + const serviceWithRepair = { ...service, repairNotLoaded }; + + const result = await runServiceRestart({ + serviceNoun: "Gateway", + service: serviceWithRepair, + renderStartHints: () => [], + opts: { json: true }, + onNotLoaded: async () => null, + }); + + expect(result).toBe(true); + expect(repairNotLoaded).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/cli/daemon-cli/lifecycle-core.ts b/src/cli/daemon-cli/lifecycle-core.ts index 38bd47d85f8..389d9609153 100644 --- a/src/cli/daemon-cli/lifecycle-core.ts +++ b/src/cli/daemon-cli/lifecycle-core.ts @@ -203,6 +203,15 @@ export async function runServiceStart(params: { return; } if (!loaded && params.service.repairNotLoaded) { + // Pre-flight config validation before attempting repair — don't re-register + // a service that will immediately crash due to invalid config. (#35862) + const configError = await getConfigValidationError(); + if (configError) { + fail( + `${params.serviceNoun} aborted: config is invalid.\n${configError}\nFix the config and retry, or run "openclaw doctor" to repair.`, + ); + return; + } // The service was previously installed but is no longer loaded (e.g. // macOS LaunchAgent silently unloaded after sleep/idle). Attempt to // re-register the existing service definition before falling through diff --git a/src/daemon/launchd.test.ts b/src/daemon/launchd.test.ts index 4c624cfeec1..6622616155b 100644 --- a/src/daemon/launchd.test.ts +++ b/src/daemon/launchd.test.ts @@ -18,6 +18,7 @@ const state = vi.hoisted(() => ({ listOutput: "", printOutput: "", bootstrapError: "", + bootstrapCode: 1, kickstartError: "", kickstartFailuresRemaining: 0, dirs: new Set(), @@ -72,7 +73,7 @@ vi.mock("./exec-file.js", () => ({ return { stdout: state.printOutput, stderr: "", code: 0 }; } if (call[0] === "bootstrap" && state.bootstrapError) { - return { stdout: "", stderr: state.bootstrapError, code: 1 }; + return { stdout: "", stderr: state.bootstrapError, code: state.bootstrapCode }; } if (call[0] === "kickstart" && state.kickstartError && state.kickstartFailuresRemaining > 0) { state.kickstartFailuresRemaining -= 1; @@ -145,6 +146,7 @@ beforeEach(() => { state.listOutput = ""; state.printOutput = ""; state.bootstrapError = ""; + state.bootstrapCode = 1; state.kickstartError = ""; state.kickstartFailuresRemaining = 0; state.dirs.clear(); @@ -246,6 +248,49 @@ describe("launchd bootstrap repair", () => { expect(kickstartIndex).toBeGreaterThanOrEqual(0); expect(bootstrapIndex).toBeLessThan(kickstartIndex); }); + + it("treats bootstrap exit 130 as success (race condition)", async () => { + state.bootstrapError = "Service already loaded"; + state.bootstrapCode = 130; + const env: Record = { + HOME: "/Users/test", + OPENCLAW_PROFILE: "default", + }; + const repair = await repairLaunchAgentBootstrap({ env }); + expect(repair.ok).toBe(true); + // Should still kickstart even though bootstrap returned 130 + const kickCalls = state.launchctlCalls.filter((c) => c[0] === "kickstart"); + expect(kickCalls.length).toBe(1); + }); + + it("treats 'already exists in domain' as success (race condition)", async () => { + state.bootstrapError = + "Could not bootstrap service: 5: Input/output error: already exists in domain for gui/501"; + state.bootstrapCode = 1; + const env: Record = { + HOME: "/Users/test", + OPENCLAW_PROFILE: "default", + }; + const repair = await repairLaunchAgentBootstrap({ env }); + expect(repair.ok).toBe(true); + const kickCalls = state.launchctlCalls.filter((c) => c[0] === "kickstart"); + expect(kickCalls.length).toBe(1); + }); + + it("returns ok:false for non-race bootstrap failures", async () => { + state.bootstrapError = "Could not find specified service"; + state.bootstrapCode = 1; + const env: Record = { + HOME: "/Users/test", + OPENCLAW_PROFILE: "default", + }; + const repair = await repairLaunchAgentBootstrap({ env }); + expect(repair.ok).toBe(false); + expect(repair.detail).toContain("Could not find specified service"); + // Should NOT kickstart when bootstrap genuinely failed + const kickCalls = state.launchctlCalls.filter((c) => c[0] === "kickstart"); + expect(kickCalls.length).toBe(0); + }); }); describe("launchd install", () => { diff --git a/src/daemon/launchd.ts b/src/daemon/launchd.ts index 29d0933558c..5e39a922357 100644 --- a/src/daemon/launchd.ts +++ b/src/daemon/launchd.ts @@ -286,7 +286,15 @@ export async function repairLaunchAgentBootstrap(args: { await execLaunchctl(["enable", `${domain}/${label}`]); const boot = await execLaunchctl(["bootstrap", domain, plistPath]); if (boot.code !== 0) { - return { ok: false, detail: (boot.stderr || boot.stdout).trim() || undefined }; + // Race condition: another process (or macOS itself) may have bootstrapped + // the service between our isLoaded check and this call. Exit code 130 or + // "already exists in domain" means the service is already registered — treat + // as success and proceed to kickstart. See #26088 for prior art. + const bootDetail = (boot.stderr || boot.stdout).trim().toLowerCase(); + const alreadyLoaded = boot.code === 130 || bootDetail.includes("already exists in domain"); + if (!alreadyLoaded) { + return { ok: false, detail: (boot.stderr || boot.stdout).trim() || undefined }; + } } const kick = await execLaunchctl(["kickstart", "-k", `${domain}/${label}`]); if (kick.code !== 0) { From 6e4602a64b433fd8791142370223e1ccff25bac6 Mon Sep 17 00:00:00 2001 From: SPFAdvisors <133194757+SPFAdvisors@users.noreply.github.com> Date: Fri, 20 Mar 2026 09:14:48 -0400 Subject: [PATCH 3/3] fix(gateway): eliminate double-kickstart and double-log in repair paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review feedback from Greptile: 1. **Double-kickstart on start**: repairLaunchAgentBootstrap already starts the service (enable → bootstrap → kickstart). Previously, repair set loaded=true which caused service.restart() to fire — a redundant kill+restart cycle. Now repair emits success directly and returns. 2. **Double-kickstart on restart**: Same issue — repair set loaded=true causing the if(loaded) branch to call service.restart(). Now repair records the result via handledNotLoaded without setting loaded=true, so the service.restart() branch is skipped. 3. **Double-log in non-JSON restart**: The repair block logged the message AND the end-of-function handledNotLoaded?.message path logged it again. Removed the premature log from the repair block — the end-of-function path handles it. 4. **New test coverage**: Added non-JSON double-log regression test and explicit double-kickstart prevention test for restart path. All 52 tests pass (26 launchd + 16 lifecycle-core + 10 config-guard). --- .../lifecycle-core.config-guard.test.ts | 3 +- src/cli/daemon-cli/lifecycle-core.test.ts | 43 ++++++++++++++++++- src/cli/daemon-cli/lifecycle-core.ts | 27 ++++++++---- 3 files changed, 62 insertions(+), 11 deletions(-) diff --git a/src/cli/daemon-cli/lifecycle-core.config-guard.test.ts b/src/cli/daemon-cli/lifecycle-core.config-guard.test.ts index fa1264544df..fdf1b7b5154 100644 --- a/src/cli/daemon-cli/lifecycle-core.config-guard.test.ts +++ b/src/cli/daemon-cli/lifecycle-core.config-guard.test.ts @@ -193,7 +193,8 @@ describe("config-guard gates repairNotLoaded (#43602 + #35862)", () => { }); expect(repairNotLoaded).toHaveBeenCalledTimes(1); - expect(service.restart).toHaveBeenCalledTimes(1); + // Repair already started the service; service.restart() is NOT called + expect(service.restart).not.toHaveBeenCalled(); }); it("restart: aborts before repairNotLoaded when config is invalid", async () => { diff --git a/src/cli/daemon-cli/lifecycle-core.test.ts b/src/cli/daemon-cli/lifecycle-core.test.ts index 9cbcc163193..561351d6ae4 100644 --- a/src/cli/daemon-cli/lifecycle-core.test.ts +++ b/src/cli/daemon-cli/lifecycle-core.test.ts @@ -205,8 +205,9 @@ describe("runServiceRestart token drift", () => { }); expect(repairNotLoaded).toHaveBeenCalledTimes(1); - // After successful repair, start should proceed to restart the service. - expect(service.restart).toHaveBeenCalledTimes(1); + // Repair already started the service (bootstrap → kickstart), so + // service.restart() should NOT be called — avoids double-kickstart. + expect(service.restart).not.toHaveBeenCalled(); const jsonLine = runtimeLogs.find((line) => line.trim().startsWith("{")); const payload = JSON.parse(jsonLine ?? "{}") as { result?: string }; expect(payload.result).toBe("started"); @@ -307,6 +308,44 @@ describe("runServiceRestart token drift", () => { expect(repairNotLoaded).not.toHaveBeenCalled(); }); + it("restart: does not double-log repair message in non-JSON mode", async () => { + service.isLoaded.mockResolvedValue(false); + const repairNotLoaded = vi.fn().mockResolvedValue({ ok: true }); + const serviceWithRepair = { ...service, repairNotLoaded }; + + const result = await runServiceRestart({ + serviceNoun: "Gateway", + service: serviceWithRepair, + renderStartHints: () => [], + opts: { json: false }, + onNotLoaded: async () => null, + }); + + expect(result).toBe(true); + expect(repairNotLoaded).toHaveBeenCalledTimes(1); + // The re-registered message should appear exactly once in non-JSON output + const reRegisterLogs = runtimeLogs.filter((line) => line.includes("re-registered")); + expect(reRegisterLogs).toHaveLength(1); + }); + + it("restart: does not call service.restart after successful repair (no double-kickstart)", async () => { + service.isLoaded.mockResolvedValue(false); + const repairNotLoaded = vi.fn().mockResolvedValue({ ok: true }); + const serviceWithRepair = { ...service, repairNotLoaded }; + + await runServiceRestart({ + serviceNoun: "Gateway", + service: serviceWithRepair, + renderStartHints: () => [], + opts: { json: true }, + onNotLoaded: async () => null, + }); + + // repairLaunchAgentBootstrap already kickstarted the service. + // service.restart() should NOT be called — avoids redundant kill+restart. + expect(service.restart).not.toHaveBeenCalled(); + }); + it("restart: falls through to hints when repair returns ok:false", async () => { service.isLoaded.mockResolvedValue(false); const repairNotLoaded = vi.fn().mockResolvedValue({ ok: false }); diff --git a/src/cli/daemon-cli/lifecycle-core.ts b/src/cli/daemon-cli/lifecycle-core.ts index 389d9609153..344ddfe84f3 100644 --- a/src/cli/daemon-cli/lifecycle-core.ts +++ b/src/cli/daemon-cli/lifecycle-core.ts @@ -219,12 +219,20 @@ export async function runServiceStart(params: { try { const repair = await params.service.repairNotLoaded({ env: process.env }); if (repair.ok) { - loaded = true; + // repairLaunchAgentBootstrap already started the service (enable → + // bootstrap → kickstart). Emit success and return — calling + // service.restart() here would be a redundant kill+restart cycle. + const message = `${params.serviceNoun} was not loaded — re-registered from existing service definition.`; + emit({ + ok: true, + result: "started", + message, + service: buildDaemonServiceSnapshot(params.service, true), + }); if (!json) { - defaultRuntime.log( - `${params.serviceNoun} was not loaded — re-registered from existing service definition.`, - ); + defaultRuntime.log(message); } + return; } } catch { // Best-effort repair; fall through to normal not-loaded handling. @@ -416,17 +424,20 @@ export async function runServiceRestart(params: { // No running process to signal, but the service definition may still // exist on disk (e.g. macOS LaunchAgent unloaded after sleep/idle). // Re-register it so `restart` can proceed normally. See #43602. + // + // repairLaunchAgentBootstrap already starts the service (enable → + // bootstrap → kickstart), so we do NOT set `loaded = true` here — + // that would cause the `if (loaded)` branch below to call + // `service.restart()`, issuing a redundant kill+restart cycle. + // Instead we record the result via `handledNotLoaded` and let the + // end-of-function emit path handle messaging. try { const repair = await params.service.repairNotLoaded({ env: process.env }); if (repair.ok) { - loaded = true; handledNotLoaded = { result: "restarted", message: `${params.serviceNoun} was not loaded — re-registered from existing service definition.`, }; - if (!json) { - defaultRuntime.log(handledNotLoaded.message); - } } } catch { // Best-effort repair; fall through to normal not-loaded handling.