fix: memory leak in classic terminal

test: add test to make sure we clean up after classic terminal

Author: AshotN

build.gradle.kts

@@ -22,6 +22,10 @@ dependencies {
     }
 
     testImplementation(kotlin("test"))
+    testImplementation("junit:junit:4.13.2")
+    intellijPlatform {
+        testFramework(org.jetbrains.intellij.platform.gradle.TestFrameworkType.Platform)
+    }
 }
 
 intellijPlatform {

src/main/kotlin/com/ashotn/opencode/companion/terminal/ClassicTuiPanel.kt

@@ -6,31 +6,30 @@ import com.intellij.openapi.Disposable
 import com.intellij.openapi.application.ApplicationManager
 import com.intellij.openapi.diagnostic.Logger
 import com.intellij.openapi.project.Project
+import com.intellij.openapi.ui.ComponentContainer
 import com.intellij.openapi.util.Disposer
 import com.intellij.terminal.ui.TerminalWidget
 import org.jetbrains.plugins.terminal.LocalTerminalDirectRunner
 import org.jetbrains.plugins.terminal.ShellStartupOptions
+import org.jetbrains.plugins.terminal.ShellTerminalWidget
 import java.awt.BorderLayout
+import java.lang.reflect.Proxy
 import javax.swing.JPanel
 
 /**
- * Hosts an embedded **classic** terminal (backed by [LocalTerminalDirectRunner] /
- * [org.jetbrains.plugins.terminal.ShellTerminalWidget]) running
- * `opencode attach <server-url>`.
+ * Hosts an embedded classic terminal running `opencode attach <server-url>`.
  *
- * This engine works on all IntelliJ versions supported by the plugin (since 233)
- * without any version-gated API.  It is wired up via [LocalTerminalDirectRunner]
- * with an explicit [ShellStartupOptions.shellCommand] override so it runs our
- * specific command instead of the user's default shell.
- *
- * The terminal is started lazily on the first call to [startIfNeeded] and lives
- * for as long as this panel's parent [Disposable] is alive.
+ * Backed by [LocalTerminalDirectRunner] with a [ShellStartupOptions.shellCommand] override.
+ * The terminal is started lazily on the first call to [startIfNeeded] and lives for as long
+ * as this panel's parent [Disposable] is alive.
  */
 class ClassicTuiPanel(
     private val project: Project,
     parentDisposable: Disposable,
     /** Invoked on the EDT when the shell process terminates. */
     private val onTerminated: (() -> Unit)? = null,
+    /** Injected process used in tests to verify the kill path without live terminal infrastructure. */
+    internal val processOverride: Process? = null,
 ) : JPanel(BorderLayout()), TuiPanel, Disposable {
 
     private var terminalWidget: TerminalWidget? = null
@@ -50,6 +49,12 @@ class ClassicTuiPanel(
     override fun startIfNeeded() {
         if (terminalWidget != null) return
 
+        //For test
+        if (processOverride != null) {
+            terminalWidget = noOpTerminalWidget()
+            return
+        }
+
         try {
             val workingDir = project.basePath ?: System.getProperty("user.home")
             val command = listOf(
@@ -109,15 +114,29 @@ class ClassicTuiPanel(
         remove(widget.component)
         revalidate()
         repaint()
-        // Disposing the widget will trigger the termination callback if the process
-        // is still alive, but since we've already cleared terminalWidget the guard
-        // (terminalWidget === widget) inside the callback will short-circuit it.
+        val process: Process? = processOverride
+            ?: widget.ttyConnectorAccessor.ttyConnector
+                ?.let { ShellTerminalWidget.getProcessTtyConnector(it)?.process }
+        val handle = process?.let { ProcessHandle.of(it.pid()).orElse(null) }
+        killProcessTree(handle)
         Disposer.dispose(widget)
     }
 
     override fun dispose() = tearDown()
 
     companion object {
         private val logger = Logger.getInstance(ClassicTuiPanel::class.java)
+
+        private fun noOpTerminalWidget(): TerminalWidget =
+            Proxy.newProxyInstance(
+                TerminalWidget::class.java.classLoader,
+                arrayOf(TerminalWidget::class.java, ComponentContainer::class.java, Disposable::class.java),
+            ) { _, method, _ ->
+                when (method.name) {
+                    "getComponent", "getPreferredFocusableComponent" -> JPanel()
+                    "dispose" -> Unit
+                    else -> if (method.returnType == Boolean::class.javaPrimitiveType) false else null
+                }
+            } as TerminalWidget
     }
 }

src/main/kotlin/com/ashotn/opencode/companion/terminal/ProcessKill.kt

@@ -0,0 +1,19 @@
+package com.ashotn.opencode.companion.terminal
+
+import com.intellij.openapi.diagnostic.Logger
+
+private val log = Logger.getInstance("com.ashotn.opencode.companion.terminal.ProcessKill")
+
+/**
+ * Forcibly kills [handle] and its entire descendant process tree.
+ * Safe to call with `null` (no-op) or on an already-dead process.
+ */
+internal fun killProcessTree(handle: ProcessHandle?) {
+    handle ?: return
+    try {
+        handle.descendants().forEach { it.destroyForcibly() }
+        handle.destroyForcibly()
+    } catch (e: Exception) {
+        log.debug("Failed to kill process tree for PID ${handle.pid()}", e)
+    }
+}

src/main/kotlin/com/ashotn/opencode/companion/terminal/ReworkedTuiPanel.kt

@@ -30,6 +30,12 @@ import javax.swing.JPanel
  *
  * The terminal is started lazily on the first call to [startIfNeeded] and lives
  * for as long as this panel's parent [Disposable] is alive.
+ *
+ * **Testability note:** unlike [ClassicTuiPanel], this panel cannot be unit-tested
+ * with a stub process. It delegates process cleanup entirely to the platform's
+ * [Content] disposal chain — there is no explicit kill path to inject into or
+ * assert against. [TerminalToolWindowTabsManager] also requires a fully initialised
+ * IDE frontend that is not available in a headless test environment.
  */
 class ReworkedTuiPanel(
     private val project: Project,
@@ -56,6 +62,7 @@ class ReworkedTuiPanel(
      */
     override fun startIfNeeded() {
         if (terminalView != null) return
+
         if (!BuildUtils.isEmbeddedTerminalSupported) return
 
         try {

src/main/kotlin/com/ashotn/opencode/companion/tui/OpenCodeTuiClient.kt

@@ -84,7 +84,7 @@ class OpenCodeTuiClient(private val project: Project) {
      *
      * If the newest known session has no messages, no tracked files, and is not busy,
      * we reuse it instead of creating a new one — there is no point in accumulating
-     * blank sessions. The [onResult] callback receives [reused]=true in that case.
+     * blank sessions. The [onResult] callback receives reused=true in that case.
      *
      * Runs all work on a pooled thread and invokes [onResult] from that thread.
      */

src/test/kotlin/com/ashotn/opencode/companion/terminal/ClassicTuiPanelProcessLeakTest.kt

@@ -0,0 +1,85 @@
+package com.ashotn.opencode.companion.terminal
+
+import com.intellij.openapi.application.ApplicationManager
+import com.intellij.openapi.util.Disposer
+import com.intellij.testFramework.fixtures.BasePlatformTestCase
+
+/**
+ * Verifies that [ClassicTuiPanel] kills the terminal process on [ClassicTuiPanel.stop]
+ * and [ClassicTuiPanel.dispose].
+ *
+ * Uses [ClassicTuiPanel.processOverride] to inject a real `sleep 3600` OS process
+ * without needing live terminal infrastructure. Assertions are purely OS-level PID checks.
+ */
+class ClassicTuiPanelProcessLeakTest : BasePlatformTestCase() {
+
+    fun `test stop kills the terminal process`() {
+        val process = spawnSleepProcess()
+        val pid = process.pid()
+        val panel = ClassicTuiPanel(project, testRootDisposable, processOverride = process)
+
+        ApplicationManager.getApplication().invokeAndWait { panel.startIfNeeded() }
+        assertTrue("pre: process must be alive (pid=$pid)", isAlive(pid))
+
+        panel.stop()
+
+        waitForDeath(pid)
+        assertFalse("process must be dead after stop() (pid=$pid)", isAlive(pid))
+    }
+
+    fun `test dispose kills the terminal process`() {
+        val process = spawnSleepProcess()
+        val pid = process.pid()
+        val panelDisposable = Disposer.newDisposable()
+        val panel = ClassicTuiPanel(project, panelDisposable, processOverride = process)
+
+        ApplicationManager.getApplication().invokeAndWait { panel.startIfNeeded() }
+        assertTrue("pre: process must be alive (pid=$pid)", isAlive(pid))
+
+        Disposer.dispose(panelDisposable) // triggers panel.dispose() → tearDown()
+
+        waitForDeath(pid)
+        assertFalse("process must be dead after dispose() (pid=$pid)", isAlive(pid))
+    }
+
+    fun `test reset cycle kills both the old and the new terminal process`() {
+        val process1 = spawnSleepProcess()
+        val pid1 = process1.pid()
+        val process2 = spawnSleepProcess()
+        val pid2 = process2.pid()
+
+        // Use a fresh panel per cycle via processOverride on each startIfNeeded call.
+        // First session: inject process1.
+        val panel = ClassicTuiPanel(project, testRootDisposable, processOverride = process1)
+        ApplicationManager.getApplication().invokeAndWait { panel.startIfNeeded() }
+        assertTrue("pre cycle 1: process1 alive (pid=$pid1)", isAlive(pid1))
+
+        panel.stop()
+        waitForDeath(pid1)
+        assertFalse("cycle 1: process1 must be dead (pid=$pid1)", isAlive(pid1))
+
+        // Second session: swap in process2 via a new panel (panel.stop() cleared terminalWidget).
+        val panel2 = ClassicTuiPanel(project, testRootDisposable, processOverride = process2)
+        ApplicationManager.getApplication().invokeAndWait { panel2.startIfNeeded() }
+        assertTrue("pre cycle 2: process2 alive (pid=$pid2)", isAlive(pid2))
+
+        panel2.stop()
+        waitForDeath(pid2)
+        assertFalse("cycle 2: process2 must be dead (pid=$pid2)", isAlive(pid2))
+
+        assertFalse("cycle 1 process must still be dead (pid=$pid1)", isAlive(pid1))
+    }
+
+    // ---- helpers ----
+
+    private fun spawnSleepProcess(): Process =
+        ProcessBuilder("sleep", "3600").redirectErrorStream(true).start()
+
+    private fun isAlive(pid: Long): Boolean =
+        ProcessHandle.of(pid).orElse(null)?.isAlive ?: false
+
+    private fun waitForDeath(pid: Long, timeoutMs: Long = 5_000) {
+        val deadline = System.currentTimeMillis() + timeoutMs
+        while (isAlive(pid) && System.currentTimeMillis() < deadline) Thread.sleep(50)
+    }
+}