From a672065d5e5539331960e1671db3655fba421e4a Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 7 Aug 2026 01:17:58 -0700 Subject: [PATCH] fix(daemon): legacy systemd unit removal reloads the user manager and stops masking failures (#120027) * fix(daemon): reload systemd user manager after removing legacy units uninstallLegacySystemdUnits removed unit files without a daemon-reload, so the user manager kept the deleted unit definitions loaded and startable until an unrelated reload. Legacy-unit removal now reuses the shared disable-or-tolerate-missing helper, surfaces non-ENOENT unlink failures instead of swallowing them, and reloads the user manager once after any unit file was removed (matching uninstallUserSystemdGatewayUnit). * chore: re-fire CI * chore: re-fire CI against fixed main baseline --- src/daemon/systemd.test.ts | 90 ++++++++++++++++++++++++++++++++++++++ src/daemon/systemd.ts | 67 ++++++++++++++++++++-------- 2 files changed, 138 insertions(+), 19 deletions(-) diff --git a/src/daemon/systemd.test.ts b/src/daemon/systemd.test.ts index 7c63fe5d2e9d..aba33754c48e 100644 --- a/src/daemon/systemd.test.ts +++ b/src/daemon/systemd.test.ts @@ -103,6 +103,7 @@ import { startSystemdService, stageSystemdService, stopSystemdService, + uninstallLegacySystemdUnits, uninstallSystemdService, isSystemUnitActiveAndEnabled, uninstallUserSystemdGatewayUnit, @@ -2774,6 +2775,48 @@ describe("isSystemUnitActiveAndEnabled", () => { ); }); +describe("uninstallLegacySystemdUnits", () => { + beforeEach(() => { + vi.restoreAllMocks(); + execFileMock.mockReset(); + }); + + it("preserves a legacy unit file when systemctl cannot disable it", async () => { + const tempHomeRoot = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-legacy-unit-")); + const env = { HOME: path.join(tempHomeRoot, "home") }; + const unitPath = path.join(env.HOME, ".config", "systemd", "user", "clawdbot-gateway.service"); + try { + await fs.mkdir(path.dirname(unitPath), { recursive: true }); + await fs.writeFile(unitPath, "[Unit]\nDescription=Clawdbot Gateway\n", "utf8"); + execFileMock + .mockImplementationOnce((_cmd, args, _opts, cb) => { + assertUserSystemctlArgs(args, "status"); + cb(null, "", ""); + }) + .mockImplementationOnce((_cmd, args, _opts, cb) => { + assertUserSystemctlArgs(args, "is-enabled", "clawdbot-gateway.service"); + cb(null, "enabled\n", ""); + }) + .mockImplementationOnce((_cmd, args, _opts, cb) => { + assertUserSystemctlArgs(args, "status"); + cb(null, "", ""); + }) + .mockImplementationOnce((_cmd, args, _opts, cb) => { + assertUserSystemctlArgs(args, "disable", "--now", "clawdbot-gateway.service"); + cb(createExecFileError("permission denied", { code: 1 }), "", "Permission denied"); + }); + + const { stdout } = createWritableStreamMock(); + await expect(uninstallLegacySystemdUnits({ env, stdout })).rejects.toThrow( + "systemctl disable failed: Permission denied", + ); + await expect(fs.access(unitPath)).resolves.toBeUndefined(); + } finally { + await fs.rm(tempHomeRoot, { recursive: true, force: true }); + } + }); +}); + describe("uninstallUserSystemdGatewayUnit", () => { async function withUserUnitFixture( run: (context: { env: Record; unitPath: string }) => Promise, @@ -2863,6 +2906,53 @@ describe("uninstallUserSystemdGatewayUnit", () => { expect(writes).toContain("systemctl unavailable; removing unit file only"); }); }); + + it("preserves the unit file when systemctl cannot disable the service", async () => { + await withUserUnitFixture(async ({ env, unitPath }) => { + await fs.writeFile(unitPath, "[Unit]\nDescription=OpenClaw Gateway\n", "utf8"); + execFileMock + .mockImplementationOnce((_cmd, args, _opts, cb) => { + assertUserSystemctlArgs(args, "status"); + cb(null, "", ""); + }) + .mockImplementationOnce((_cmd, args, _opts, cb) => { + assertUserSystemctlArgs(args, "disable", "--now", GATEWAY_SERVICE); + cb(createExecFileError("permission denied", { code: 1 }), "", "Permission denied"); + }); + + const { stdout } = createWritableStreamMock(); + await expect(uninstallUserSystemdGatewayUnit({ env, stdout })).rejects.toThrow( + "systemctl disable failed: Permission denied", + ); + await expect(fs.access(unitPath)).resolves.toBeUndefined(); + expect(execFileMock).toHaveBeenCalledTimes(2); + }); + }); + + it("surfaces daemon-reload failure after removing the disabled unit", async () => { + await withUserUnitFixture(async ({ env, unitPath }) => { + await fs.writeFile(unitPath, "[Unit]\nDescription=OpenClaw Gateway\n", "utf8"); + execFileMock + .mockImplementationOnce((_cmd, args, _opts, cb) => { + assertUserSystemctlArgs(args, "status"); + cb(null, "", ""); + }) + .mockImplementationOnce((_cmd, args, _opts, cb) => { + assertUserSystemctlArgs(args, "disable", "--now", GATEWAY_SERVICE); + cb(null, "", ""); + }) + .mockImplementationOnce((_cmd, args, _opts, cb) => { + assertUserSystemctlArgs(args, "daemon-reload"); + cb(createExecFileError("bus unavailable", { code: 1 }), "", "Bus unavailable"); + }); + + const { stdout } = createWritableStreamMock(); + await expect(uninstallUserSystemdGatewayUnit({ env, stdout })).rejects.toThrow( + "systemctl daemon-reload failed: Bus unavailable", + ); + await expect(fs.access(unitPath)).rejects.toMatchObject({ code: "ENOENT" }); + }); + }); }); describe("systemd service control", () => { diff --git a/src/daemon/systemd.ts b/src/daemon/systemd.ts index 3653abee1f82..14dc543627d6 100644 --- a/src/daemon/systemd.ts +++ b/src/daemon/systemd.ts @@ -863,6 +863,17 @@ function isSystemdUnitMissingDetail(detail: string): boolean { ); } +function isSystemdUnitAlreadyMissingOrInactive(detail: string, unitName: string): boolean { + const escapedUnitName = escapeRegExp(normalizeLowercaseStringOrEmpty(unitName)); + return new RegExp( + `^(?:failed to (?:disable unit|stop\\s+${escapedUnitName}):\\s*)?` + + `(?:unit file\\s+${escapedUnitName}\\s+does not exist|` + + `unit\\s+${escapedUnitName}(?:\\s+is)?\\s+` + + `(?:inactive|not\\s+active|not\\s+loaded|not-found|could not be found))[.!]?$`, + "u", + ).test(normalizeLowercaseStringOrEmpty(detail)); +} + const isSystemctlBusUnavailable = isSystemdUserBusUnavailableDetail; function isSystemdUserScopeUnavailable(detail: string): boolean { @@ -1054,6 +1065,30 @@ async function execSystemctlUser( return await execSystemctl([...machineScopeArgs, ...args], env, timeoutMs); } +async function disableSystemdUserUnitForRemoval( + env: GatewayServiceEnv, + unitName: string, +): Promise { + const result = await execSystemctlUser(env, ["disable", "--now", unitName]); + if (result.code === 0) { + return; + } + const detail = readSystemctlDetail(result); + if (isSystemdUnitAlreadyMissingOrInactive(detail, unitName)) { + return; + } + throw new Error(`systemctl disable failed: ${detail || "unknown error"}`); +} + +async function reloadSystemdUserManager(env: GatewayServiceEnv): Promise { + const result = await execSystemctlUser(env, ["daemon-reload"]); + if (result.code !== 0) { + throw new Error( + `systemctl daemon-reload failed: ${readSystemctlDetail(result) || "unknown error"}`, + ); + } +} + export async function isSystemdUserServiceAvailable( env: GatewayServiceEnv = process.env as GatewayServiceEnv, ): Promise { @@ -1543,21 +1578,7 @@ export async function uninstallSystemdService({ await assertSystemdAvailable(env); const serviceName = resolveSystemdServiceName(env); const unitName = `${serviceName}.service`; - const disabled = await execSystemctlUser(env, ["disable", "--now", unitName]); - if (disabled.code !== 0) { - const detail = readSystemctlDetail(disabled); - const escapedUnitName = escapeRegExp(normalizeLowercaseStringOrEmpty(unitName)); - const alreadyMissingOrInactive = new RegExp( - `^(?:failed to (?:disable unit|stop\\s+${escapedUnitName}):\\s*)?` + - `(?:unit file\\s+${escapedUnitName}\\s+does not exist|` + - `unit\\s+${escapedUnitName}(?:\\s+is)?\\s+` + - `(?:inactive|not\\s+active|not\\s+loaded|not-found|could not be found))[.!]?$`, - "u", - ).test(normalizeLowercaseStringOrEmpty(detail)); - if (!alreadyMissingOrInactive) { - throw new Error(`systemctl disable failed: ${detail || "unknown error"}`); - } - } + await disableSystemdUserUnitForRemoval(env, unitName); const unitPath = resolveSystemdUnitPath(env); let removed = false; @@ -1800,20 +1821,28 @@ export async function uninstallLegacySystemdUnits({ } const systemctlAvailable = await isSystemctlAvailable(env); + let removedAny = false; for (const unit of units) { if (systemctlAvailable) { - await execSystemctlUser(env, ["disable", "--now", `${unit.name}.service`]); + await disableSystemdUserUnitForRemoval(env, `${unit.name}.service`); } else { stdout.write(`systemctl unavailable; removed legacy unit file only: ${unit.name}.service\n`); } try { await fs.unlink(unit.unitPath); + removedAny = true; stdout.write(`${formatLine("Removed legacy systemd service", unit.unitPath)}\n`); - } catch { + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ENOENT") { + throw error; + } stdout.write(`Legacy systemd unit not found at ${unit.unitPath}\n`); } } + if (systemctlAvailable && removedAny) { + await reloadSystemdUserManager(env); + } return units; } @@ -1844,7 +1873,7 @@ export async function uninstallUserSystemdGatewayUnit({ const unitPath = resolveSystemdUnitPath(env); let disabled = false; if (await isSystemctlAvailable(env)) { - await execSystemctlUser(env, ["disable", "--now", unitName]); + await disableSystemdUserUnitForRemoval(env, unitName); disabled = true; } else { stdout.write( @@ -1865,7 +1894,7 @@ export async function uninstallUserSystemdGatewayUnit({ // The manager keeps a deleted unit's definition loaded until it reloads, so // without this the unit stays startable while the detector reports it gone. if (removed && disabled) { - await execSystemctlUser(env, ["daemon-reload"]); + await reloadSystemdUserManager(env); } return { unitName, unitPath, removed, disabled }; }