From b72a4d2fbbd8cb78dded1f41b395c414f5de17ab Mon Sep 17 00:00:00 2001
From: Diego Rodrigues de Sa e Souza
 <8016841+diegosouzapw@users.noreply.github.com>
Date: Thu, 25 Jun 2026 22:59:41 -0300
Subject: [PATCH] fix(cli): verify launchd registration + skip self-SIGTERM in
 macOS autostart (#4765)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Integrated into release/v3.8.37 — cherry-picked defining commit onto release tip; tests green.
---
 bin/cli/tray/autostart.mjs                    | 85 ++++++++++++++++-
 .../cli/autostart-macos-launchctl.test.ts     | 91 +++++++++++++++++++
 2 files changed, 172 insertions(+), 4 deletions(-)
 create mode 100644 tests/unit/cli/autostart-macos-launchctl.test.ts

diff --git a/bin/cli/tray/autostart.mjs b/bin/cli/tray/autostart.mjs
index 742bf99c20..605161c2a1 100644
--- a/bin/cli/tray/autostart.mjs
+++ b/bin/cli/tray/autostart.mjs
@@ -204,6 +204,64 @@ export function isAutostartEnabled() {
   return false;
 }
 
+/**
+ * Pure parse of `launchctl list <label>` output: true only when the agent's
+ * "PID" line matches the given process id. launchctl prints the running PID for
+ * a managed Aqua agent; a loaded-but-not-running agent omits the key entirely.
+ *
+ * Exported for unit testing — the darwin branch can't execute on the Linux CI.
+ */
+export function parseAgentSelfFromLaunchctl(output, pid) {
+  if (!output) return false;
+  const match = output.match(/"PID"\s*=\s*(\d+)/);
+  return !!(match && parseInt(match[1], 10) === pid);
+}
+
+/**
+ * True when `runList()` (which should run `launchctl list <label>`) succeeds,
+ * i.e. launchd actually recognizes the agent. A plist sitting on disk that
+ * launchd never loaded must NOT count as enabled — checking file existence
+ * alone makes the tray menu lie ("✓ Enabled" while launchd has the agent in a
+ * failed state or never loaded it).
+ *
+ * `runList` is injectable so the parse/branch logic is testable without a real
+ * launchctl on the (Linux) CI runner.
+ */
+export function isLaunchdAgentLoaded(runList) {
+  try {
+    runList();
+    return true;
+  } catch {
+    return false;
+  }
+}
+
+/**
+ * Returns true when the current Node process IS the running instance launchd is
+ * managing under our agent label.
+ *
+ * `launchctl unload`/`load -w` for a user-domain agent sends SIGTERM to the
+ * running process. When the running OmniRoute cli was itself spawned by the
+ * autostart launchd agent (autostart was enabled, then the machine rebooted,
+ * then the user clicked the tray "Disable Autostart" item), an unload would
+ * kill the very process executing the click handler — the tray icon would
+ * silently disappear instead of the menu label flipping. Enable/disable skip
+ * launchctl in that case; the on-disk plist change is what matters for the next
+ * login, and launchd already has us loaded under our own PID.
+ */
+function isAgentSelfMac() {
+  try {
+    const output = execFileSync("launchctl", ["list", APP_LABEL], {
+      encoding: "utf8",
+      stdio: ["ignore", "pipe", "ignore"],
+      timeout: 3000,
+    });
+    return parseAgentSelfFromLaunchctl(output, process.pid);
+  } catch {
+    return false;
+  }
+}
+
 function enableMac() {
   const plistDir = join(homedir(), "Library", "LaunchAgents");
   mkdirSync(plistDir, { recursive: true });
@@ -225,6 +283,10 @@ function enableMac() {
   <key>KeepAlive</key><false/>
 </dict></plist>`;
   writeFileSync(plistPath, plist, { mode: 0o644 });
+  // If we're already the running agent, launchctl load/unload would SIGTERM us.
+  // The plist is updated on disk and launchd already has us loaded under our own
+  // PID — nothing more to do for the current session.
+  if (isAgentSelfMac()) return existsSync(plistPath);
   try {
     execSync("launchctl load -w " + JSON.stringify(plistPath), { stdio: "ignore" });
   } catch {}
@@ -233,9 +295,15 @@ function enableMac() {
 
 function disableMac() {
   const plistPath = join(homedir(), "Library", "LaunchAgents", `${APP_LABEL}.plist`);
-  try {
-    execSync("launchctl unload -w " + JSON.stringify(plistPath), { stdio: "ignore" });
-  } catch {}
+  // Don't kill ourselves: when the current process is the running agent,
+  // `launchctl unload` sends SIGTERM and a user clicking "Disable Autostart"
+  // from the tray would lose the tray icon instead of just flipping the label.
+  // Removing the plist file is enough to stop the agent at the next login.
+  if (!isAgentSelfMac()) {
+    try {
+      execSync("launchctl unload -w " + JSON.stringify(plistPath), { stdio: "ignore" });
+    } catch {}
+  }
   try {
     unlinkSync(plistPath);
   } catch {}
@@ -243,7 +311,16 @@ function disableMac() {
 }
 
 function isEnabledMac() {
-  return existsSync(join(homedir(), "Library", "LaunchAgents", `${APP_LABEL}.plist`));
+  const plistPath = join(homedir(), "Library", "LaunchAgents", `${APP_LABEL}.plist`);
+  if (!existsSync(plistPath)) return false;
+  // The plist file existing is not enough — launchd must actually recognize the
+  // agent, otherwise the tray menu reports "Enabled" for an inert/failed agent.
+  return isLaunchdAgentLoaded(() =>
+    execFileSync("launchctl", ["list", APP_LABEL], {
+      stdio: ["ignore", "ignore", "ignore"],
+      timeout: 3000,
+    })
+  );
 }
 
 function enableWin() {
diff --git a/tests/unit/cli/autostart-macos-launchctl.test.ts b/tests/unit/cli/autostart-macos-launchctl.test.ts
new file mode 100644
index 0000000000..329bbb1bff
--- /dev/null
+++ b/tests/unit/cli/autostart-macos-launchctl.test.ts
@@ -0,0 +1,91 @@
+import test from "node:test";
+import assert from "node:assert/strict";
+import { readFileSync } from "node:fs";
+import { join } from "node:path";
+import {
+  parseAgentSelfFromLaunchctl,
+  isLaunchdAgentLoaded,
+} from "../../../bin/cli/tray/autostart.mjs";
+
+// ---------------------------------------------------------------------------
+// parseAgentSelfFromLaunchctl — pure parse of `launchctl list <label>` output.
+// True only when the agent's "PID" line matches the current process PID, so the
+// enable()/disable() macOS paths can skip launchctl unload/load when the running
+// process IS the autostart agent (the unload would self-SIGTERM the tray).
+// ---------------------------------------------------------------------------
+
+test("parseAgentSelfFromLaunchctl returns true when PID matches", () => {
+  const output = `{
+\t"StandardOutPath" = "/tmp/omniroute.out.log";
+\t"PID" = 4242;
+\t"Label" = "com.omniroute.autostart";
+}`;
+  assert.equal(parseAgentSelfFromLaunchctl(output, 4242), true);
+});
+
+test("parseAgentSelfFromLaunchctl returns false when PID differs", () => {
+  const output = `{
+\t"PID" = 9999;
+\t"Label" = "com.omniroute.autostart";
+}`;
+  assert.equal(parseAgentSelfFromLaunchctl(output, 4242), false);
+});
+
+test("parseAgentSelfFromLaunchctl returns false when no PID present", () => {
+  // Loaded-but-not-running agent: launchctl omits the PID key.
+  const output = `{
+\t"Label" = "com.omniroute.autostart";
+\t"LastExitStatus" = 0;
+}`;
+  assert.equal(parseAgentSelfFromLaunchctl(output, 4242), false);
+});
+
+test("parseAgentSelfFromLaunchctl tolerates whitespace variations", () => {
+  assert.equal(parseAgentSelfFromLaunchctl('"PID"=4242;', 4242), true);
+  assert.equal(parseAgentSelfFromLaunchctl('"PID"   =   4242 ;', 4242), true);
+});
+
+test("parseAgentSelfFromLaunchctl returns false on empty/garbage", () => {
+  assert.equal(parseAgentSelfFromLaunchctl("", 4242), false);
+  assert.equal(parseAgentSelfFromLaunchctl("not launchctl output", 4242), false);
+});
+
+// ---------------------------------------------------------------------------
+// isLaunchdAgentLoaded — true only when `launchctl list <label>` succeeds.
+// This is what closes the false-"Enabled" gap: a plist on disk that launchd
+// does not actually recognize must report NOT enabled.
+// ---------------------------------------------------------------------------
+
+test("isLaunchdAgentLoaded returns true when launchctl list succeeds", () => {
+  const ran = isLaunchdAgentLoaded(() => {
+    /* launchctl exited 0 — agent recognized */
+  });
+  assert.equal(ran, true);
+});
+
+test("isLaunchdAgentLoaded returns false when launchctl list throws", () => {
+  const ran = isLaunchdAgentLoaded(() => {
+    throw new Error("Could not find service");
+  });
+  assert.equal(ran, false);
+});
+
+// ---------------------------------------------------------------------------
+// Source-level guard: isEnabledMac() must verify with launchctl, not just the
+// plist file existence. Mirrors the repo's existing Linux source-assertion test
+// (the darwin branch can't execute on the Linux CI runner).
+// ---------------------------------------------------------------------------
+
+test("isEnabledMac verifies launchd registration, not just plist existence", () => {
+  const source = readFileSync(join(process.cwd(), "bin/cli/tray/autostart.mjs"), "utf8");
+  // The verification helper is wired into the darwin enabled-check.
+  assert.match(source, /isLaunchdAgentLoaded/);
+  // launchctl list is queried with the app label, no shell interpolation.
+  assert.match(source, /launchctl",\s*\["list",\s*APP_LABEL\]/);
+});
+
+test("enable/disable macOS skip launchctl when the current process is the agent", () => {
+  const source = readFileSync(join(process.cwd(), "bin/cli/tray/autostart.mjs"), "utf8");
+  assert.match(source, /isAgentSelfMac/);
+  assert.match(source, /parseAgentSelfFromLaunchctl/);
+});