🤖 fix: bound task_terminate with abort support, timeouts, and per-task errors

src/constants/terminationTimeouts.ts

@@ -0,0 +1,4 @@
+export const TASK_TERMINATION_TOOL_TIMEOUT_MS = 5 * 60 * 1000;
+export const TASK_TERMINATION_STOP_STREAM_TIMEOUT_MS = 20 * 1000;
+export const TASK_TERMINATION_WORKSPACE_REMOVE_TIMEOUT_MS = 2 * 60 * 1000;
+export const WORKTREE_DELETE_GIT_TIMEOUT_MS = 60 * 1000;

src/node/git.ts

@@ -2,7 +2,7 @@ import * as fs from "fs";
 import * as path from "path";
 import type { Config } from "@/node/config";
 import type { RuntimeConfig } from "@/common/types/runtime";
-import { execFileAsync } from "@/node/utils/disposableExec";
+import { execFileAsync, type ExecFileAsyncOptions } from "@/node/utils/disposableExec";
 import { createRuntime } from "./runtime/runtimeFactory";
 import { log } from "./services/log";
 import { getErrorMessage } from "@/common/utils/errors";
@@ -47,14 +47,15 @@ export interface CreateWorktreeOptions {
   runtimeConfig?: RuntimeConfig;
 }
 
