diff --git a/src/node/services/taskService.ts b/src/node/services/taskService.ts index b4d5ab7706..4f294afe01 100644 --- a/src/node/services/taskService.ts +++ b/src/node/services/taskService.ts @@ -8797,6 +8797,109 @@ export class TaskService implements AgentTaskIntegration { return this.listDescendantAgentTaskIdsFromIndex(index, workspaceId).length > 0; } + /** + * List all descendant agent task IDs sorted deepest-first so callers can + * cascade-remove children before their parents without tripping the orphan guard. + */ + listDescendantAgentTaskIdsDeepestFirst(workspaceId: string): string[] { + assert( + workspaceId.length > 0, + "listDescendantAgentTaskIdsDeepestFirst: workspaceId must be non-empty" + ); + + const cfg = this.config.loadConfigOrDefault(); + const index = this.buildAgentTaskIndex(cfg); + const ids = this.listDescendantAgentTaskIdsFromIndex(index, workspaceId); + + // Sort by depth (deepest first) so leaf children are removed before their parents. + // Ties are broken by insertion order (stable sort). + // Cap the parent-chain walk at ids.length to avoid hanging on corrupted + // parentWorkspaceId cycles (depth can never exceed the descendant count). + const maxDepth = ids.length; + const depthById = new Map(); + for (const id of ids) { + let depth = 0; + let current: string | undefined = id; + while (current != null && current !== workspaceId && depth < maxDepth) { + depth++; + current = index.parentById.get(current); + } + depthById.set(id, depth); + } + + return ids.sort((a, b) => (depthById.get(b) ?? 0) - (depthById.get(a) ?? 0)); + } + + /** + * Cascade-remove all inactive descendant agent tasks deepest-first. + * Must be called while the task-tree lifecycle lock is already held (the caller + * in WorkspaceService.remove() acquires it). Includes all safeguards from + * removeInactiveDescendantAgentTask: active/streaming checks, git-patch-artifact + * wait, tombstone persistence, and force-flag passthrough. + */ + async cascadeRemoveInactiveDescendantsWhileTaskTreeLocked( + workspaceId: string, + force: boolean + ): Promise> { + const descendantIds = this.listDescendantAgentTaskIdsDeepestFirst(workspaceId); + + for (const descendantId of descendantIds) { + const config = this.config.loadConfigOrDefault(); + const entry = findWorkspaceEntry(config, descendantId); + if (entry == null) continue; // already removed + + // Safety: active tasks should have been caught by the guard in removeUnlocked, + // but double-check to avoid removing a task that became active in the meantime. + if ( + this.isActiveAgentTaskEntry({ ...entry.workspace, projectPath: entry.projectPath }) || + this.aiService.isStreaming(descendantId) + ) { + return Err( + `Descendant workspace ${descendantId} is still active. Stop it before removing.` + ); + } + + const result = await this.gitPatchArtifactService.withOperationLock( + descendantId, + async () => { + // Wait for any in-flight format-patch job before removal so the artifact + // isn't lost. Refuse removal if a durable pending marker remains (needs the + // child worktree to recover). + await this.gitPatchArtifactService.waitForGeneration(descendantId); + const parentWsId = entry.workspace.parentWorkspaceId; + if (parentWsId) { + const patchArtifact = await readSubagentGitPatchArtifact( + path.join(this.config.sessionsDir, parentWsId), + descendantId + ); + if (patchArtifact?.status === "pending") { + return Err( + `Cannot cascade-remove descendant ${descendantId}: git patch artifact is still pending.` + ); + } + } + + const tombstoneResult = await this.persistRemovedAgentTaskTombstones(descendantId); + if (!tombstoneResult.success) { + return Err( + `Failed to persist tombstones for descendant ${descendantId}: ${tombstoneResult.error}` + ); + } + + return await this.workspaceService.removeWhileTaskTreeLocked(descendantId, force); + } + ); + + if (!result.success) { + return Err( + `Failed to cascade-remove descendant workspace ${descendantId}: ${result.error}` + ); + } + } + + return Ok(undefined); + } + hasActiveDescendantAgentTasksForWorkspace(workspaceId: string): boolean { assert( workspaceId.length > 0, diff --git a/src/node/services/taskWorkspaceSeam.testUtils.ts b/src/node/services/taskWorkspaceSeam.testUtils.ts index 5b81af0977..8be5cd231b 100644 --- a/src/node/services/taskWorkspaceSeam.testUtils.ts +++ b/src/node/services/taskWorkspaceSeam.testUtils.ts @@ -65,6 +65,7 @@ export function makeAgentTaskIntegrationFake( operation(), hasDescendantAgentTasks: () => false, hasActiveDescendantAgentTasksForWorkspace: () => false, + cascadeRemoveInactiveDescendantsWhileTaskTreeLocked: () => Promise.resolve(Ok(undefined)), hasActiveTopLevelWorkflowRunsForWorkspace: () => Promise.resolve(false), getAgentTaskStatus: () => undefined, resetAutoResumeCount: () => undefined, diff --git a/src/node/services/taskWorkspaceSeam.ts b/src/node/services/taskWorkspaceSeam.ts index ecd8810187..31064fde4c 100644 --- a/src/node/services/taskWorkspaceSeam.ts +++ b/src/node/services/taskWorkspaceSeam.ts @@ -514,6 +514,10 @@ export interface AgentTaskIntegration { withTaskTreeLifecycleLock(workspaceId: string, operation: () => Promise): Promise; hasDescendantAgentTasks(workspaceId: string): boolean; hasActiveDescendantAgentTasksForWorkspace(workspaceId: string): boolean; + cascadeRemoveInactiveDescendantsWhileTaskTreeLocked( + workspaceId: string, + force: boolean + ): Promise>; hasActiveTopLevelWorkflowRunsForWorkspace(workspaceId: string): Promise; getAgentTaskStatus(workspaceId: string): AgentTaskStatus | null | undefined; resetAutoResumeCount(workspaceId: string): void; diff --git a/src/node/services/workspaceService.test.ts b/src/node/services/workspaceService.test.ts index 66bc71107a..073c5a3103 100644 --- a/src/node/services/workspaceService.test.ts +++ b/src/node/services/workspaceService.test.ts @@ -12347,7 +12347,7 @@ describe("WorkspaceService assertPricedModelForBudgetedGoal", () => { }); describe("WorkspaceService remove lifecycle coordination", () => { - test("checks descendant tasks while holding the task-tree lifecycle lock", async () => { + test("blocks removal when active descendant tasks exist", async () => { const workspaceId = "parent-remove-lifecycle"; const workspaceService = createWorkspaceServiceForTest({ config: { @@ -12370,24 +12370,27 @@ describe("WorkspaceService remove lifecycle coordination", () => { insideLifecycleLock = false; } }; - const hasDescendantAgentTasks = mock(() => { + const hasActiveDescendantAgentTasksForWorkspace = mock(() => { expect(insideLifecycleLock).toBe(true); return true; }); workspaceService.setAgentTaskIntegration( makeAgentTaskIntegrationFake({ withTaskTreeLifecycleLock: runWithTaskTreeLifecycleLock, - hasDescendantAgentTasks, + hasActiveDescendantAgentTasksForWorkspace, + cascadeRemoveInactiveDescendantsWhileTaskTreeLocked: mock(() => + Promise.resolve(Ok(undefined)) + ), }) ); expect(await workspaceService.remove(workspaceId, true)).toEqual( Err( - "This workspace has descendant sub-agent workspaces. Remove those descendants deepest-first before removing their parent." + "This workspace has active descendant sub-agent workspaces. Stop them before removing their parent." ) ); expect(withTaskTreeLifecycleLock).toHaveBeenCalledWith(workspaceId, expect.any(Function)); - expect(hasDescendantAgentTasks).toHaveBeenCalledWith(workspaceId); + expect(hasActiveDescendantAgentTasksForWorkspace).toHaveBeenCalledWith(workspaceId); }); }); diff --git a/src/node/services/workspaceService.ts b/src/node/services/workspaceService.ts index 3e24599c35..f43b7d1d05 100644 --- a/src/node/services/workspaceService.ts +++ b/src/node/services/workspaceService.ts @@ -642,7 +642,7 @@ type WorkspaceDevToolsCleanup = Pick