From a4cf98a7a584330a6635a2a1b7e90fd45be13161 Mon Sep 17 00:00:00 2001 From: Wolfgang Schoenberger <221313372+wolfiesch@users.noreply.github.com> Date: Sun, 19 Jul 2026 12:39:40 -0700 Subject: [PATCH] fix macOS service upgrade retries --- packages/service-manager/src/index.ts | 36 ++++++++++++------- .../test/service-manager.test.ts | 20 +++++++++++ scripts/tailnet-service.mjs | 26 +++++++++++++- scripts/tailnet-service.test.mjs | 24 +++++++++++++ 4 files changed, 93 insertions(+), 13 deletions(-) diff --git a/packages/service-manager/src/index.ts b/packages/service-manager/src/index.ts index 78d3086d..e3187283 100644 --- a/packages/service-manager/src/index.ts +++ b/packages/service-manager/src/index.ts @@ -126,18 +126,30 @@ async function runChecked( runner: ServiceRunner, command: readonly string[], ): Promise { - let result: ServiceRunnerResult; - try { - result = await runner.run(command); - } catch (cause) { - throw new ServiceCommandError(command, { - exitCode: null, - stdout: "", - stderr: sanitizeDiagnostic(String(cause)), - }); - } - if (result.exitCode !== 0) throw new ServiceCommandError(command, result); - return result; + const isLaunchctlBootstrap = command[0] === "launchctl" && command[1] === "bootstrap"; + // launchd can accept bootout before it has finished removing the service. + // A replacement bootstrap then returns EINPROGRESS (37) for a short window. + const retryDeadline = Date.now() + 3_000; + while (true) { + let result: ServiceRunnerResult; + try { + result = await runner.run(command); + } catch (cause) { + throw new ServiceCommandError(command, { + exitCode: null, + stdout: "", + stderr: sanitizeDiagnostic(String(cause)), + }); + } + if (result.exitCode === 0) return result; + const diagnostics = `${result.stdout}\n${result.stderr}`; + const removalStillFinishing = + isLaunchctlBootstrap && + (result.exitCode === 37 || /operation already in progress|bootstrap failed:\s*37\b/iu.test(diagnostics)); + if (!removalStillFinishing || Date.now() >= retryDeadline) + throw new ServiceCommandError(command, result); + await new Promise((resolve) => setTimeout(resolve, 100)); + } } abstract class BaseManager implements ServiceManager { diff --git a/packages/service-manager/test/service-manager.test.ts b/packages/service-manager/test/service-manager.test.ts index 7695277a..9124b664 100644 --- a/packages/service-manager/test/service-manager.test.ts +++ b/packages/service-manager/test/service-manager.test.ts @@ -346,6 +346,26 @@ describe("service-manager lifecycle", () => { await manager.restart(); expect(runner.calls[1]).toEqual(["launchctl", "bootstrap", "gui/501", manager.definitionPath]); }); + it("waits for launchd to finish removing an upgraded Mac agent before bootstrap", async () => { + const fs = new MemoryFs(); + const runner = new MemoryRunner(); + const manager = new MacLaunchAgentManager(spec, { ...options(fs, runner), uid: 501 }); + fs.files.set(manager.definitionPath, "old definition"); + runner.results = [ + { exitCode: 0, stdout: "state = running\n", stderr: "" }, + { exitCode: 0, stdout: "", stderr: "" }, + { exitCode: 37, stdout: "", stderr: "Bootstrap failed: 37: Operation already in progress" }, + { exitCode: 0, stdout: "", stderr: "" }, + { exitCode: 0, stdout: "", stderr: "" }, + ]; + + await manager.install(); + + expect( + runner.calls.filter((call) => call[0] === "launchctl" && call[1] === "bootstrap"), + ).toHaveLength(2); + expect(fs.files.get(manager.definitionPath)).toBe(renderMacLaunchAgentDefinition(spec)); + }); it("rejects unsupported executable argv and never emits a canary", () => { const canary = "API_TOKEN_CANARY"; expect( diff --git a/scripts/tailnet-service.mjs b/scripts/tailnet-service.mjs index a0dc6e03..ea539b64 100644 --- a/scripts/tailnet-service.mjs +++ b/scripts/tailnet-service.mjs @@ -397,7 +397,12 @@ function sanitizedOutput(value) { .slice(0, 2_048); } -async function runCommand(command) { +export function isTransientLaunchctlBootstrap(command, result) { + if (command.argv[0] !== "launchctl" || command.argv[1] !== "bootstrap") return false; + return result.code === 37 || /operation already in progress|bootstrap failed:\s*37\b/iu.test(`${result.stdout}\n${result.stderr}`); +} + +async function runCommandOnce(command) { return await new Promise((resolvePromise, reject) => { const [executable, ...args] = command.argv; const child = spawn(executable, args, { shell: false, stdio: ["ignore", "pipe", "pipe"] }); @@ -425,6 +430,25 @@ async function runCommand(command) { }); } +async function runCommand(command) { + // macOS service removal is asynchronous even after `launchctl bootout` exits. + // Retry only launchd's explicit EINPROGRESS response during replacement. + const retryDeadline = Date.now() + 3_000; + while (true) { + const result = await runCommandOnce({ ...command, allowFailure: true }); + if (result.code === 0) return result; + if (isTransientLaunchctlBootstrap(command, result) && Date.now() < retryDeadline) { + await new Promise((resolvePromise) => setTimeout(resolvePromise, 100)); + continue; + } + if (command.allowFailure !== true) { + const executable = command.argv[0]; + throw new Error(`${executable} exited ${result.code}: ${result.stderr || result.stdout || "no diagnostics"}`); + } + return result; + } +} + async function runCommands(commands) { const results = []; for (const command of commands) results.push(await runCommand(command)); diff --git a/scripts/tailnet-service.test.mjs b/scripts/tailnet-service.test.mjs index 63fee871..ee17df8f 100644 --- a/scripts/tailnet-service.test.mjs +++ b/scripts/tailnet-service.test.mjs @@ -8,6 +8,7 @@ import { test } from "node:test"; import { DEFAULT_GATEWAY_PORT, SERVICE_LABEL, + isTransientLaunchctlBootstrap, parseCli, parseProfileRoutesOption, renderLaunchAgent, @@ -258,6 +259,29 @@ test("supervisor command plans never use a shell and include durable enablement" } }); +test("only launchctl bootstrap's asynchronous-removal error is retryable", () => { + const bootstrap = { argv: ["launchctl", "bootstrap", "gui/501", "/tmp/service.plist"] }; + assert.equal( + isTransientLaunchctlBootstrap(bootstrap, { + code: 37, + stdout: "", + stderr: "Bootstrap failed: 37: Operation already in progress", + }), + true, + ); + assert.equal( + isTransientLaunchctlBootstrap(bootstrap, { code: 5, stdout: "", stderr: "Input/output error" }), + false, + ); + assert.equal( + isTransientLaunchctlBootstrap( + { argv: ["launchctl", "kickstart", "gui/501/example"] }, + { code: 37, stdout: "", stderr: "Operation already in progress" }, + ), + false, + ); +}); + test("CLI parser rejects ambiguous values and accepts the documented install shape", () => { assert.deepEqual(parseCli(["--help"]), { command: "help", options: {} }); assert.deepEqual(