address some nick comments in the review

Author: ericallam

packages/cli-v3/src/dev/devSupervisor.ts

@@ -104,31 +104,34 @@ class DevSupervisor implements WorkerRuntime {
     const enableProcessReuse =
       typeof this.options.config.experimental_processKeepAlive === "boolean"
         ? this.options.config.experimental_processKeepAlive
+        : typeof this.options.config.experimental_processKeepAlive === "object"
+        ? this.options.config.experimental_processKeepAlive.enabled
         : false;
 
+    const maxPoolSize =
+      typeof this.options.config.experimental_processKeepAlive === "object"
+        ? this.options.config.experimental_processKeepAlive.devMaxPoolSize ?? 25
+        : 25;
+
+    const maxExecutionsPerProcess =
+      typeof this.options.config.experimental_processKeepAlive === "object"
+        ? this.options.config.experimental_processKeepAlive.maxExecutionsPerProcess ?? 50
+        : 50;
+
     if (enableProcessReuse) {
       logger.debug("[DevSupervisor] Enabling process reuse", {
         enableProcessReuse,
+        maxPoolSize,
+        maxExecutionsPerProcess,
       });
     }
 
     this.taskRunProcessPool = new TaskRunProcessPool({
       env,
       cwd: this.options.config.workingDir,
-      enableProcessReuse:
-        typeof this.options.config.experimental_processKeepAlive === "boolean"
-          ? this.options.config.experimental_processKeepAlive
-          : typeof this.options.config.experimental_processKeepAlive === "object"
-          ? this.options.config.experimental_processKeepAlive.enabled
-          : false,
-      maxPoolSize:
-        typeof this.options.config.experimental_processKeepAlive === "object"
-          ? this.options.config.experimental_processKeepAlive.devMaxPoolSize ?? 25
-          : 25,
-      maxExecutionsPerProcess:
-        typeof this.options.config.experimental_processKeepAlive === "object"
-          ? this.options.config.experimental_processKeepAlive.maxExecutionsPerProcess ?? 50
-          : 50,
+      enableProcessReuse,
+      maxPoolSize,
+      maxExecutionsPerProcess,
     });
 
     this.socket = this.#createSocket();
@@ -355,6 +358,9 @@ class DevSupervisor implements WorkerRuntime {
           httpClient: this.options.client,
           logLevel: this.options.args.logLevel,
           taskRunProcessPool: this.taskRunProcessPool,
+          cwd: this.options.config.experimental_devProcessCwdInBuildDir
+            ? worker.build.outputPath
+            : undefined,
           onFinished: () => {
             logger.debug("[DevSupervisor] Run finished", { runId: message.run.friendlyId });
 

packages/cli-v3/src/entryPoints/managed/controller.ts

@@ -207,7 +207,15 @@ export class ManagedRunController {
           runId: runFriendlyId,
           message: "killing existing execution before starting new run",
         });
-        await this.currentExecution.kill().catch(() => {});
+
+        await this.currentExecution.shutdown().catch((error) => {
+          this.sendDebugLog({
+            runId: runFriendlyId,
+            message: "Error during execution shutdown",
+            properties: { error: error instanceof Error ? error.message : String(error) },
+          });
+        });
+
         this.currentExecution = null;
       }
 

packages/cli-v3/src/entryPoints/managed/execution.ts

@@ -120,10 +120,18 @@ export class RunExecution {
     }
 
     if (exitExecution) {
-      this.shutdown("kill");
+      this.shutdownExecution("kill");
     }
   }
 
