refactor(engine): consolidate plan step readiness helpers

Extract shared isDone and nextReadySteps into plan-step-readiness.ts so plan-blocked, plan-overall-status, and plan-ready cannot drift.

Co-authored-by: Cursor <cursoragent@cursor.com>

Author: claytonlin1110

packages/loopover-engine/src/plan-blocked.ts

@@ -1,13 +1,5 @@
-import type { PlanDag, PlanStep, PlanStepStatus } from "./plan-export.js";
-
-const isDone = (status: PlanStepStatus): boolean => status === "completed" || status === "skipped";
-
-function nextReadySteps(plan: PlanDag): PlanStep[] {
-  const statusById = new Map(plan.steps.map((step) => [step.id, step.status]));
-  return plan.steps.filter(
-    (step) => step.status === "pending" && step.dependsOn.every((dep) => isDone(statusById.get(dep) ?? "pending")),
-  );
-}
+import type { PlanDag } from "./plan-export.js";
+import { nextReadySteps } from "./plan-step-readiness.js";
 
 /**
  * Return whether the plan is deadlocked: pending steps remain but none are runnable. Mirrors the `blocked`

packages/loopover-engine/src/plan-overall-status.ts

@@ -1,16 +1,8 @@
-import type { PlanDag, PlanStep, PlanStepStatus } from "./plan-export.js";
+import type { PlanDag } from "./plan-export.js";
+import { nextReadySteps } from "./plan-step-readiness.js";
 
 export type PlanOverallStatus = "pending" | "running" | "completed" | "failed" | "blocked";
 
-const isDone = (status: PlanStepStatus): boolean => status === "completed" || status === "skipped";
-
-function nextReadySteps(plan: PlanDag): PlanStep[] {
-  const statusById = new Map(plan.steps.map((step) => [step.id, step.status]));
-  return plan.steps.filter(
-    (step) => step.status === "pending" && step.dependsOn.every((dep) => isDone(statusById.get(dep) ?? "pending")),
-  );
-}
-
 /**
  * Resolve the coarse plan status matching hosted `planProgress`'s `status` field. Pure — reads the plan DAG only.
  */

packages/loopover-engine/src/plan-ready.ts

@@ -1,13 +1,5 @@
-import type { PlanDag, PlanStep, PlanStepStatus } from "./plan-export.js";
-
-const isDone = (status: PlanStepStatus): boolean => status === "completed" || status === "skipped";
-
-function nextReadySteps(plan: PlanDag): PlanStep[] {
-  const statusById = new Map(plan.steps.map((step) => [step.id, step.status]));
-  return plan.steps.filter(
-    (step) => step.status === "pending" && step.dependsOn.every((dep) => isDone(statusById.get(dep) ?? "pending")),
-  );
-}
+import type { PlanDag } from "./plan-export.js";
+import { nextReadySteps } from "./plan-step-readiness.js";
 
 /**
  * Return whether any step is runnable now: `pending` with every dependency `completed` or `skipped`. Mirrors hosted

packages/loopover-engine/src/plan-step-readiness.ts

@@ -0,0 +1,12 @@
+import type { PlanDag, PlanStep, PlanStepStatus } from "./plan-export.js";
+
+export function isDone(status: PlanStepStatus): boolean {
+  return status === "completed" || status === "skipped";
+}
+
+export function nextReadySteps(plan: PlanDag): PlanStep[] {
+  const statusById = new Map(plan.steps.map((step) => [step.id, step.status]));
+  return plan.steps.filter(
+    (step) => step.status === "pending" && step.dependsOn.every((dep) => isDone(statusById.get(dep) ?? "pending")),
+  );
+}

test/unit/plan-step-readiness.test.ts

@@ -0,0 +1,57 @@
+import { describe, expect, it } from "vitest";
+
+import { isDone, nextReadySteps } from "../../packages/loopover-engine/src/plan-step-readiness";
+import type { PlanStep, PlanStepStatus } from "../../packages/loopover-engine/src/plan-export";
+
+function step(over: Partial<PlanStep> & { id: string; title: string }): PlanStep {
+  return {
+    actionClass: undefined,
+    dependsOn: [],
+    status: "pending",
+    attempts: 0,
+    maxAttempts: 3,
+    lastError: null,
+    ...over,
+  };
+}
+
+describe("isDone", () => {
+  it.each<PlanStepStatus>(["pending", "running", "failed"])("returns false for %s", (status) => {
+    expect(isDone(status)).toBe(false);
+  });
+
+  it.each<PlanStepStatus>(["completed", "skipped"])("returns true for %s", (status) => {
+    expect(isDone(status)).toBe(true);
+  });
+});
+
+describe("nextReadySteps", () => {
+  it("returns a pending step with no dependencies", () => {
+    const ready = step({ id: "a", title: "Build", status: "pending" });
+    expect(nextReadySteps({ steps: [ready] })).toEqual([ready]);
+  });
+
+  it("returns a pending step when its dependency is completed", () => {
+    const dep = step({ id: "a", title: "Build", status: "completed" });
+    const ready = step({ id: "b", title: "Test", status: "pending", dependsOn: ["a"] });
+    expect(nextReadySteps({ steps: [dep, ready] })).toEqual([ready]);
+  });
+
+  it("returns a pending step when its dependency is skipped", () => {
+    const dep = step({ id: "a", title: "Build", status: "skipped" });
+    const ready = step({ id: "b", title: "Test", status: "pending", dependsOn: ["a"] });
+    expect(nextReadySteps({ steps: [dep, ready] })).toEqual([ready]);
+  });
+
+  it.each<PlanStepStatus>(["running", "failed"])("returns no ready steps when a dependency is %s", (status) => {
+    const dep = step({ id: "a", title: "Build", status });
+    const blocked = step({ id: "b", title: "Test", status: "pending", dependsOn: ["a"] });
+    expect(nextReadySteps({ steps: [dep, blocked] })).toEqual([]);
+  });
+
+  it("returns no ready steps when a dependency is still pending", () => {
+    const dep = step({ id: "a", title: "Build", status: "pending", dependsOn: ["ghost"] });
+    const blocked = step({ id: "b", title: "Test", status: "pending", dependsOn: ["a"] });
+    expect(nextReadySteps({ steps: [dep, blocked] })).toEqual([]);
+  });
+});