Refactor OC shutdown process, make it actually work

src/ProcessManager.ts

@@ -85,7 +85,7 @@ export class ProcessManager {
         cwd: this.projectDirectory,
         env: { ...process.env, NODE_USE_SYSTEM_CA: "1" },
         stdio: ["ignore", "pipe", "pipe"],
-        detached: false,
+        detached: true,
       }
     );
 
@@ -133,7 +133,7 @@ export class ProcessManager {
       return false;
     }
 
-    this.stop();
+    await this.stop();
     if (this.earlyExitCode !== null) {
       return this.setError(`Process exited unexpectedly (exit code ${this.earlyExitCode})`);
     }
@@ -143,27 +143,102 @@ export class ProcessManager {
     return this.setError("Server failed to start within timeout");
   }
 
-  stop(): void {
+  async stop(): Promise<void> {
     if (!this.process) {
       this.setState("stopped");
       return;
     }
 
+    const pid = this.process.pid;
     const proc = this.process;
-    console.log("[OpenCode] Stopping process with PID:", proc.pid);
+    
+    if (!pid) {
+      console.log("[OpenCode] No PID available, cleaning up state");
+      this.setState("stopped");
+      this.process = null;
+      return;
+    }
+    
+    console.log("[OpenCode] Stopping server process tree, PID:", pid);
 
     this.setState("stopped");
     this.process = null;
 
-    proc.kill("SIGTERM");
+    await this.killProcessTree(pid, "SIGTERM");
 
-    // Force kill after 2 seconds if still running
-    setTimeout(() => {
-      if (proc.exitCode === null && proc.signalCode === null) {
-        console.log("[OpenCode] Process still running, sending SIGKILL");
-        proc.kill("SIGKILL");
-      }
-    }, 2000);
+    const gracefulExited = await this.waitForProcessExit(proc, 2000);
+
+    if (gracefulExited) {
+      console.log("[OpenCode] Server stopped gracefully");
+      return;
+    }
+
+    console.log("[OpenCode] Process didn't exit gracefully, sending SIGKILL");
+
+    await this.killProcessTree(pid, "SIGKILL");
+
+    // Step 4: Wait for force kill (up to 3 more seconds)
+    const forceExited = await this.waitForProcessExit(proc, 3000);
+
+    if (forceExited) {
+      console.log("[OpenCode] Server stopped with SIGKILL");
+    } else {
+      console.error("[OpenCode] Failed to stop server within timeout");
+    }
+  }
+
+  private async killProcessTree(pid: number, signal: "SIGTERM" | "SIGKILL"): Promise<void> {
+    const platform = process.platform;
+
+    if (platform === "win32") {
+      // Windows: Use taskkill with /T flag to kill process tree
+      await this.execAsync(`taskkill /T /F /PID ${pid}`);
+      return;
+    }
+
+    // Unix: Try process group kill (negative PID)
+    process.kill(-pid, signal);
+    return;
+  }
+
+  private async waitForProcessExit(proc: ChildProcess, timeoutMs: number): Promise<boolean> {
+    if (proc.exitCode !== null || proc.signalCode !== null) {
+      return true; // Already exited
+    }
+
+    return new Promise((resolve) => {
+      const timeout = setTimeout(() => {
+        cleanup();
+        resolve(false);
+      }, timeoutMs);
+
+      const onExit = () => {
+        cleanup();
+        resolve(true);
+      };
+
+      const cleanup = () => {
+        clearTimeout(timeout);
+        proc.off("exit", onExit);
+        proc.off("error", onExit);
+      };
+
+      proc.once("exit", onExit);
+      proc.once("error", onExit);
+    });
+  }
+
+  private execAsync(command: string): Promise<void> {
+    return new Promise((resolve, reject) => {
+      const { exec } = require("child_process");
+      exec(command, (error: Error | null) => {
+        if (error) {
+          reject(error);
+        } else {
+          resolve();
+        }
+      });
+    });
   }
 
   private setState(state: ProcessState): void {

src/main.ts

@@ -89,11 +89,14 @@ export default class OpenCodePlugin extends Plugin {
       }
     });
 
+    // Register cleanup handlers for when Obsidian quits
+    this.registerCleanupHandlers();
+
     console.log("OpenCode plugin loaded");
   }
 
   async onunload(): Promise<void> {
-    this.stopServer();
+    await this.stopServer();
     this.app.workspace.detachLeavesOfType(OPENCODE_VIEW_TYPE);
   }
 
@@ -184,8 +187,8 @@ export default class OpenCodePlugin extends Plugin {
     return success;
   }
 
-  stopServer(): void {
-    this.processManager.stop();
+  async stopServer(): Promise<void> {
+    await this.processManager.stop();
     new Notice("OpenCode server stopped");
   }
 
@@ -431,4 +434,13 @@ export default class OpenCodePlugin extends Plugin {
     console.log("[OpenCode] Using vault path as project directory:", vaultPath);
     return vaultPath;
   }
+
+  private registerCleanupHandlers(): void {
+    this.registerEvent(
+      this.app.workspace.on("quit", () => {
+        console.log("[OpenCode] Obsidian quitting - performing sync cleanup");
+        this.stopServer();
+      })
+    );
+  }
 }

