diff --git a/openspec/changes/feat-completion-dependencies/proposal.md b/openspec/changes/archive/2026-04-10-feat-completion-dependencies/proposal.md similarity index 100% rename from openspec/changes/feat-completion-dependencies/proposal.md rename to openspec/changes/archive/2026-04-10-feat-completion-dependencies/proposal.md diff --git a/openspec/changes/feat-completion-dependencies/tasks.md b/openspec/changes/archive/2026-04-10-feat-completion-dependencies/tasks.md similarity index 100% rename from openspec/changes/feat-completion-dependencies/tasks.md rename to openspec/changes/archive/2026-04-10-feat-completion-dependencies/tasks.md diff --git a/src/TaskRunner.ts b/src/TaskRunner.ts index 63ba44e..892b3b8 100644 --- a/src/TaskRunner.ts +++ b/src/TaskRunner.ts @@ -143,7 +143,8 @@ export class TaskRunner { if (step.dependencies) { for (const dep of step.dependencies) { - const depId = getUniqueId(dep); + const depName = typeof dep === "string" ? dep : dep.step; + const depId = getUniqueId(depName); edgeLines.add(` ${depId} --> ${stepId}`); } } @@ -180,7 +181,10 @@ export class TaskRunner { const taskGraph: TaskGraph = { tasks: steps.map((step) => ({ id: step.name, - dependencies: step.dependencies ?? [], + dependencies: + step.dependencies?.map((dep) => + typeof dep === "string" ? dep : dep.step + ) ?? [], })), }; diff --git a/src/TaskStateManager.ts b/src/TaskStateManager.ts index 8e660e8..5ab16db 100644 --- a/src/TaskStateManager.ts +++ b/src/TaskStateManager.ts @@ -1,4 +1,4 @@ -import { TaskStep } from "./TaskStep.js"; +import { TaskRunCondition, TaskStep } from "./TaskStep.js"; import { TaskResult } from "./TaskResult.js"; import { EventBus } from "./EventBus.js"; @@ -12,7 +12,7 @@ export class TaskStateManager { private readonly running = new Set(); // Optimization structures - private readonly dependencyGraph = new Map[]>(); + private readonly dependencyGraph = new Map; condition: TaskRunCondition }[]>(); private readonly dependencyCounts = new Map(); private readyQueue: TaskStep[] = []; private readonly taskDefinitions = new Map>(); @@ -42,12 +42,15 @@ export class TaskStateManager { this.readyQueue.push(step); } else { for (const dep of deps) { - let dependents = this.dependencyGraph.get(dep); + const depName = typeof dep === "string" ? dep : dep.step; + const condition = typeof dep === "string" ? "success" : (dep.runCondition ?? "success"); + + let dependents = this.dependencyGraph.get(depName); if (dependents === undefined) { dependents = []; - this.dependencyGraph.set(dep, dependents); + this.dependencyGraph.set(depName, dependents); } - dependents.push(step); + dependents.push({ step, condition }); } } } @@ -182,15 +185,22 @@ export class TaskStateManager { if (!dependents) return; for (const dependent of dependents) { - const currentCount = this.dependencyCounts.get(dependent.name)!; - const newCount = currentCount - 1; - this.dependencyCounts.set(dependent.name, newCount); - - if (newCount === 0) { - // Task is ready. Ensure it's still pending. - if (this.pendingSteps.has(dependent)) { - this.readyQueue.push(dependent); - } + this.decrementDependencyCount(dependent.step); + } + } + + /** + * Decrements the dependency count for a task and queues it if ready. + */ + private decrementDependencyCount(dependent: TaskStep): void { + const currentCount = this.dependencyCounts.get(dependent.name)!; + const newCount = currentCount - 1; + this.dependencyCounts.set(dependent.name, newCount); + + if (newCount === 0) { + // Task is ready. Ensure it's still pending. + if (this.pendingSteps.has(dependent)) { + this.readyQueue.push(dependent); } } } @@ -216,13 +226,19 @@ export class TaskStateManager { const depError = currentResult?.error ? `: ${currentResult.error}` : ""; for (const dependent of dependents) { + // If the dependent runs 'always' and the parent task *actually failed* (not skipped), we can unblock it + if (dependent.condition === "always" && currentResult?.status === "failure") { + this.decrementDependencyCount(dependent.step); + continue; + } + const result: TaskResult = { status: "skipped", message: `Skipped because dependency '${currentName}' failed${depError}`, }; - if (this.internalMarkSkipped(dependent, result)) { - queue.push(dependent.name); + if (this.internalMarkSkipped(dependent.step, result)) { + queue.push(dependent.step.name); } } } diff --git a/src/TaskStep.ts b/src/TaskStep.ts index 10fed0c..a30d6e6 100644 --- a/src/TaskStep.ts +++ b/src/TaskStep.ts @@ -2,6 +2,13 @@ import { TaskResult } from "./TaskResult.js"; import { TaskRetryConfig } from "./contracts/TaskRetryConfig.js"; import { TaskLoopConfig } from "./contracts/TaskLoopConfig.js"; +export type TaskRunCondition = "success" | "always"; + +export interface TaskDependencyConfig { + step: string; + runCondition?: TaskRunCondition; +} + /** * Represents a single, executable step within a workflow. * @template TContext The shape of the shared context object. @@ -10,7 +17,7 @@ export interface TaskStep { /** A unique identifier for this task. */ name: string; /** An optional list of task names that must complete successfully before this step can run. */ - dependencies?: string[]; + dependencies?: (string | TaskDependencyConfig)[]; /** Optional retry configuration for the task. */ retry?: TaskRetryConfig; /** Optional loop configuration for the task. */ diff --git a/tests/TaskRunner.test.ts b/tests/TaskRunner.test.ts index 24f765f..b10a971 100644 --- a/tests/TaskRunner.test.ts +++ b/tests/TaskRunner.test.ts @@ -267,4 +267,33 @@ describe("TaskRunner", () => { const results = await runner.execute(steps); expect(results.get("A")?.message).toBe("Executed by custom strategy"); }); + + describe("teardown scenarios", () => { + it("should execute a cleanup task even if the preceding task fails, using 'always' runCondition", async () => { + const runner = new TaskRunner>({}); + + const setup: TaskStep> = { + name: "Setup", + run: async () => ({ status: "success" }), + }; + + const work: TaskStep> = { + name: "Work", + dependencies: ["Setup"], + run: async () => ({ status: "failure", error: "Something broke" }), + }; + + const teardown: TaskStep> = { + name: "Teardown", + dependencies: [{ step: "Work", runCondition: "always" }], + run: async () => ({ status: "success" }), + }; + + const results = await runner.execute([setup, work, teardown]); + + expect(results.get("Setup")?.status).toBe("success"); + expect(results.get("Work")?.status).toBe("failure"); + expect(results.get("Teardown")?.status).toBe("success"); // Teardown ran despite Work failing! + }); + }); }); diff --git a/tests/TaskRunnerMermaid.test.ts b/tests/TaskRunnerMermaid.test.ts index eff9f7a..0f8dbd1 100644 --- a/tests/TaskRunnerMermaid.test.ts +++ b/tests/TaskRunnerMermaid.test.ts @@ -17,6 +17,20 @@ describe("TaskRunner Mermaid Graph", () => { expect(lines).toContain(" B[\"B\"]"); }); + it("should generate a graph for a sequence of tasks with mixed dependencies", () => { + const steps: TaskStep[] = [ + { name: "A", run: async () => ({ status: "success" }) }, + { name: "B", dependencies: [{ step: "A", runCondition: "always" }], run: async () => ({ status: "success" }) }, + { name: "C", dependencies: ["B"], run: async () => ({ status: "success" }) }, + ]; + const graph = TaskRunner.getMermaidGraph(steps); + expect(graph).toContain("A[\"A\"]"); + expect(graph).toContain("B[\"B\"]"); + expect(graph).toContain("C[\"C\"]"); + expect(graph).toContain("A --> B"); + expect(graph).toContain("B --> C"); + }); + it("should generate a graph with dependencies", () => { const steps: TaskStep[] = [ { name: "A", run: async () => ({ status: "success" }) }, diff --git a/tests/TaskStateManager_coverage.test.ts b/tests/TaskStateManager_coverage.test.ts index 7ed542a..20283c0 100644 --- a/tests/TaskStateManager_coverage.test.ts +++ b/tests/TaskStateManager_coverage.test.ts @@ -66,4 +66,68 @@ describe("TaskStateManager Coverage", () => { expect(stateManager.getResults().get("B")?.status).toBe("cancelled"); }); + it("should push 'always' dependent to readyQueue when dependency fails", () => { + const eventBus = new EventBus(); + const stateManager = new TaskStateManager(eventBus); + + const stepA: TaskStep = { name: "A", run: async () => ({ status: "failure" }) }; + const stepB: TaskStep = { name: "B", dependencies: [{ step: "A", runCondition: "always" }], run: async () => ({ status: "success" }) }; + + stateManager.initialize([stepA, stepB]); + + const readyA = stateManager.processDependencies(); + expect(readyA).toHaveLength(1); + expect(readyA[0].name).toBe("A"); + + stateManager.markRunning(readyA[0]); + stateManager.markCompleted(readyA[0], { status: "failure", error: "failed A" }); + + // B should be queued because of 'always' condition despite A failing + const readyB = stateManager.processDependencies(); + expect(readyB).toHaveLength(1); + expect(readyB[0].name).toBe("B"); + }); + + it("should skip 'always' dependent if dependency is skipped", () => { + const eventBus = new EventBus(); + const stateManager = new TaskStateManager(eventBus); + + const stepX: TaskStep = { name: "X", run: async () => ({ status: "failure" }) }; + const stepA: TaskStep = { name: "A", dependencies: ["X"], run: async () => ({ status: "success" }) }; + const stepB: TaskStep = { name: "B", dependencies: [{ step: "A", runCondition: "always" }], run: async () => ({ status: "success" }) }; + + stateManager.initialize([stepX, stepA, stepB]); + + const readyX = stateManager.processDependencies(); + expect(readyX).toHaveLength(1); + + stateManager.markRunning(readyX[0]); + stateManager.markCompleted(readyX[0], { status: "failure" }); // X fails -> A skips + + // processDependencies should be empty because A is skipped, and B is skipped too + const readyB = stateManager.processDependencies(); + expect(readyB).toHaveLength(0); + + expect(stateManager.getResults().get("A")?.status).toBe("skipped"); + expect(stateManager.getResults().get("B")?.status).toBe("skipped"); + }); + + it("should handle default runCondition when TaskDependencyConfig omits it", () => { + const eventBus = new EventBus(); + const stateManager = new TaskStateManager(eventBus); + + const stepA: TaskStep = { name: "A", run: async () => ({ status: "failure" }) }; + const stepB: TaskStep = { name: "B", dependencies: [{ step: "A" }], run: async () => ({ status: "success" }) }; + + stateManager.initialize([stepA, stepB]); + + const readyA = stateManager.processDependencies(); + stateManager.markRunning(readyA[0]); + stateManager.markCompleted(readyA[0], { status: "failure" }); + + // B should be skipped because it defaults to 'success' + const readyB = stateManager.processDependencies(); + expect(readyB).toHaveLength(0); + expect(stateManager.getResults().get("B")?.status).toBe("skipped"); + }); });