fix: properly shutdown all services using preDestroy and unbindAll (#646)

Author: jonathanlab

apps/twig/src/main/services/agent/service.ts

@@ -12,7 +12,7 @@ import {
 } from "@agentclientprotocol/sdk";
 import { Agent, getLlmGatewayUrl, type OnLogCallback } from "@posthog/agent";
 import { app } from "electron";
-import { injectable } from "inversify";
+import { injectable, preDestroy } from "inversify";
 import type { AcpMessage } from "../../../shared/types/session-events.js";
 import { logger } from "../../lib/logger.js";
 import { TypedEventEmitter } from "../../lib/typed-event-emitter.js";
@@ -748,6 +748,7 @@ For git operations while detached:
     return `Your worktree is back on branch \`${context.branchName}\`. Normal git commands work again.`;
   }
 
+  @preDestroy()
   async cleanupAll(): Promise<void> {
     log.info("Cleaning up all agent sessions", {
       sessionCount: this.sessions.size,

apps/twig/src/main/services/app-lifecycle/service.test.ts

@@ -1,13 +1,13 @@
 import { beforeEach, describe, expect, it, vi } from "vitest";
 import { AppLifecycleService } from "./service.js";
 
-const { mockApp, mockAgentService, mockTrackAppEvent, mockShutdownPostHog } =
+const { mockApp, mockContainer, mockTrackAppEvent, mockShutdownPostHog } =
   vi.hoisted(() => ({
     mockApp: {
       exit: vi.fn(),
     },
-    mockAgentService: {
-      cleanupAll: vi.fn(() => Promise.resolve()),
+    mockContainer: {
+      unbindAll: vi.fn(() => Promise.resolve()),
     },
     mockTrackAppEvent: vi.fn(),
     mockShutdownPostHog: vi.fn(() => Promise.resolve()),
@@ -33,10 +33,8 @@ vi.mock("../posthog-analytics.js", () => ({
   shutdownPostHog: mockShutdownPostHog,
 }));
 
-vi.mock("../../di/tokens.js", () => ({
-  MAIN_TOKENS: {
-    AgentService: Symbol.for("AgentService"),
-  },
+vi.mock("../../di/container.js", () => ({
+  container: mockContainer,
 }));
 
 vi.mock("../../../types/analytics.js", () => ({
@@ -50,11 +48,7 @@ describe("AppLifecycleService", () => {
 
   beforeEach(() => {
     vi.clearAllMocks();
-
     service = new AppLifecycleService();
-    (
-      service as unknown as { agentService: typeof mockAgentService }
-    ).agentService = mockAgentService;
   });
 
   describe("isQuittingForUpdate", () => {
@@ -69,9 +63,9 @@ describe("AppLifecycleService", () => {
   });
 
   describe("shutdown", () => {
-    it("cleans up agents", async () => {
+    it("unbinds all container services", async () => {
       await service.shutdown();
-      expect(mockAgentService.cleanupAll).toHaveBeenCalled();
+      expect(mockContainer.unbindAll).toHaveBeenCalled();
     });
 
     it("tracks app quit event", async () => {
@@ -87,8 +81,8 @@ describe("AppLifecycleService", () => {
     it("calls cleanup steps in order", async () => {
       const callOrder: string[] = [];
 
-      mockAgentService.cleanupAll.mockImplementation(async () => {
-        callOrder.push("cleanupAll");
+      mockContainer.unbindAll.mockImplementation(async () => {
+        callOrder.push("unbindAll");
       });
       mockTrackAppEvent.mockImplementation(() => {
         callOrder.push("trackAppEvent");
@@ -100,16 +94,14 @@ describe("AppLifecycleService", () => {
       await service.shutdown();
 
       expect(callOrder).toEqual([
-        "cleanupAll",
+        "unbindAll",
         "trackAppEvent",
         "shutdownPostHog",
       ]);
     });
 
-    it("continues shutdown if agent cleanup fails", async () => {
-      mockAgentService.cleanupAll.mockRejectedValue(
-        new Error("cleanup failed"),
-      );
+    it("continues shutdown if container unbind fails", async () => {
+      mockContainer.unbindAll.mockRejectedValue(new Error("unbind failed"));
 
       await service.shutdown();
 
@@ -120,7 +112,6 @@ describe("AppLifecycleService", () => {
     it("continues shutdown if PostHog shutdown fails", async () => {
       mockShutdownPostHog.mockRejectedValue(new Error("posthog failed"));
 
-      // Should not throw
       await expect(service.shutdown()).resolves.toBeUndefined();
     });
   });
@@ -129,16 +120,16 @@ describe("AppLifecycleService", () => {
     it("calls shutdown before exit", async () => {
       const callOrder: string[] = [];
 
-      mockAgentService.cleanupAll.mockImplementation(async () => {
-        callOrder.push("cleanupAll");
+      mockContainer.unbindAll.mockImplementation(async () => {
+        callOrder.push("unbindAll");
       });
       mockApp.exit.mockImplementation(() => {
         callOrder.push("exit");
       });
 
       await service.shutdownAndExit();
 
-      expect(callOrder[0]).toBe("cleanupAll");
+      expect(callOrder[0]).toBe("unbindAll");
       expect(callOrder[callOrder.length - 1]).toBe("exit");
     });
 

apps/twig/src/main/services/app-lifecycle/service.ts

@@ -1,22 +1,14 @@
 import { app } from "electron";
-import { inject, injectable } from "inversify";
+import { injectable } from "inversify";
 import { ANALYTICS_EVENTS } from "../../../types/analytics.js";
-import { MAIN_TOKENS } from "../../di/tokens.js";
+import { container } from "../../di/container.js";
 import { logger } from "../../lib/logger.js";
-import type { AgentService } from "../agent/service.js";
 import { shutdownPostHog, trackAppEvent } from "../posthog-analytics.js";
-import type { ShellService } from "../shell/service.js";
 
 const log = logger.scope("app-lifecycle");
 
 @injectable()
 export class AppLifecycleService {
-  @inject(MAIN_TOKENS.AgentService)
-  private agentService!: AgentService;
-
-  @inject(MAIN_TOKENS.ShellService)
-  private shellService!: ShellService;
-
   private _isQuittingForUpdate = false;
 
   get isQuittingForUpdate(): boolean {
@@ -31,15 +23,9 @@ export class AppLifecycleService {
     log.info("Performing graceful shutdown...");
 
     try {
-      this.shellService.destroyAll();
-    } catch (error) {
-      log.error("Error cleaning up ShellService during shutdown", error);
-    }
-
-    try {
-      await this.agentService.cleanupAll();
+      await container.unbindAll();
     } catch (error) {
-      log.error("Error cleaning up agents during shutdown", error);
+      log.error("Error during container unbind", error);
     }
 
     trackAppEvent(ANALYTICS_EVENTS.APP_QUIT);

apps/twig/src/main/services/connectivity/service.ts

@@ -1,5 +1,5 @@
 import { net } from "electron";
-import { injectable, postConstruct } from "inversify";
+import { injectable, postConstruct, preDestroy } from "inversify";
 import { getBackoffDelay } from "../../../shared/utils/backoff.js";
 import { logger } from "../../lib/logger.js";
 import { TypedEventEmitter } from "../../lib/typed-event-emitter.js";
@@ -102,4 +102,12 @@ export class ConnectivityService extends TypedEventEmitter<ConnectivityEvents> {
       this.schedulePoll();
     }, interval);
   }
+
+  @preDestroy()
+  stopPolling(): void {
+    if (this.pollTimeoutId) {
+      clearTimeout(this.pollTimeoutId);
+      this.pollTimeoutId = null;
+    }
+  }
 }

apps/twig/src/main/services/file-watcher/service.ts

@@ -3,7 +3,7 @@ import fs from "node:fs/promises";
 import path from "node:path";
 import * as watcher from "@parcel/watcher";
 import { app } from "electron";
-import { injectable } from "inversify";
+import { injectable, preDestroy } from "inversify";
 import { logger } from "../../lib/logger.js";
 import { TypedEventEmitter } from "../../lib/typed-event-emitter.js";
 import {
@@ -89,6 +89,15 @@ export class FileWatcherService extends TypedEventEmitter<FileWatcherEvents> {
     this.watchers.delete(repoPath);
   }
 
+  @preDestroy()
+  async shutdown(): Promise<void> {
+    log.info("Shutting down file watcher service", {
+      watcherCount: this.watchers.size,
+    });
+    const repoPaths = Array.from(this.watchers.keys());
+    await Promise.all(repoPaths.map((repoPath) => this.stopWatching(repoPath)));
+  }
+
   private get snapshotsDir(): string {
     return path.join(app.getPath("userData"), "snapshots");
   }

apps/twig/src/main/services/focus/service.ts

@@ -3,7 +3,7 @@ import * as fs from "node:fs/promises";
 import * as path from "node:path";
 import { promisify } from "node:util";
 import * as watcher from "@parcel/watcher";
-import { injectable } from "inversify";
+import { injectable, preDestroy } from "inversify";
 import { logger } from "../../lib/logger";
 import { TypedEventEmitter } from "../../lib/typed-event-emitter";
 import { type FocusSession, focusStore } from "../../utils/store.js";
@@ -108,6 +108,7 @@ export class FocusService extends TypedEventEmitter<FocusServiceEvents> {
     });
   }
 
+  @preDestroy()
   async stopWatchingMainRepo(): Promise<void> {
     if (this.mainRepoWatcher) {
       await this.mainRepoWatcher.unsubscribe();

apps/twig/src/main/services/focus/sync-service.ts

@@ -2,7 +2,7 @@ import fs from "node:fs/promises";
 import path from "node:path";
 import * as watcher from "@parcel/watcher";
 import ignore, { type Ignore } from "ignore";
-import { injectable } from "inversify";
+import { injectable, preDestroy } from "inversify";
 import { logger } from "../../lib/logger.js";
 import { git, withGitLock } from "./service.js";
 
@@ -149,6 +149,7 @@ export class FocusSyncService {
     }
   }
 
+  @preDestroy()
   async stopSync(): Promise<void> {
     if (this.pending.timer) {
       clearTimeout(this.pending.timer);

apps/twig/src/main/services/shell/service.ts

@@ -2,7 +2,7 @@ import { exec, execSync } from "node:child_process";
 import { existsSync } from "node:fs";
 import { homedir, platform } from "node:os";
 import path from "node:path";
-import { injectable } from "inversify";
+import { injectable, preDestroy } from "inversify";
 import * as pty from "node-pty";
 import { logger } from "../../lib/logger.js";
 import { TypedEventEmitter } from "../../lib/typed-event-emitter.js";
@@ -215,6 +215,7 @@ export class ShellService extends TypedEventEmitter<ShellEvents> {
    * Destroy all active shell sessions.
    * Used during application shutdown to ensure all child processes are cleaned up.
    */
+  @preDestroy()
   destroyAll(): void {
     log.info(`Destroying all shell sessions (${this.sessions.size} active)`);
     for (const sessionId of this.sessions.keys()) {

apps/twig/src/main/services/updates/service.ts

@@ -1,5 +1,5 @@
 import { app, autoUpdater } from "electron";
-import { inject, injectable, postConstruct } from "inversify";
+import { inject, injectable, postConstruct, preDestroy } from "inversify";
 import { MAIN_TOKENS } from "../../di/tokens.js";
 import { logger } from "../../lib/logger.js";
 import { TypedEventEmitter } from "../../lib/typed-event-emitter.js";
@@ -30,6 +30,7 @@ export class UpdatesService extends TypedEventEmitter<UpdatesEvents> {
   private pendingNotification = false;
   private checkingForUpdates = false;
   private checkTimeoutId: ReturnType<typeof setTimeout> | null = null;
+  private checkIntervalId: ReturnType<typeof setInterval> | null = null;
   private downloadedVersion: string | null = null;
   private initialized = false;
 
@@ -155,7 +156,10 @@ export class UpdatesService extends TypedEventEmitter<UpdatesEvents> {
     this.performCheck();
 
     // Set up periodic checks
-    setInterval(() => this.performCheck(), UpdatesService.CHECK_INTERVAL_MS);
+    this.checkIntervalId = setInterval(
+      () => this.performCheck(),
+      UpdatesService.CHECK_INTERVAL_MS,
+    );
   }
 
   private handleError(error: Error): void {
@@ -265,4 +269,13 @@ export class UpdatesService extends TypedEventEmitter<UpdatesEvents> {
       this.checkTimeoutId = null;
     }
   }
+
+  @preDestroy()
+  shutdown(): void {
+    this.clearCheckTimeout();
+    if (this.checkIntervalId) {
+      clearInterval(this.checkIntervalId);
+      this.checkIntervalId = null;
+    }
+  }
 }