From bf10a5ecd9133d058b4c7bf0656aaf5f4a697644 Mon Sep 17 00:00:00 2001 From: Mateusz Tymek Date: Mon, 2 Feb 2026 17:07:21 +0100 Subject: [PATCH] Refactor OC shutdown process, make it actually work --- src/ProcessManager.ts | 99 ++++++++++++++++++++++++---- src/main.ts | 18 ++++- tests/ProcessManager.test.ts | 124 ++++++++++++++++++++++++++++++++++- 3 files changed, 223 insertions(+), 18 deletions(-) diff --git a/src/ProcessManager.ts b/src/ProcessManager.ts index bbb5ac3..26bbcf4 100644 --- a/src/ProcessManager.ts +++ b/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 { 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 { + 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 { + 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 { + 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 { diff --git a/src/main.ts b/src/main.ts index a4c076e..d628ec9 100644 --- a/src/main.ts +++ b/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 { - 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 { + 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(); + }) + ); + } } diff --git a/tests/ProcessManager.test.ts b/tests/ProcessManager.test.ts index a8eedf6..31fe0ec 100644 --- a/tests/ProcessManager.test.ts +++ b/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"); + }); + }); });