-export async function listLocalBranches(projectPath: string): Promise<string[]> {
-  using proc = execFileAsync("git", [
-    "-C",
-    projectPath,
-    "for-each-ref",
-    "--format=%(refname:short)",
-    "refs/heads",
-  ]);
+export async function listLocalBranches(
+  projectPath: string,
+  options?: ExecFileAsyncOptions
+): Promise<string[]> {
+  using proc = execFileAsync(
+    "git",
+    ["-C", projectPath, "for-each-ref", "--format=%(refname:short)", "refs/heads"],
+    options
+  );
   const { stdout } = await proc.result;
   return stdout
     .split("\n")
@@ -63,9 +64,16 @@ export async function listLocalBranches(projectPath: string): Promise<string[]>
     .sort((a, b) => a.localeCompare(b));
 }
 
-export async function getCurrentBranch(projectPath: string): Promise<string | null> {
+export async function getCurrentBranch(
+  projectPath: string,
+  options?: ExecFileAsyncOptions
+): Promise<string | null> {
   try {
-    using proc = execFileAsync("git", ["-C", projectPath, "rev-parse", "--abbrev-ref", "HEAD"]);
+    using proc = execFileAsync(
+      "git",
+      ["-C", projectPath, "rev-parse", "--abbrev-ref", "HEAD"],
+      options
+    );
     const { stdout } = await proc.result;
     const branch = stdout.trim();
     if (!branch || branch === "HEAD") {

src/node/services/taskService.test.ts

@@ -4,6 +4,10 @@ import * as path from "path";
 import * as os from "os";
 import { execSync } from "node:child_process";
 
+import {
+  TASK_TERMINATION_STOP_STREAM_TIMEOUT_MS,
+  TASK_TERMINATION_WORKSPACE_REMOVE_TIMEOUT_MS,
+} from "@/constants/terminationTimeouts";
 import {
   Config,
   type ProjectConfig,
@@ -9197,6 +9201,180 @@ describe("TaskService", () => {
     expect(remove).toHaveBeenNthCalledWith(2, parentTaskId, true);
   });
 
+  test("terminateDescendantAgentTask skips a timed-out stream and continues other descendants", async () => {
+    const config = await createTestConfig(rootDir);
+    const projectPath = path.join(rootDir, "repo");
+    const rootWorkspaceId = "root-111";
+    const parentTaskId = "task-parent";
+    const stuckTaskId = "task-stuck";
+    const siblingTaskId = "task-sibling";
+
+    await saveWorkspaces(
+      config,
+      projectPath,
+      [
+        projectWorkspace(projectPath, "root", rootWorkspaceId),
+        projectWorkspace(projectPath, "parent-task", parentTaskId, {
+          parentWorkspaceId: rootWorkspaceId,
+          agentType: "exec",
+          taskStatus: "running",
+        }),
+        projectWorkspace(projectPath, "stuck-task", stuckTaskId, {
+          parentWorkspaceId: parentTaskId,
+          agentType: "explore",
+          taskStatus: "running",
+        }),
+        projectWorkspace(projectPath, "sibling-task", siblingTaskId, {
+          parentWorkspaceId: parentTaskId,
+          agentType: "explore",
+          taskStatus: "running",
+        }),
+      ],
+      testTaskSettings()
+    );
+
+    const stopStream = mock((workspaceId: string): Promise<Result<void>> => {
+      return workspaceId === stuckTaskId
+        ? new Promise(() => undefined)
+        : Promise.resolve(Ok(undefined));
+    });
+    const { aiService } = createAIServiceMocks(config, { stopStream });
+    const { workspaceService, remove } = createWorkspaceServiceMocks();
+    const { taskService } = createTaskServiceHarness(config, { aiService, workspaceService });
+    const originalSetTimeout = globalThis.setTimeout;
+    const timeoutSpy = spyOn(globalThis, "setTimeout").mockImplementation(((
+      handler: () => void,
+      timeout?: number
+    ) => {
+      if (timeout === TASK_TERMINATION_STOP_STREAM_TIMEOUT_MS) {
+        // Fire on a 0ms macrotask, not a microtask: already-resolved stop
+        // promises settle through several microtask hops first, so only the
+        // genuinely stuck stream can lose the race to this timer.
+        return originalSetTimeout(handler, 0);
+      }
+      return originalSetTimeout(handler, timeout);
+    }) as typeof setTimeout);
+
+    const terminateResult = await taskService.terminateDescendantAgentTask(
+      rootWorkspaceId,
+      parentTaskId
+    );
+    timeoutSpy.mockRestore();
+
+    expect(terminateResult.success).toBe(false);
+    if (terminateResult.success) return;
+    expect(terminateResult.error).toContain(`Timed out stopping task stream (${stuckTaskId})`);
+    expect(terminateResult.error).toContain(
+      `Skipped removing task workspace (${parentTaskId}): a descendant task workspace was not removed`
+    );
+    expect(remove).not.toHaveBeenCalledWith(stuckTaskId, true);
+    expect(remove).toHaveBeenCalledWith(siblingTaskId, true);
+    // The stuck child survives, so its ancestor must survive too: a live child
+    // must never point at removed parent metadata.
+    expect(remove).not.toHaveBeenCalledWith(parentTaskId, true);
+  });
+
+  test("terminate retry awaits the original in-flight removal instead of trusting a dedup Ok", async () => {
+    const config = await createTestConfig(rootDir);
+    const projectPath = path.join(rootDir, "repo");
+    const rootWorkspaceId = "root-111";
+    const parentTaskId = "task-parent";
+    const childTaskId = "task-child";
+
+    await saveWorkspaces(
+      config,
+      projectPath,
+      [
+        projectWorkspace(projectPath, "root", rootWorkspaceId),
+        projectWorkspace(projectPath, "parent-task", parentTaskId, {
+          parentWorkspaceId: rootWorkspaceId,
+          agentType: "exec",
+          taskStatus: "running",
+        }),
+        projectWorkspace(projectPath, "child-task", childTaskId, {
+          parentWorkspaceId: parentTaskId,
+          agentType: "explore",
+          taskStatus: "running",
+        }),
+      ],
+      testTaskSettings()
+    );
+
+    // First removal of the child hangs; later calls return Ok, simulating
+    // WorkspaceService.remove()'s short-circuit for IDs already being removed.
+    let childRemoveCalls = 0;
+    const remove = mock((workspaceId: string, _force?: boolean): Promise<Result<void>> => {
+      if (workspaceId === childTaskId) {
+        childRemoveCalls += 1;
+        if (childRemoveCalls === 1) {
+          return new Promise(() => undefined);
+        }
+      }
+      return Promise.resolve(Ok(undefined));
+    });
+    const { workspaceService } = createWorkspaceServiceMocks({ remove });
+    const { taskService } = createTaskServiceHarness(config, { workspaceService });
+    const originalSetTimeout = globalThis.setTimeout;
+    const timeoutSpy = spyOn(globalThis, "setTimeout").mockImplementation(((
+      handler: () => void,
+      timeout?: number
+    ) => {
+      if (timeout === TASK_TERMINATION_WORKSPACE_REMOVE_TIMEOUT_MS) {
+        // 0ms macrotask so pending microtask chains settle first (see above).
+        return originalSetTimeout(handler, 0);
+      }
+      return originalSetTimeout(handler, timeout);
+    }) as typeof setTimeout);
+
+    const firstAttempt = await taskService.terminateDescendantAgentTask(
+      rootWorkspaceId,
+      parentTaskId
+    );
+    const retryAttempt = await taskService.terminateDescendantAgentTask(
+      rootWorkspaceId,
+      parentTaskId
+    );
+    timeoutSpy.mockRestore();
+
+    expect(firstAttempt.success).toBe(false);
+    expect(retryAttempt.success).toBe(false);
+    if (retryAttempt.success) return;
+    expect(retryAttempt.error).toContain(`Timed out removing task workspace (${childTaskId})`);
+    // The retry must not re-call remove for the child (a fresh call would
+    // return the dedup Ok) and must keep the parent blocked.
+    expect(childRemoveCalls).toBe(1);
+    expect(remove).not.toHaveBeenCalledWith(parentTaskId, true);
+  });
+
+  test("descendant traversal terminates when task metadata contains a cycle", () => {
+    const config = new Config(rootDir);
+    const { taskService } = createTaskServiceHarness(config);
+    const traversal = taskService as unknown as {
+      listDescendantAgentTaskIdsFromIndex: (
+        index: {
+          byId: Map<string, unknown>;
+          childrenByParent: Map<string, string[]>;
+          parentById: Map<string, string>;
+        },
+        workspaceId: string
+      ) => string[];
+    };
+    const index = {
+      byId: new Map<string, unknown>(),
+      childrenByParent: new Map([
+        ["root", ["child"]],
+        ["child", ["grandchild"]],
+        ["grandchild", ["child", "root"]],
+      ]),
+      parentById: new Map<string, string>(),
+    };
+
+    expect(traversal.listDescendantAgentTaskIdsFromIndex(index, "root")).toEqual([
+      "child",
+      "grandchild",
+    ]);
+  });
+
   test("terminateAllDescendantAgentTasks interrupts entire subtree leaf-first", async () => {
     const config = await createTestConfig(rootDir);