diff --git a/infrastructure/tools/src/main/kotlin/com/correx/infrastructure/tools/shell/ShellTool.kt b/infrastructure/tools/src/main/kotlin/com/correx/infrastructure/tools/shell/ShellTool.kt index 9fb40f08..85faa146 100644 --- a/infrastructure/tools/src/main/kotlin/com/correx/infrastructure/tools/shell/ShellTool.kt +++ b/infrastructure/tools/src/main/kotlin/com/correx/infrastructure/tools/shell/ShellTool.kt @@ -16,10 +16,9 @@ import com.correx.core.tools.contract.ToolResult import com.correx.core.tools.contract.ValidationResult import com.correx.core.tools.process.ChildProcess import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.TimeoutCancellationException import kotlinx.coroutines.async +import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.withContext -import kotlinx.coroutines.withTimeout import kotlinx.serialization.json.Json import kotlinx.serialization.json.JsonObject import kotlinx.serialization.json.JsonPrimitive @@ -28,6 +27,7 @@ import kotlinx.serialization.json.buildJsonObject import kotlinx.serialization.json.put import kotlinx.serialization.json.putJsonObject import java.nio.file.Path +import java.util.concurrent.TimeUnit class ShellTool( private val allowedExecutables: Set = emptySet(), @@ -195,20 +195,12 @@ class ShellTool( runCatching { runCmd(request, process) }.getOrElse { - process.destroyForcibly() - if (it is TimeoutCancellationException) { - ToolResult.Failure( - invocationId = request.invocationId, - reason = "Process timed out after ${timeoutMs}ms", - recoverable = false, - ) - } else { - ToolResult.Failure( - invocationId = request.invocationId, - reason = it.message ?: "Unknown error occurred during execution", - recoverable = false, - ) - } + killTree(process) + ToolResult.Failure( + invocationId = request.invocationId, + reason = it.message ?: "Unknown error occurred during execution", + recoverable = false, + ) } } ?: ToolResult.Failure( invocationId = request.invocationId, @@ -254,11 +246,32 @@ class ShellTool( .joinToString("\n") } - private suspend fun runCmd(request: ToolRequest, process: Process): ToolResult = withTimeout(timeoutMs) { - val stdoutDeferred = async { process.inputStream.bufferedReader().use { it.readText() } } - val stderrDeferred = async { process.errorStream.bufferedReader().use { it.readText() } } + // Kill the whole process tree. destroyForcibly() signals only the direct child, so an `sh -c` + // command's grandchildren (the actual npm/tsc/…) would survive as orphans; descendants() reaches + // them. Snapshot descendants BEFORE destroying the parent — reparenting can hide them afterwards. + private fun killTree(process: Process) { + process.toHandle().descendants().forEach { it.destroyForcibly() } + process.destroyForcibly() + } - val exitCode = process.waitFor() + private suspend fun runCmd(request: ToolRequest, process: Process): ToolResult = coroutineScope { + val stdoutDeferred = async(Dispatchers.IO) { process.inputStream.bufferedReader().use { it.readText() } } + val stderrDeferred = async(Dispatchers.IO) { process.errorStream.bufferedReader().use { it.readText() } } + + // waitFor(timeout) enforces the deadline on the clock — the plain blocking waitFor() is not + // coroutine-cancellable, so a withTimeout around it can't actually interrupt a hung process. + val finished = withContext(Dispatchers.IO) { process.waitFor(timeoutMs, TimeUnit.MILLISECONDS) } + if (!finished) { + killTree(process) + stdoutDeferred.cancel() + stderrDeferred.cancel() + return@coroutineScope ToolResult.Failure( + invocationId = request.invocationId, + reason = "Process timed out after ${timeoutMs}ms", + recoverable = false, + ) + } + val exitCode = process.exitValue() val stdout = stdoutDeferred.await() val stderr = stderrDeferred.await()