tests/ProcessManager.test.ts

@@ -22,6 +22,9 @@ function createTestSettings(port: number): OpenCodeSettings {
     projectDirectory: "",
     startupTimeout: TEST_TIMEOUT_MS,
     defaultViewLocation: "sidebar",
+    injectWorkspaceContext: true,
+    maxNotesInContext: 20,
+    maxSelectionLength: 2000,
   };
 }
 
@@ -47,7 +50,7 @@ beforeAll(async () => {
 // Cleanup after each test
 afterEach(async () => {
   if (currentManager) {
-    currentManager.stop();
+    await currentManager.stop();
     // Give process time to fully terminate
     await new Promise((resolve) => setTimeout(resolve, 500));
     currentManager = null;
@@ -108,7 +111,7 @@ describe("ProcessManager", () => {
       await currentManager.start();
       expect(currentManager.getState()).toBe("running");
 
-      currentManager.stop();
+      await currentManager.stop();
 
       expect(currentManager.getState()).toBe("stopped");
       expect(stateHistory).toContain("stopped");
@@ -151,7 +154,7 @@ describe("ProcessManager", () => {
       expect(currentManager.getState()).toBe("running");
 
       // Stop
-      currentManager.stop();
+      await currentManager.stop();
       expect(currentManager.getState()).toBe("stopped");
 
       // Wait for process to fully terminate
@@ -216,4 +219,119 @@ describe("ProcessManager", () => {
       expect(response.ok).toBe(true);
     });
   });
+
+  describe("async stop behavior", () => {
+    test("stop returns immediately when no process", async () => {
+      const port = getNextPort();
+      const settings = createTestSettings(port);
+      const stateHistory: ProcessState[] = [];
+
+      currentManager = new ProcessManager(
+        settings,
+        PROJECT_DIR,
+        (state) => stateHistory.push(state)
+      );
+
+      // Stop without starting - should not throw and set state
+      await currentManager.stop();
+
+      expect(currentManager.getState()).toBe("stopped");
+    });
+
+    test("stop completes within timeout when process exits quickly", async () => {
+      const port = getNextPort();
+      const settings = createTestSettings(port);
+
+      currentManager = new ProcessManager(
+        settings,
+        PROJECT_DIR,
+        () => {}
+      );
+
+      await currentManager.start();
+      expect(currentManager.getState()).toBe("running");
+
+      // Stop should complete within 5 seconds (2s SIGTERM wait + 3s SIGKILL wait)
+      const stopStart = Date.now();
+      await currentManager.stop();
+      const stopDuration = Date.now() - stopStart;
+
+      expect(currentManager.getState()).toBe("stopped");
+      // Should complete well before 5 second timeout
+      expect(stopDuration).toBeLessThan(6000);
+    });
+
+    test("process is fully terminated after stop completes", async () => {
+      const port = getNextPort();
+      const settings = createTestSettings(port);
+
+      currentManager = new ProcessManager(
+        settings,
+        PROJECT_DIR,
+        () => {}
+      );
+
+      await currentManager.start();
+
+      const url = currentManager.getUrl();
+      
+      await currentManager.stop();
+
+      // Wait a bit then verify server is not accessible
+      await new Promise((resolve) => setTimeout(resolve, 1000));
+
+      try {
+        const response = await fetch(`${url}/global/health`, {
+          signal: AbortSignal.timeout(1000),
+        });
+        // If we get here, server is still running - test should fail
+        expect(response.ok).toBe(false);
+      } catch (e) {
+        // Expected - server should not be accessible
+        expect(e).toBeDefined();
+      }
+    });
+  });
+
+  describe("error handling", () => {
+    test("handles missing executable gracefully", async () => {
+      const port = getNextPort();
+      const settings = createTestSettings(port);
+      settings.opencodePath = "/nonexistent/path/to/opencode";
+
+      currentManager = new ProcessManager(
+        settings,
+        PROJECT_DIR,
+        () => {}
+      );
+
+      const success = await currentManager.start();
+
+      expect(success).toBe(false);
+      expect(currentManager.getState()).toBe("error");
+      expect(currentManager.getLastError()).toContain("not found");
+    });
+
+    test("handles double stop gracefully", async () => {
+      const port = getNextPort();
+      const settings = createTestSettings(port);
+
+      currentManager = new ProcessManager(
+        settings,
+        PROJECT_DIR,
+        () => {}
+      );
+
+      await currentManager.start();
+      expect(currentManager.getState()).toBe("running");
+
+      // First stop
+      await currentManager.stop();
+      expect(currentManager.getState()).toBe("stopped");
+
+      // Second stop should not throw
+      await currentManager.stop();
+      expect(currentManager.getState()).toBe("stopped");
+    });
+  });
 });