diff --git a/src/service-manager-probe.ts b/src/service-manager-probe.ts index 95e35f7b2e..d9c752ad32 100644 --- a/src/service-manager-probe.ts +++ b/src/service-manager-probe.ts @@ -323,7 +323,7 @@ function inspectLaunchd(deps: Required>) }; } -function systemdProperty(out: string, key: string): string | null { +export function systemdProperty(out: string, key: string): string | null { for (const line of out.split("\n")) { const match = line.match(new RegExp(`^${key}=(.*)$`)); if (match) return match[1].trim(); diff --git a/src/service.ts b/src/service.ts index c07c036c4e..4c4967d6de 100644 --- a/src/service.ts +++ b/src/service.ts @@ -49,6 +49,7 @@ import { windowsEnvIndirectBatchPathList, windowsEnvIndirectBatchValue } from ". import { recordOwnedConfigPath } from "./lib/config-ownership"; import { killWindowsSchedulerWrappers } from "./lib/windows-service-wrappers"; import { maybeShowStarPrompt } from "./cli/star-prompt"; +import { systemdProperty } from "./service-manager-probe"; const LABEL = "com.opencodex.proxy"; const TASK = "opencodex-proxy"; @@ -706,12 +707,6 @@ export function resolvedProxyEnv(env: NodeJS.ProcessEnv = process.env): { name: return resolved; } -function systemdOutputTarget(value: string): string { - // StandardOutput/StandardError use output specifiers such as append:/path. - // Quoting the full specifier makes systemd reject it as an invalid output target. - return value.replace(/%/g, "%%").replace(/\n/g, "\\n"); -} - function sh(cmd: string): string { return execSync(cmd, { encoding: "utf8", stdio: ["pipe", "pipe", "pipe"] }).trim(); } @@ -2549,6 +2544,7 @@ export function buildUnit(proxyEnv: { name: string; value: string }[] = resolved opencodexHome, ...proxyEnv.map(({ name, value }) => systemdEnvironmentAssignment(name, value)), ].filter((line): line is string => Boolean(line)).join("\n"); + const command = `${buildServiceShellCommand(bun, cli)} >> ${shellQuote(log)} 2>&1`; return `[Unit] Description=OpenCodex Proxy Server After=network-online.target @@ -2556,12 +2552,10 @@ Wants=network-online.target [Service] Type=simple -ExecStart=${systemdQuote("/bin/sh")} -lc ${systemdQuote(buildServiceShellCommand(bun, cli))} +ExecStart=${systemdQuote("/bin/sh")} -lc ${systemdQuote(command)} Restart=on-failure RestartSec=5 ${envLines} -StandardOutput=${systemdOutputTarget(`append:${log}`)} -StandardError=${systemdOutputTarget(`append:${log}`)} [Install] WantedBy=default.target @@ -2660,10 +2654,18 @@ function startSystemd(): void { } function stopSystemd(): void { try { sh(`systemctl --user stop ${TASK}`); } catch { /* not running */ } } function statusSystemd(): string { try { return sh(`systemctl --user status ${TASK}`); } catch { return ""; } } -function uninstallSystemd(): void { - try { sh(`systemctl --user disable --now ${TASK}`); } catch { /* absent */ } - if (existsSync(unitPath())) unlinkSync(unitPath()); - try { sh("systemctl --user daemon-reload"); } catch { /* best-effort */ } +export function uninstallSystemd(deps: { + run?: (command: string) => string; + unitExists?: () => boolean; + removeUnit?: () => void; +} = {}): void { + const run = deps.run ?? sh; + try { run(`systemctl --user stop ${TASK}`); } catch { /* not running */ } + try { run(`systemctl --user disable ${TASK}`); } catch { /* absent */ } + if ((deps.unitExists ?? (() => existsSync(unitPath())))()) { + (deps.removeUnit ?? (() => unlinkSync(unitPath())))(); + } + try { run("systemctl --user daemon-reload"); } catch { /* best-effort */ } } type ServiceOps = { @@ -2676,6 +2678,21 @@ type ServiceInstallCleanupOps = { stop: () => void; }; +export function systemdServiceInstallCleanupOps(deps: { + run?: (command: string) => string; +} = {}): ServiceInstallCleanupOps { + const run = deps.run ?? sh; + return { + status: () => { + const output = run(`systemctl --user show -p LoadState ${TASK}`); + const loadState = systemdProperty(output, "LoadState")?.toLowerCase(); + if (!loadState) throw new Error("systemd service status could not be verified."); + return loadState === "not-found" ? null : loadState; + }, + stop: () => { run(`systemctl --user stop ${TASK}`); }, + }; +} + function platformOps(backend: ServiceBackend = "scheduler"): ServiceOps | null { if (process.platform === "darwin") return { install: installLaunchd, start: startLaunchd, stop: stopLaunchd, status: statusLaunchd, uninstall: uninstallLaunchd }; @@ -2744,20 +2761,13 @@ function platformServiceInstallCleanupOps(backend: ServiceBackend): ServiceInsta }; } if (process.platform === "linux") { - return { - status: () => { - // `list-unit-files ` exits non-zero when the unit has never been - // installed, which made a clean first install look like an unknown manager - // failure. `show LoadState` gives us the tri-state we actually need: a - // healthy user manager returns `not-found` for a missing unit, while an - // unreachable/permission-denied manager still makes `sh()` throw and the - // caller therefore fails closed. - const loadState = sh(`systemctl --user show ${TASK} --property=LoadState --value`).trim().toLowerCase(); - if (!loadState) throw new Error("systemd service status could not be verified."); - return loadState === "not-found" ? null : loadState; - }, - stop: () => { sh(`systemctl --user stop ${TASK}`); }, - }; + // `list-unit-files ` exits non-zero when the unit has never been + // installed, which made a clean first install look like an unknown manager + // failure. `show LoadState` gives us the tri-state we actually need: a + // healthy user manager returns `not-found` for a missing unit, while an + // unreachable/permission-denied manager still makes `sh()` throw and the + // caller therefore fails closed. + return systemdServiceInstallCleanupOps(); } return null; } diff --git a/tests/service.test.ts b/tests/service.test.ts index 29712adca9..1144845815 100644 --- a/tests/service.test.ts +++ b/tests/service.test.ts @@ -5,7 +5,7 @@ import { isAbsolute, join, posix, win32 } from "node:path"; import * as serviceModule from "../src/service"; import { saveConfig } from "../src/config"; import { windowsEnvIndirectBatchValue } from "../src/lib/win-paths"; -import { assertServiceAuthEnvironment, assertServiceEnvironmentMatchesInstall, bakedServicePathsDiagnostic, confirmServiceServing, launchdListenPort, systemdListenPort, buildPlist, buildUnit, buildWindowsLauncherVbs, buildWindowsSchtasksCreateArgs, buildWindowsSchtasksCreateArgsForXml, buildWindowsServiceScript, buildWindowsTaskXml, deriveWindowsServiceDiagnostic, installFreshWindowsSchedulerSafely, installServiceSafely, launchctlLoadFailed, launchdJobMatchesPlist, normalizeServiceSubcommand, parseServiceArgs, parseServiceInstallState, planServiceCommand, prepareServiceInstall, probeServiceInstallation, readWindowsSchedulerXmlState, registerFreshWindowsSchedulerTask, removeNativeWindowsServiceForScheduler, repairService, resolveServiceListenPort, runLaunchctl, selectServiceSubcommand, serviceLogPath, serviceStartableFromTray, serviceStatusReport, serviceRetryCommand, serviceStatusSummary, systemdNeedsDaemonReload, windowsListenPort, winswListenPort, startLaunchd, windowsTaskRegistrationHealthy } from "../src/service"; +import { assertServiceAuthEnvironment, assertServiceEnvironmentMatchesInstall, bakedServicePathsDiagnostic, confirmServiceServing, launchdListenPort, systemdListenPort, buildPlist, buildUnit, buildWindowsLauncherVbs, buildWindowsSchtasksCreateArgs, buildWindowsSchtasksCreateArgsForXml, buildWindowsServiceScript, buildWindowsTaskXml, deriveWindowsServiceDiagnostic, installFreshWindowsSchedulerSafely, installServiceSafely, launchctlLoadFailed, launchdJobMatchesPlist, normalizeServiceSubcommand, parseServiceArgs, parseServiceInstallState, planServiceCommand, prepareServiceInstall, probeServiceInstallation, readWindowsSchedulerXmlState, registerFreshWindowsSchedulerTask, removeNativeWindowsServiceForScheduler, repairService, resolveServiceListenPort, runLaunchctl, selectServiceSubcommand, serviceLogPath, serviceStartableFromTray, serviceStatusReport, serviceRetryCommand, serviceStatusSummary, systemdNeedsDaemonReload, systemdServiceInstallCleanupOps, uninstallSystemd, windowsListenPort, winswListenPort, startLaunchd, windowsTaskRegistrationHealthy } from "../src/service"; import type { ServiceDiagnostic } from "../src/service"; import { definitionCarriesCredential, resolvedProxyEnv, writeServiceDefinitionFile } from "../src/service"; import { buildWinswXml } from "../src/lib/winsw"; @@ -185,13 +185,12 @@ describe("systemd service unit", () => { expect(nativeUnknown.detail).toContain("WinSW status"); }); - test("uses unquoted append targets for service logs", () => { + test("redirects service output through the ExecStart shell for legacy systemd", () => { const unit = buildUnit(); - expect(unit).toContain("StandardOutput=append:"); - expect(unit).toContain("StandardError=append:"); - expect(unit).not.toContain('StandardOutput="append:'); - expect(unit).not.toContain('StandardError="append:'); + expect(unit).toMatch(/ExecStart=.* start --port \d+ >> '[^'\n]*service\.log' 2>&1/); + expect(unit).not.toContain("StandardOutput="); + expect(unit).not.toContain("StandardError="); }); test("bakes outbound proxy env into the unit so the service is not cut off from upstream (#2107)", () => { @@ -1449,6 +1448,95 @@ describe("service lifecycle cleanup ordering", () => { ]); }); + test("legacy systemd reports an absent unit without stopping cleanup or blocking install", async () => { + const commands: string[] = []; + const manager = systemdServiceInstallCleanupOps({ + run: command => { + commands.push(command); + return "LoadState=not-found\n"; + }, + }); + + expect(manager.status()).toBeNull(); + expect(commands).toEqual(["systemctl --user show -p LoadState opencodex-proxy"]); + expect(commands[0]).not.toContain("--value"); + + commands.length = 0; + let installed = false; + await installServiceSafely("scheduler", () => { installed = true; }, { + platform: "linux", + managerOps: () => manager, + stopTrackedProxy: async () => {}, + }); + + expect(installed).toBe(true); + expect(commands).toEqual(["systemctl --user show -p LoadState opencodex-proxy"]); + }); + + test("legacy systemd still stops a loaded unit before installation", async () => { + const commands: string[] = []; + const manager = systemdServiceInstallCleanupOps({ + run: command => { + commands.push(command); + return command.includes(" show ") ? "LoadState=loaded\n" : ""; + }, + }); + let installed = false; + + await installServiceSafely("scheduler", () => { installed = true; }, { + platform: "linux", + managerOps: () => manager, + stopTrackedProxy: async () => {}, + }); + + expect(installed).toBe(true); + expect(commands).toEqual([ + "systemctl --user show -p LoadState opencodex-proxy", + "systemctl --user stop opencodex-proxy", + ]); + }); + + test("legacy systemd status fails closed when LoadState is missing or empty", async () => { + for (const output of ["ActiveState=inactive\n", "LoadState=\n"]) { + const commands: string[] = []; + let installed = false; + const manager = systemdServiceInstallCleanupOps({ + run: command => { + commands.push(command); + return output; + }, + }); + + await expect(installServiceSafely("scheduler", () => { installed = true; }, { + platform: "linux", + managerOps: () => manager, + stopTrackedProxy: async () => {}, + })).rejects.toThrow("systemd service status could not be verified"); + expect(installed).toBe(false); + expect(commands).toEqual(["systemctl --user show -p LoadState opencodex-proxy"]); + } + }); + + test("legacy systemd uninstall stops and disables separately even when stop fails", () => { + const commands: string[] = []; + + uninstallSystemd({ + run: command => { + commands.push(command); + if (command.includes(" stop ")) throw new Error("not running"); + return ""; + }, + unitExists: () => false, + }); + + expect(commands).toEqual([ + "systemctl --user stop opencodex-proxy", + "systemctl --user disable opencodex-proxy", + "systemctl --user daemon-reload", + ]); + expect(commands.join(" ")).not.toContain("--now"); + }); + test("service install fails closed before install on manager or standalone cleanup errors", async () => { for (const failure of ["status", "stop", "standalone"] as const) { let installed = false;