+  public async shutdown() {
+    if (this.taskRunProcess) {
+      await this.taskRunProcessProvider.handleProcessAbort(this.taskRunProcess);
+    }
+
+    this.shutdownExecution("shutdown");
+  }
+
   /**
    * Prepares the execution with task run environment variables.
    * This should be called before executing, typically after a successful run to prepare for the next one.
@@ -232,7 +240,6 @@ export class RunExecution {
     if (this.currentAttemptNumber && this.currentAttemptNumber !== run.attemptNumber) {
       this.sendDebugLog("error: attempt number mismatch", snapshotMetadata);
       // This is a rogue execution, a new one will already have been created elsewhere
-      // TODO: keep this one, kill the process even if it's a keep-alive one
       await this.exitTaskRunProcessWithoutFailingRun({
         flush: false,
         reason: "attempt number mismatch",
@@ -250,7 +257,6 @@ export class RunExecution {
     if (deprecated) {
       this.sendDebugLog("run execution is deprecated", { incomingSnapshot: snapshot });
 
-      // TODO: keep this one, kill the process even if it's a keep-alive one
       await this.exitTaskRunProcessWithoutFailingRun({
         flush: false,
         reason: "deprecated execution",
@@ -469,7 +475,7 @@ export class RunExecution {
     if (startError) {
       this.sendDebugLog("failed to start attempt", { error: startError.message });
 
-      this.shutdown("failed to start attempt");
+      this.shutdownExecution("failed to start attempt");
       return;
     }
 
@@ -480,12 +486,12 @@ export class RunExecution {
     if (executeError) {
       this.sendDebugLog("failed to execute run", { error: executeError.message });
 
-      this.shutdown("failed to execute run");
+      this.shutdownExecution("failed to execute run");
       return;
     }
 
     // This is here for safety, but it
-    this.shutdown("execute call finished");
+    this.shutdownExecution("execute call finished");
   }
 
   private async executeRunWrapper({
@@ -786,7 +792,7 @@ export class RunExecution {
     if (startError) {
       this.sendDebugLog("failed to start attempt for retry", { error: startError.message });
 
-      this.shutdown("retryImmediately: failed to start attempt");
+      this.shutdownExecution("retryImmediately: failed to start attempt");
       return;
     }
 
@@ -797,7 +803,7 @@ export class RunExecution {
     if (executeError) {
       this.sendDebugLog("failed to execute run for retry", { error: executeError.message });
 
-      this.shutdown("retryImmediately: failed to execute run");
+      this.shutdownExecution("retryImmediately: failed to execute run");
       return;
     }
   }
@@ -841,7 +847,7 @@ export class RunExecution {
     await this.taskRunProcessProvider.suspendProcess(flush, this.taskRunProcess);
 
     // No services should be left running after this line - let's make sure of it
-    this.shutdown(`exitTaskRunProcessWithoutFailingRun: ${reason}`);
+    this.shutdownExecution(`exitTaskRunProcessWithoutFailingRun: ${reason}`);
   }
 
   /**
@@ -1012,10 +1018,10 @@ export class RunExecution {
     }
 
     this.executionAbortController.abort();
-    this.shutdown("abortExecution");
+    this.shutdownExecution("abortExecution");
   }
 
-  private shutdown(reason: string) {
+  private shutdownExecution(reason: string) {
     if (this.isShuttingDown) {
       this.sendDebugLog(`[shutdown] ${reason} (already shutting down)`, {
         firstShutdownReason: this.shutdownReason,

packages/cli-v3/src/entryPoints/managed/taskRunProcessProvider.ts

@@ -205,6 +205,19 @@ export class TaskRunProcessProvider {
     await process.cleanup(true);
   }
 
+  async killProcess(process: TaskRunProcess): Promise<void> {
+    this.sendDebugLog("Killing process");
+
+    // If this was our persistent process, clear it
+    if (this.persistentProcess?.pid === process.pid) {
+      this.persistentProcess = null;
+      this.executionCount = 0;
+    }
+
+    // Kill the process
+    await this.cleanupProcess(process);
+  }
+
   /**
    * Forces cleanup of any persistent process
    */
@@ -287,7 +300,7 @@ export class TaskRunProcessProvider {
   }
 
   private async cleanupProcess(taskRunProcess: TaskRunProcess): Promise<void> {
-    if (taskRunProcess) {
+    if (taskRunProcess && taskRunProcess.pid !== undefined) {
       this.sendDebugLog("Cleaning up TaskRunProcess", { pid: taskRunProcess.pid });
 
       await taskRunProcess.kill("SIGKILL").catch(() => {});