diff --git a/src/node/services/taskService.ts b/src/node/services/taskService.ts index 7e43d21fde..350bbc1bcf 100644 --- a/src/node/services/taskService.ts +++ b/src/node/services/taskService.ts @@ -10408,6 +10408,109 @@ export class TaskService { 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( + this.config.getSessionDir(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/workspaceService.test.ts b/src/node/services/workspaceService.test.ts index ec69f044d2..5473898bd1 100644 --- a/src/node/services/workspaceService.test.ts +++ b/src/node/services/workspaceService.test.ts @@ -12360,7 +12360,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: { @@ -12378,22 +12378,25 @@ describe("WorkspaceService remove lifecycle coordination", () => { } } ); - const hasDescendantAgentTasks = mock(() => { + const hasActiveDescendantAgentTasksForWorkspace = mock(() => { expect(insideLifecycleLock).toBe(true); return true; }); workspaceService.setTaskService({ withTaskTreeLifecycleLock, - hasDescendantAgentTasks, + hasActiveDescendantAgentTasksForWorkspace, + cascadeRemoveInactiveDescendantsWhileTaskTreeLocked: mock(() => + Promise.resolve(Ok(undefined)) + ), } as unknown as TaskService); 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 6393454098..5e97f7ce84 100644 --- a/src/node/services/workspaceService.ts +++ b/src/node/services/workspaceService.ts @@ -605,7 +605,7 @@ type WorkspaceRuntimeStatus = "running" | "stopped" | "unknown" | "unsupported"; const POST_COMPACTION_METADATA_REFRESH_DEBOUNCE_MS = 100; const DESCENDANT_WORKSPACE_REMOVE_ERROR = - "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."; export interface ArchiveWorkspaceOptions { /** * Refuse to archive when the effective worktree archive behavior would delete the checkout @@ -6154,10 +6154,24 @@ export class WorkspaceService extends EventEmitter { // Try to remove from runtime (filesystem) try { - if (this.taskService?.hasDescendantAgentTasks?.(workspaceId) === true) { + // Block deletion when active descendants are still running — the user must + // stop them first. Inactive (reported/interrupted) descendants are cascade-removed + // deepest-first so persistent sub-agents don't permanently block parent deletion. + if (this.taskService?.hasActiveDescendantAgentTasksForWorkspace?.(workspaceId) === true) { return Err(DESCENDANT_WORKSPACE_REMOVE_ERROR); } + // Cascade-remove inactive descendants through TaskService, which handles + // git-patch-artifact waits, ownership tombstones, and force-flag passthrough. + const cascadeResult = + await this.taskService?.cascadeRemoveInactiveDescendantsWhileTaskTreeLocked?.( + workspaceId, + force + ); + if (cascadeResult != null && !cascadeResult.success) { + return Err(cascadeResult.error); + } + // Stop any active stream before deleting metadata/config to avoid tool calls racing with removal. // // IMPORTANT: AIService forwards "stream-abort" asynchronously after partial cleanup. If we roll up