feat: Implement AppLifecycleService and refactor to fix auto updater (#530)

charlesvien · web-flow · commit 844e1f4e0644 · 2026-01-20T21:35:22.000-08:00
## Alright here's what I found in the cave...

**the bug:** when `autoUpdater.quitAndInstall()` was invoked, it triggered the `before-quit` event, but the handler would intercept it, prevent the default behavior, and forcibly exit the app before the updater could complete its installation flow.

**solution:** this PR introduces a new `AppLifecycleService` that centralizes all shutdown logic and tracks an `isQuittingForUpdate` flag. Now when installing an update, the `UpdatesService` first calls `lifecycleService.shutdown()` to gracefully clean up, then sets `lifecycleService.setQuittingForUpdate()` before calling `quitAndInstall()`. The `before-quit` handler now checks this flag and returns early if set, allowing the auto updater to proceed unblocked. This separates the "normal quit with cleanup" path from the "quit for update" path  
  
## NOTE: There is a different event sequence for `quitAndInstall()`
_Excerpt taken from electron docs_

The Event Sequence for `quitAndInstall()`

**Normal quit:**

1. `before-quit` emitted
2. Windows receive `close` events
3. Windows close
4. `will-quit` emitted
5. App exits

**`quitAndInstall()`** **sequence (different!):**

1. All windows receive `close` events first
2. Windows close
3. `before-quit` emitted **after** windows are closed
4. Installer runs
diff --git a/apps/twig/src/main/di/container.ts b/apps/twig/src/main/di/container.ts
@@ -1,6 +1,7 @@
 import "reflect-metadata";
 import { Container } from "inversify";
 import { AgentService } from "../services/agent/service.js";
+import { AppLifecycleService } from "../services/app-lifecycle/service.js";
 import { ConnectivityService } from "../services/connectivity/service.js";
 import { ContextMenuService } from "../services/context-menu/service.js";
 import { DeepLinkService } from "../services/deep-link/service.js";
@@ -23,6 +24,7 @@ export const container = new Container({
 });
 
 container.bind(MAIN_TOKENS.AgentService).to(AgentService);
+container.bind(MAIN_TOKENS.AppLifecycleService).to(AppLifecycleService);
 container.bind(MAIN_TOKENS.ConnectivityService).to(ConnectivityService);
 container.bind(MAIN_TOKENS.ContextMenuService).to(ContextMenuService);
 container.bind(MAIN_TOKENS.DeepLinkService).to(DeepLinkService);
diff --git a/apps/twig/src/main/di/tokens.ts b/apps/twig/src/main/di/tokens.ts
@@ -7,6 +7,7 @@
 export const MAIN_TOKENS = Object.freeze({
   // Services
   AgentService: Symbol.for("Main.AgentService"),
+  AppLifecycleService: Symbol.for("Main.AppLifecycleService"),
   ConnectivityService: Symbol.for("Main.ConnectivityService"),
   ContextMenuService: Symbol.for("Main.ContextMenuService"),
   DockBadgeService: Symbol.for("Main.DockBadgeService"),
diff --git a/apps/twig/src/main/index.ts b/apps/twig/src/main/index.ts
@@ -27,19 +27,18 @@ import "./lib/logger";
 import { ANALYTICS_EVENTS } from "../types/analytics.js";
 import { container } from "./di/container.js";
 import { MAIN_TOKENS } from "./di/tokens.js";
-import type { AgentService } from "./services/agent/service.js";
 import type { DockBadgeService } from "./services/dock-badge/service.js";
 import type { UIService } from "./services/ui/service.js";
 import { setMainWindowGetter } from "./trpc/context.js";
 import { trpcRouter } from "./trpc/index.js";
 
 import "./services/index.js";
+import type { AppLifecycleService } from "./services/app-lifecycle/service.js";
 import type { DeepLinkService } from "./services/deep-link/service.js";
 import type { ExternalAppsService } from "./services/external-apps/service.js";
 import type { OAuthService } from "./services/oauth/service.js";
 import {
   initializePostHog,
-  shutdownPostHog,
   trackAppEvent,
 } from "./services/posthog-analytics.js";
 import type { TaskLinkService } from "./services/task-link/service";
@@ -365,19 +364,26 @@ app.whenReady().then(() => {
 
 app.on("window-all-closed", async () => {
   if (process.platform !== "darwin") {
-    trackAppEvent(ANALYTICS_EVENTS.APP_QUIT);
-    await shutdownPostHog();
+    const lifecycleService = container.get<AppLifecycleService>(
+      MAIN_TOKENS.AppLifecycleService,
+    );
+    await lifecycleService.shutdown();
     app.quit();
   }
 });
 
 app.on("before-quit", async (event) => {
+  const lifecycleService = container.get<AppLifecycleService>(
+    MAIN_TOKENS.AppLifecycleService,
+  );
+
+  // If quitting to install an update, don't block - let the updater handle it
+  if (lifecycleService.isQuittingForUpdate) {
+    return;
+  }
+
   event.preventDefault();
-  const agentService = container.get<AgentService>(MAIN_TOKENS.AgentService);
-  await agentService.cleanupAll();
-  trackAppEvent(ANALYTICS_EVENTS.APP_QUIT);
-  await shutdownPostHog();
-  app.exit(0);
+  await lifecycleService.shutdownAndExit();
 });
 
 app.on("activate", () => {
@@ -389,8 +395,10 @@ app.on("activate", () => {
 // Handle process signals to ensure clean shutdown
 const handleShutdownSignal = async (_signal: string) => {
   try {
-    const agentService = container.get<AgentService>(MAIN_TOKENS.AgentService);
-    await agentService.cleanupAll();
+    const lifecycleService = container.get<AppLifecycleService>(
+      MAIN_TOKENS.AppLifecycleService,
+    );
+    await lifecycleService.shutdown();
   } catch (_err) {}
   process.exit(0);
 };
@@ -402,16 +410,20 @@ process.on("SIGHUP", () => handleShutdownSignal("SIGHUP"));
 // Handle uncaught exceptions to attempt cleanup before crash
 process.on("uncaughtException", async (_error) => {
   try {
-    const agentService = container.get<AgentService>(MAIN_TOKENS.AgentService);
-    await agentService.cleanupAll();
+    const lifecycleService = container.get<AppLifecycleService>(
+      MAIN_TOKENS.AppLifecycleService,
+    );
+    await lifecycleService.shutdown();
   } catch (_cleanupErr) {}
   process.exit(1);
 });
 
 process.on("unhandledRejection", async (_reason) => {
   try {
-    const agentService = container.get<AgentService>(MAIN_TOKENS.AgentService);
-    await agentService.cleanupAll();
+    const lifecycleService = container.get<AppLifecycleService>(
+      MAIN_TOKENS.AppLifecycleService,
+    );
+    await lifecycleService.shutdown();
   } catch (_cleanupErr) {}
   process.exit(1);
 });
diff --git a/apps/twig/src/main/services/app-lifecycle/service.test.ts b/apps/twig/src/main/services/app-lifecycle/service.test.ts
@@ -0,0 +1,150 @@
+import { beforeEach, describe, expect, it, vi } from "vitest";
+import { AppLifecycleService } from "./service.js";
+
+const { mockApp, mockAgentService, mockTrackAppEvent, mockShutdownPostHog } =
+  vi.hoisted(() => ({
+    mockApp: {
+      exit: vi.fn(),
+    },
+    mockAgentService: {
+      cleanupAll: vi.fn(() => Promise.resolve()),
+    },
+    mockTrackAppEvent: vi.fn(),
+    mockShutdownPostHog: vi.fn(() => Promise.resolve()),
+  }));
+
+vi.mock("electron", () => ({
+  app: mockApp,
+}));
+
+vi.mock("../../lib/logger.js", () => ({
+  logger: {
+    scope: () => ({
+      info: vi.fn(),
+      error: vi.fn(),
+      warn: vi.fn(),
+      debug: vi.fn(),
+    }),
+  },
+}));
+
+vi.mock("../posthog-analytics.js", () => ({
+  trackAppEvent: mockTrackAppEvent,
+  shutdownPostHog: mockShutdownPostHog,
+}));
+
+vi.mock("../../di/tokens.js", () => ({
+  MAIN_TOKENS: {
+    AgentService: Symbol.for("AgentService"),
+  },
+}));
+
+vi.mock("../../../types/analytics.js", () => ({
+  ANALYTICS_EVENTS: {
+    APP_QUIT: "app_quit",
+  },
+}));
+
+describe("AppLifecycleService", () => {
+  let service: AppLifecycleService;
+
+  beforeEach(() => {
+    vi.clearAllMocks();
+
+    service = new AppLifecycleService();
+    (
+      service as unknown as { agentService: typeof mockAgentService }
+    ).agentService = mockAgentService;
+  });
+
+  describe("isQuittingForUpdate", () => {
+    it("returns false by default", () => {
+      expect(service.isQuittingForUpdate).toBe(false);
+    });
+
+    it("returns true after setQuittingForUpdate is called", () => {
+      service.setQuittingForUpdate();
+      expect(service.isQuittingForUpdate).toBe(true);
+    });
+  });
+
+  describe("shutdown", () => {
+    it("cleans up agents", async () => {
+      await service.shutdown();
+      expect(mockAgentService.cleanupAll).toHaveBeenCalled();
+    });
+
+    it("tracks app quit event", async () => {
+      await service.shutdown();
+      expect(mockTrackAppEvent).toHaveBeenCalledWith("app_quit");
+    });
+
+    it("shuts down PostHog", async () => {
+      await service.shutdown();
+      expect(mockShutdownPostHog).toHaveBeenCalled();
+    });
+
+    it("calls cleanup steps in order", async () => {
+      const callOrder: string[] = [];
+
+      mockAgentService.cleanupAll.mockImplementation(async () => {
+        callOrder.push("cleanupAll");
+      });
+      mockTrackAppEvent.mockImplementation(() => {
+        callOrder.push("trackAppEvent");
+      });
+      mockShutdownPostHog.mockImplementation(async () => {
+        callOrder.push("shutdownPostHog");
+      });
+
+      await service.shutdown();
+
+      expect(callOrder).toEqual([
+        "cleanupAll",
+        "trackAppEvent",
+        "shutdownPostHog",
+      ]);
+    });
+
+    it("continues shutdown if agent cleanup fails", async () => {
+      mockAgentService.cleanupAll.mockRejectedValue(
+        new Error("cleanup failed"),
+      );
+
+      await service.shutdown();
+
+      expect(mockTrackAppEvent).toHaveBeenCalled();
+      expect(mockShutdownPostHog).toHaveBeenCalled();
+    });
+
+    it("continues shutdown if PostHog shutdown fails", async () => {
+      mockShutdownPostHog.mockRejectedValue(new Error("posthog failed"));
+
+      // Should not throw
+      await expect(service.shutdown()).resolves.toBeUndefined();
+    });
+  });
+
+  describe("shutdownAndExit", () => {
+    it("calls shutdown before exit", async () => {
+      const callOrder: string[] = [];
+
+      mockAgentService.cleanupAll.mockImplementation(async () => {
+        callOrder.push("cleanupAll");
+      });
+      mockApp.exit.mockImplementation(() => {
+        callOrder.push("exit");
+      });
+
+      await service.shutdownAndExit();
+
+      expect(callOrder[0]).toBe("cleanupAll");
+      expect(callOrder[callOrder.length - 1]).toBe("exit");
+    });
+
+    it("exits with code 0", async () => {
+      await service.shutdownAndExit();
+      expect(mockApp.exit).toHaveBeenCalledWith(0);
+    });
+  });
+});
diff --git a/apps/twig/src/main/services/app-lifecycle/service.ts b/apps/twig/src/main/services/app-lifecycle/service.ts
@@ -0,0 +1,50 @@
+import { app } from "electron";
+import { inject, injectable } from "inversify";
+import { ANALYTICS_EVENTS } from "../../../types/analytics.js";
+import { MAIN_TOKENS } from "../../di/tokens.js";
+import { logger } from "../../lib/logger.js";
+import type { AgentService } from "../agent/service.js";
+import { shutdownPostHog, trackAppEvent } from "../posthog-analytics.js";
+
+const log = logger.scope("app-lifecycle");
+
+@injectable()
+export class AppLifecycleService {
+  @inject(MAIN_TOKENS.AgentService)
+  private agentService!: AgentService;
+
+  private _isQuittingForUpdate = false;
+
+  get isQuittingForUpdate(): boolean {
+    return this._isQuittingForUpdate;
+  }
+
+  setQuittingForUpdate(): void {
+    this._isQuittingForUpdate = true;
+  }
+
+  async shutdown(): Promise<void> {
+    log.info("Performing graceful shutdown...");
+
+    try {
+      await this.agentService.cleanupAll();
+    } catch (error) {
+      log.error("Error cleaning up agents during shutdown", error);
+    }
+
+    trackAppEvent(ANALYTICS_EVENTS.APP_QUIT);
+
+    try {
+      await shutdownPostHog();
+    } catch (error) {
+      log.error("Error shutting down PostHog", error);
+    }
+
+    log.info("Graceful shutdown complete");
+  }
+
+  async shutdownAndExit(): Promise<void> {
+    await this.shutdown();
+    app.exit(0);
+  }
+}
diff --git a/apps/twig/src/main/services/updates/service.test.ts b/apps/twig/src/main/services/updates/service.test.ts
@@ -2,7 +2,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 import { UpdatesEvent } from "./schemas.js";
 
 // Use vi.hoisted to ensure mocks are available when vi.mock is hoisted
-const { mockApp, mockAutoUpdater } = vi.hoisted(() => ({
+const { mockApp, mockAutoUpdater, mockLifecycleService } = vi.hoisted(() => ({
   mockAutoUpdater: {
     setFeedURL: vi.fn(),
     checkForUpdates: vi.fn(),
@@ -15,6 +15,10 @@ const { mockApp, mockAutoUpdater } = vi.hoisted(() => ({
     on: vi.fn(),
     whenReady: vi.fn(() => Promise.resolve()),
   },
+  mockLifecycleService: {
+    shutdown: vi.fn(() => Promise.resolve()),
+    setQuittingForUpdate: vi.fn(),
+  },
 }));
 
 vi.mock("electron", () => ({
@@ -33,6 +37,12 @@ vi.mock("../../lib/logger.js", () => ({
   },
 }));
 
+vi.mock("../../di/tokens.js", () => ({
+  MAIN_TOKENS: {
+    AppLifecycleService: Symbol.for("AppLifecycleService"),
+  },
+}));
+
 // Import the service after mocks are set up
 import { UpdatesService } from "./service.js";
 
@@ -72,6 +82,10 @@ describe("UpdatesService", () => {
     delete process.env.ELECTRON_DISABLE_AUTO_UPDATE;
 
     service = new UpdatesService();
+    // Manually inject the mock lifecycle service (normally done by DI container)
+    (
+      service as unknown as { lifecycleService: typeof mockLifecycleService }
+    ).lifecycleService = mockLifecycleService;
   });
 
   afterEach(() => {
@@ -292,8 +306,8 @@ describe("UpdatesService", () => {
   });
 
   describe("installUpdate", () => {
-    it("returns false when no update is ready", () => {
-      const result = service.installUpdate();
+    it("returns false when no update is ready", async () => {
+      const result = await service.installUpdate();
       expect(result).toEqual({ installed: false });
     });
 
@@ -309,8 +323,10 @@ describe("UpdatesService", () => {
         updateDownloadedHandler({}, "Release notes", "v2.0.0");
       }
 
-      const result = service.installUpdate();
+      const result = await service.installUpdate();
       expect(result).toEqual({ installed: true });
+      expect(mockLifecycleService.shutdown).toHaveBeenCalled();
+      expect(mockLifecycleService.setQuittingForUpdate).toHaveBeenCalled();
       expect(mockAutoUpdater.quitAndInstall).toHaveBeenCalled();
     });
 
@@ -330,7 +346,7 @@ describe("UpdatesService", () => {
         throw new Error("Failed to install");
       });
 
-      const result = service.installUpdate();
+      const result = await service.installUpdate();
       expect(result).toEqual({ installed: false });
     });
   });
diff --git a/apps/twig/src/main/services/updates/service.ts b/apps/twig/src/main/services/updates/service.ts
