Fixes process detection for defunct processes in unix environment

README.md

@@ -72,6 +72,13 @@ npm install
 npm start
 ```
 
+## Trouble?
+
+Is the installer crashing? Make sure these packages are installed:
+```
+apt-get install	fdisk	procps
+```
+
 ## License
 
 MIT

src/handlers/processControl.js

@@ -12,8 +12,9 @@ export class ProcessControl {
     if (this.os.isWindows()) {
       return processes.filter((p) => p.name === "codex.exe");
     } else {
-      return processes.filter((p) => 
-        p.name === "codex" && !p.cmd.includes("<defunct>"));
+      return processes.filter(
+        (p) => p.name === "codex" && !p.cmd.includes("<defunct>"),
+      );
     }
   };
 
@@ -37,14 +38,14 @@ export class ProcessControl {
       this.os.terminateProcess(pid);
       await this.sleep();
     }
-  }
+  };
 
   isProcessRunning = async (pid) => {
     const processes = await this.os.listProcesses();
     const p = processes.filter((p) => p.pid == pid);
     const result = p.length > 0;
     return result;
-  }
+  };
 
   startCodexProcess = async () => {
     await this.saveCodexConfigFile();

src/handlers/processControl.test.js

@@ -27,12 +27,12 @@ describe("ProcessControl", () => {
 
   describe("getCodexProcesses", () => {
     const processes = [
-      { id: 0, name: "a.exe" },
-      { id: 1, name: "aaa" },
-      { id: 2, name: "codex" },
-      { id: 3, name: "codex.exe" },
-      { id: 4, name: "notcodex" },
-      { id: 5, name: "alsonotcodex.exe" },
+      { id: 0, name: "a.exe", cmd: "" },
+      { id: 1, name: "codex", cmd: "<defunct>" },
+      { id: 2, name: "codex", cmd: "" },
+      { id: 3, name: "codex.exe", cmd: "<defunct>" },
+      { id: 4, name: "notcodex", cmd: "" },
+      { id: 5, name: "alsonotcodex.exe", cmd: "" },
     ];
 
     beforeEach(() => {
@@ -80,7 +80,7 @@ describe("ProcessControl", () => {
       );
     });
 
-    it("stops the first codex process", async () => {
+    it("calls stopProcess with pid of first codex process", async () => {
       const pid = 12345;
       processControl.getCodexProcesses.mockResolvedValue([
         { pid: pid },
@@ -88,21 +88,58 @@ describe("ProcessControl", () => {
         { pid: 222 },
       ]);
 
+      processControl.stopProcess = vi.fn();
       await processControl.stopCodexProcess();
 
+      expect(processControl.stopProcess).toHaveBeenCalledWith(pid);
+    });
+  });
+
+  describe("stopProcess", () => {
+    const pid = 234;
+    beforeEach(() => {
+      processControl.isProcessRunning = vi.fn();
+    });
+
+    it("stops the process", async () => {
+      processControl.isProcessRunning.mockResolvedValue(false);
+
+      await processControl.stopProcess(pid);
+
       expect(mockOsService.stopProcess).toHaveBeenCalledWith(pid);
     });
 
     it("sleeps", async () => {
-      processControl.getCodexProcesses.mockResolvedValue([
-        { pid: 111 },
-        { pid: 222 },
-      ]);
+      processControl.isProcessRunning.mockResolvedValue(false);
 
-      await processControl.stopCodexProcess();
+      await processControl.stopProcess(pid);
 
       expect(processControl.sleep).toHaveBeenCalled();
     });
+
+    it("terminates process if it is running after stop", async () => {
+      processControl.isProcessRunning.mockResolvedValue(true);
+
+      await processControl.stopProcess(pid);
+
+      expect(mockOsService.terminateProcess).toHaveBeenCalledWith(pid);
+    });
+  });
+
+  describe("isProcessRunning", () => {
+    const pid = 345;
+
+    it("is true when process is in process list", async () => {
+      mockOsService.listProcesses.mockResolvedValue([{ pid: pid }]);
+
+      expect(await processControl.isProcessRunning(pid)).toBeTruthy();
+    });
+
+    it("is false when process is not in process list", async () => {
+      mockOsService.listProcesses.mockResolvedValue([{ pid: pid + 11 }]);
+
+      expect(await processControl.isProcessRunning(pid)).toBeFalsy();
+    });
   });
 
   describe("startCodexProcess", () => {

src/main.js

@@ -116,6 +116,7 @@ export async function main() {
   );
   const installMenu = new InstallMenu(
     uiService,
+    new MenuLoop(),
     configService,
     pathSelector,
     installer,

src/services/fsService.js

@@ -15,8 +15,7 @@ export class FsService {
             if (!fs.lstatSync(mount).isFile()) {
               mountPoints.push(mount);
             }
-          } catch {
-          }
+          } catch {}
         }
       });
     });
@@ -24,7 +23,7 @@ export class FsService {
     if (mountPoints.length < 1) {
       // In certain containerized environments, the devices don't reveal any
       // useful mounts. We'll proceed under the assumption that '/' is valid here.
-      return ['/'];
+      return ["/"];
     }
     return mountPoints;
   };

src/services/osService.js

@@ -32,5 +32,5 @@ export class OsService {
 
   terminateProcess = (pid) => {
     process.kill(pid, "SIGTERM");
-  }
+  };
 }

src/ui/configMenu.test.js

@@ -2,10 +2,7 @@ import { describe, beforeEach, it, expect, vi } from "vitest";
 import { ConfigMenu } from "./configMenu.js";
 import { mockUiService } from "../__mocks__/service.mocks.js";
 import { mockConfigService } from "../__mocks__/service.mocks.js";
-import {
-  mockNumberSelector,
-  mockMenuLoop,
-} from "../__mocks__/utils.mocks.js";
+import { mockNumberSelector, mockMenuLoop } from "../__mocks__/utils.mocks.js";
 
 describe("ConfigMenu", () => {
   const config = {

src/ui/installMenu.js

@@ -1,13 +1,20 @@
 export class InstallMenu {
-  constructor(uiService, configService, pathSelector, installer) {
+  constructor(uiService, menuLoop, configService, pathSelector, installer) {
     this.ui = uiService;
+    this.loop = menuLoop;
     this.configService = configService;
     this.config = configService.get();
     this.pathSelector = pathSelector;
     this.installer = installer;
+
+    this.loop.initialize(this.showMenu);
   }
 
   show = async () => {
+    await this.loop.showLoop();
+  };
+
+  showMenu = async () => {
     if (await this.installer.isCodexInstalled()) {
       await this.showUninstallMenu();
     } else {
@@ -86,14 +93,18 @@ export class InstallMenu {
   };
 
   performInstall = async () => {
+    this.loop.stopLoop();
     await this.installer.installCodex(this);
   };
 
   performUninstall = async () => {
+    this.loop.stopLoop();
     this.installer.uninstallCodex();
   };
 
-  doNothing = async () => {};
+  doNothing = async () => {
+    this.loop.stopLoop();
+  };
 
   // Progress callbacks from installer module:
   installStarts = () => {

src/ui/installMenu.test.js

@@ -2,7 +2,7 @@ import { describe, beforeEach, it, expect, vi } from "vitest";
 import { InstallMenu } from "./installMenu.js";
 import { mockUiService } from "../__mocks__/service.mocks.js";
 import { mockConfigService } from "../__mocks__/service.mocks.js";
-import { mockPathSelector } from "../__mocks__/utils.mocks.js";
+import { mockMenuLoop, mockPathSelector } from "../__mocks__/utils.mocks.js";
 import { mockInstaller } from "../__mocks__/handler.mocks.js";
 
 describe("InstallMenu", () => {
@@ -17,13 +17,30 @@ describe("InstallMenu", () => {
 
     installMenu = new InstallMenu(
       mockUiService,
+      mockMenuLoop,
       mockConfigService,
       mockPathSelector,
       mockInstaller,
     );
   });
 
+  describe("constructor", () => {
+    it("initializes the menu loop with the showMenu function", () => {
+      expect(mockMenuLoop.initialize).toHaveBeenCalledWith(
+        installMenu.showMenu,
+      );
+    });
+  });
+
   describe("show", () => {
+    it("starts the menu loop", async () => {
+      await installMenu.show();
+
+      expect(mockMenuLoop.showLoop).toHaveBeenCalled();
+    });
+  });
+
+  describe("showMenu", () => {
     beforeEach(() => {
       installMenu.showInstallMenu = vi.fn();
       installMenu.showUninstallMenu = vi.fn();
@@ -32,7 +49,7 @@ describe("InstallMenu", () => {
     it("shows uninstall menu when codex is installed", async () => {
       mockInstaller.isCodexInstalled.mockResolvedValue(true);
 
-      await installMenu.show();
+      await installMenu.showMenu();
 
       expect(installMenu.showUninstallMenu).toHaveBeenCalled();
     });
@@ -40,7 +57,7 @@ describe("InstallMenu", () => {
     it("shows install menu when codex is not installed", async () => {
       mockInstaller.uninstallCodex.mockResolvedValue(false);
 
-      await installMenu.show();
+      await installMenu.showMenu();
 
       expect(installMenu.showInstallMenu).toHaveBeenCalled();
     });
@@ -140,12 +157,20 @@ describe("InstallMenu", () => {
     await installMenu.performInstall();
 
     expect(mockInstaller.installCodex).toHaveBeenCalledWith(installMenu);
+    expect(mockMenuLoop.stopLoop).toHaveBeenCalled();
   });
 
   it("calls installer for deinstallation", async () => {
     await installMenu.performUninstall();
 
     expect(mockInstaller.uninstallCodex).toHaveBeenCalled();
+    expect(mockMenuLoop.stopLoop).toHaveBeenCalled();
+  });
+
+  it("stops the menu loop when nothing is selected", async () => {
+    await installMenu.doNothing();
+
+    expect(mockMenuLoop.stopLoop).toHaveBeenCalled();
   });
 
   describe("process callback handling", () => {