Skip to content

Commit a9a7f25

Browse files
committed
feat(engine): collapse the duplicated plan-DAG state machine into @loopover/engine
Completes #9537's last requirement, and fixes the build failure the first push of PR #9565 hit. THE BUILD FIX: `ToolContract` was imported from @loopover/contract/tools, which only imports that type internally and never re-exports it. The local typecheck resolves the package through a src alias and passed; the real package build resolves the export map and failed at 'Build MCP'. Now imported from the package root, which does export it. THE PLAN-DAG DEDUP: buildPlanDag/validatePlanDag/the step state machine existed in src/services/plan-dag.ts AND hand-copied, untyped, into the stdio MCP bin -- whose own comment explained why: it resolves @loopover/engine through the published package, whose export map did not surface them. It does now. Moving it turned up a THIRD partial copy already in the engine: plan-export.ts holds the step/plan types, plan-step-readiness.ts holds nextReadySteps/isDone, and plan-overall-status.ts holds the status vocabulary, each decomposed into its own module. The moved file imports all three rather than restating them -- a fourth PlanDag would have defeated the point -- and contributes only the mutating half (build, validate, advance) that had no engine home. src/services/plan-dag.ts stays as the Worker's import path, now a re-export, so nothing else had to move. Release ordering, which is why this looked blocked: both publish workflows are workflow_dispatch-only, so nothing auto-publishes on merge and an operator publishes the engine before the CLI, as usual. The engine takes a minor bump for the new export, with packages/loopover-miner/expected-engine.version and both consumers' dependency ranges moved to ^3.16.0 in the same commit so a published CLI can never resolve an engine without it. Also declares @loopover/contract#build as an edge of the root typecheck task. That was working only because ci.yml happens to run a contract build first; the edge makes a bare 'turbo run typecheck' correct on its own.
1 parent c86edc2 commit a9a7f25

9 files changed

Lines changed: 169 additions & 253 deletions

File tree

package-lock.json

Lines changed: 3 additions & 19 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/loopover-engine/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@loopover/engine",
3-
"version": "3.15.3",
3+
"version": "3.16.0",
44
"license": "AGPL-3.0-only",
55
"type": "module",
66
"description": "Shared deterministic engine logic for the LoopOver review stack and loopover-miner.",

packages/loopover-engine/src/index.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -927,3 +927,17 @@ export { parsePullRequestTargetKey } from "./parse-pull-request-target-key.js";
927927
// loopover-mcp production local-branch collector so both enforce the same forbidden-source-upload-key /
928928
// oversized-content, metadata-only contract on the real collection path.
929929
export { assertScenarioLocalBranchInputSafe } from "./scenario-input-safety.js";
930+
931+
// #783 multi-step plan DAG (#9537) -- the per-step state machine loopover_build_plan /
932+
// loopover_plan_status / loopover_record_step_result advance. Lives here because BOTH MCP servers
933+
// need it and the stdio one resolves this package from the registry; before #9537 it existed twice,
934+
// once in src/services/plan-dag.ts and once hand-copied and untyped into the stdio bin.
935+
export {
936+
buildPlanDag,
937+
validatePlanDag,
938+
markStepRunning,
939+
applyStepResult,
940+
planProgress,
941+
type PlanProgress,
942+
} from "./plan-dag.js";
943+
export { isDone as isPlanStepDone, nextReadySteps } from "./plan-step-readiness.js";
Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
// #783 multi-step action DAG. A miner plan is a set of steps with dependencies ("close 1 stale PR → land 2 →
2+
// open a new direct PR"); loopover tracks per-step state + retries so the plan survives across MCP tool
3+
// calls and resumes where it left off. PURE + deterministic — the harness performs each step's real work and
4+
// reports the result back; this module only advances the state machine.
5+
6+
// #9537: the step/plan TYPES and the readiness + overall-status functions already lived in this
7+
// package, decomposed one-per-module. This file adds only the mutating half of the state machine
8+
// (build, validate, advance) that had no engine home and was therefore duplicated in the Worker and
9+
// hand-copied into the stdio MCP bin. It imports the existing pieces rather than restating them --
10+
// a fourth copy of `PlanDag` would defeat the point of the move.
11+
import type { PlanDag, PlanStep, PlanStepStatus } from "./plan-export.js";
12+
import { isDone, nextReadySteps } from "./plan-step-readiness.js";
13+
import type { PlanOverallStatus } from "./plan-overall-status.js";
14+
15+
export type PlanProgress = {
16+
total: number;
17+
completed: number;
18+
failed: number;
19+
running: number;
20+
pending: number;
21+
skipped: number;
22+
status: PlanOverallStatus;
23+
};
24+
25+
const DEFAULT_MAX_ATTEMPTS = 1;
26+
27+
/** Build a normalized DAG from raw step input: default status pending / attempts 0, clamp maxAttempts to [1,10],
28+
* drop self-deps + duplicate dep ids. Pure. */
29+
export function buildPlanDag(steps: Array<{ id: string; title: string; actionClass?: string | undefined; dependsOn?: string[] | undefined; maxAttempts?: number | undefined }>): PlanDag {
30+
return {
31+
steps: steps.map((step) => ({
32+
id: step.id,
33+
title: step.title,
34+
...(step.actionClass !== undefined ? { actionClass: step.actionClass } : {}),
35+
dependsOn: [...new Set((step.dependsOn ?? []).filter((dep) => dep !== step.id))],
36+
status: "pending" as PlanStepStatus,
37+
attempts: 0,
38+
maxAttempts: Math.min(10, Math.max(1, Math.trunc(step.maxAttempts ?? DEFAULT_MAX_ATTEMPTS))),
39+
})),
40+
};
41+
}
42+
43+
/** Validate the DAG: unique ids, every dependency exists, and no cycles. Pure. */
44+
export function validatePlanDag(plan: PlanDag): { valid: boolean; errors: string[] } {
45+
const errors: string[] = [];
46+
const ids = plan.steps.map((step) => step.id);
47+
const idSet = new Set(ids);
48+
if (idSet.size !== ids.length) errors.push("duplicate step ids");
49+
for (const step of plan.steps) {
50+
for (const dep of step.dependsOn) {
51+
if (!idSet.has(dep)) errors.push(`step ${step.id} depends on unknown step ${dep}`);
52+
}
53+
}
54+
// Cycle detection via DFS coloring.
55+
const color = new Map<string, 0 | 1 | 2>();
56+
const byId = new Map(plan.steps.map((step) => [step.id, step]));
57+
const hasCycle = (id: string): boolean => {
58+
color.set(id, 1);
59+
/* v8 ignore next -- hasCycle is only ever called with an id present in byId, so the [] fallback is defensive. */
60+
for (const dep of byId.get(id)?.dependsOn ?? []) {
61+
const depColor = color.get(dep) ?? 0;
62+
if (depColor === 1) return true;
63+
if (depColor === 0 && byId.has(dep) && hasCycle(dep)) return true;
64+
}
65+
color.set(id, 2);
66+
return false;
67+
};
68+
for (const step of plan.steps) {
69+
if ((color.get(step.id) ?? 0) === 0 && hasCycle(step.id)) {
70+
errors.push("plan has a dependency cycle");
71+
break;
72+
}
73+
}
74+
return { valid: errors.length === 0, errors };
75+
}
76+
77+
function mapStep(plan: PlanDag, stepId: string, update: (step: PlanStep) => PlanStep): PlanDag {
78+
return { steps: plan.steps.map((step) => (step.id === stepId ? update(step) : step)) };
79+
}
80+
81+
/** Mark a ready step as running (the harness has started it). No-op for an unknown/non-pending step. Pure. */
82+
export function markStepRunning(plan: PlanDag, stepId: string): PlanDag {
83+
return mapStep(plan, stepId, (step) => (step.status === "pending" ? { ...step, status: "running" } : step));
84+
}
85+
86+
/**
87+
* Record the outcome of a step the harness ran. `completed` / `skipped` are terminal. `failed` increments the
88+
* attempt count and retries (back to pending) until maxAttempts is exhausted, after which it stays failed. An
89+
* unknown step id is a no-op. Pure.
90+
*/
91+
export function applyStepResult(plan: PlanDag, stepId: string, result: { outcome: "completed" | "failed" | "skipped"; error?: string | null | undefined }): PlanDag {
92+
return mapStep(plan, stepId, (step) => {
93+
if (isDone(step.status) || step.status === "failed") return step;
94+
if (result.outcome === "completed") return { ...step, status: "completed", lastError: null };
95+
if (result.outcome === "skipped") return { ...step, status: "skipped", lastError: null };
96+
const attempts = step.attempts + 1;
97+
const exhausted = attempts >= step.maxAttempts;
98+
return { ...step, attempts, status: exhausted ? "failed" : "pending", lastError: result.error ?? "step failed" };
99+
});
100+
}
101+
102+
/** Aggregate progress + the overall plan status. Pure. */
103+
export function planProgress(plan: PlanDag): PlanProgress {
104+
const count = (status: PlanStepStatus) => plan.steps.filter((step) => step.status === status).length;
105+
const completed = count("completed");
106+
const skipped = count("skipped");
107+
const failed = count("failed");
108+
const running = count("running");
109+
const pending = count("pending");
110+
const total = plan.steps.length;
111+
let status: PlanOverallStatus;
112+
if (total > 0 && completed + skipped === total) status = "completed";
113+
else if (failed > 0) status = "failed";
114+
else if (running > 0) status = "running";
115+
else if (pending > 0 && nextReadySteps(plan).length === 0) status = "blocked";
116+
else status = "pending";
117+
return { total, completed, failed, running, pending, skipped, status };
118+
}

packages/loopover-mcp/bin/loopover-mcp.ts

Lines changed: 8 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import { fileURLToPath } from "node:url";
99
import { McpServer, ResourceTemplate } from "@modelcontextprotocol/sdk/server/mcp.js";
1010
import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js";
1111
import { buildFeasibilityVerdict, buildPrTextLint, buildGateDispositions, buildPublicPrBodyDraft } from "@loopover/engine";
12+
// #9537: the plan-DAG state machine, formerly hand-copied into this file, untyped.
13+
import { applyStepResult, buildPlanDag, nextReadySteps, planProgress, validatePlanDag, type PlanDag } from "@loopover/engine";
1214
// #6149: the miner write-tools are PURE local-execution spec builders (loopover never performs the write);
1315
// registering them locally is just importing the same engine builders the remote server uses.
1416
import {
@@ -143,8 +145,8 @@ import {
143145
WatchIssuesInput,
144146
getToolContract,
145147
ListPendingActionsStdioInput,
146-
type ToolContract,
147148
} from "@loopover/contract/tools";
149+
import type { ToolContract } from "@loopover/contract";
148150
import { buildBranchAnalysisPayload, collectLocalDiff, collectLocalBranchMetadata, probeLocalScorer, referenceScorePreviewExample, resolveScorePreviewCommand, resolveWorkspaceCwd, sanitizeLocalScorerStatus, setupGuidanceForLocalScorer, isTestFile } from "../lib/local-branch.js";
149151
import { formatTable } from "../lib/format-table.js";
150152
import { argsWantJson, describeCliError, reportCliFailure } from "../lib/cli-error.js";
@@ -274,105 +276,15 @@ const MAINTAIN_AUTONOMY_LEVELS = ["observe", "auto_with_approval", "auto"];
274276
// route + MCP tool accept, while set-level keeps its own autonomy-configurable subset above.
275277
const PROPOSE_ACTION_CLASSES = ["review", "request_changes", "approve", "merge", "close", "label", "review_state_label"];
276278

277-
// #6150 — plan-DAG step tracking for loopover_build_plan/loopover_plan_status/loopover_record_step_result.
278-
// Hand-duplicated from src/services/plan-dag.ts (packages/loopover-engine/src/services/plan-dag.ts is NOT
279-
// where it lives -- this module was never extracted to @loopover/engine, so there is nothing to import from
280-
// the published package's export map), same rationale as MAINTAIN_ACTION_CLASSES/AUTONOMY_LEVELS above: this
281-
// file resolves @loopover/engine through the published package, whose export map does not surface it.
282-
// PURE + stateless (no DB, no repo/network access) -- the harness performs each step's real work and calls
283-
// loopover_record_step_result to report it back; this only advances the in-memory state machine the caller
284-
// passes in and gets back on every call.
285-
const DEFAULT_PLAN_MAX_ATTEMPTS = 1;
286-
287-
function buildPlanDag(steps: any) {
288-
return {
289-
steps: steps.map((step: any) => ({
290-
id: step.id,
291-
title: step.title,
292-
...(step.actionClass !== undefined ? { actionClass: step.actionClass } : {}),
293-
dependsOn: [...new Set((step.dependsOn ?? []).filter((dep: any) => dep !== step.id))],
294-
status: "pending",
295-
attempts: 0,
296-
maxAttempts: Math.min(10, Math.max(1, Math.trunc(step.maxAttempts ?? DEFAULT_PLAN_MAX_ATTEMPTS))),
297-
})),
298-
};
299-
}
300-
301-
function validatePlanDag(plan: any) {
302-
const errors = [];
303-
const ids = plan.steps.map((step: any) => step.id);
304-
const idSet = new Set(ids);
305-
if (idSet.size !== ids.length) errors.push("duplicate step ids");
306-
for (const step of plan.steps) {
307-
for (const dep of step.dependsOn) {
308-
if (!idSet.has(dep)) errors.push(`step ${step.id} depends on unknown step ${dep}`);
309-
}
310-
}
311-
const color = new Map();
312-
const byId = new Map<any, any>(plan.steps.map((step: any) => [step.id, step]));
313-
const hasCycle = (id: any) => {
314-
color.set(id, 1);
315-
for (const dep of byId.get(id)?.dependsOn ?? []) {
316-
const depColor = color.get(dep) ?? 0;
317-
if (depColor === 1) return true;
318-
if (depColor === 0 && byId.has(dep) && hasCycle(dep)) return true;
319-
}
320-
color.set(id, 2);
321-
return false;
322-
};
323-
for (const step of plan.steps) {
324-
if ((color.get(step.id) ?? 0) === 0 && hasCycle(step.id)) {
325-
errors.push("plan has a dependency cycle");
326-
break;
327-
}
328-
}
329-
return { valid: errors.length === 0, errors };
330-
}
331-
332-
const isPlanStepDone = (status: any) => status === "completed" || status === "skipped";
333-
334-
function nextReadySteps(plan: any) {
335-
const statusById = new Map(plan.steps.map((step: any) => [step.id, step.status]));
336-
return plan.steps.filter((step: any) => step.status === "pending" && step.dependsOn.every((dep: any) => isPlanStepDone(statusById.get(dep) ?? "pending")));
337-
}
338-
339-
function mapPlanStep(plan: any, stepId: any, update: any) {
340-
return { steps: plan.steps.map((step: any) => (step.id === stepId ? update(step) : step)) };
341-
}
342-
343-
function applyStepResult(plan: any, stepId: any, result: any) {
344-
return mapPlanStep(plan, stepId, (step: any) => {
345-
if (isPlanStepDone(step.status) || step.status === "failed") return step;
346-
if (result.outcome === "completed") return { ...step, status: "completed", lastError: null };
347-
if (result.outcome === "skipped") return { ...step, status: "skipped", lastError: null };
348-
const attempts = step.attempts + 1;
349-
const exhausted = attempts >= step.maxAttempts;
350-
return { ...step, attempts, status: exhausted ? "failed" : "pending", lastError: result.error ?? "step failed" };
351-
});
352-
}
279+
// #783 plan DAG -- one implementation, in @loopover/engine (#9537). This file carried a
280+
// hand-copied, untyped duplicate of buildPlanDag/validatePlanDag/nextReadySteps/the step state
281+
// machine because the engine's export map did not surface them; it does now, so the copy is gone.
353282

354-
function planProgress(plan: any) {
355-
const count = (status: any) => plan.steps.filter((step: any) => step.status === status).length;
356-
const completed = count("completed");
357-
const skipped = count("skipped");
358-
const failed = count("failed");
359-
const running = count("running");
360-
const pending = count("pending");
361-
const total = plan.steps.length;
362-
let status;
363-
if (total > 0 && completed + skipped === total) status = "completed";
364-
else if (failed > 0) status = "failed";
365-
else if (running > 0) status = "running";
366-
else if (pending > 0 && nextReadySteps(plan).length === 0) status = "blocked";
367-
else status = "pending";
368-
return { total, completed, failed, running, pending, skipped, status };
369-
}
370-
371-
function planView(plan: any) {
283+
function planView(plan: PlanDag) {
372284
return {
373285
plan,
374286
progress: planProgress(plan),
375-
readySteps: nextReadySteps(plan).map((step: any) => ({ id: step.id, title: step.title })),
287+
readySteps: nextReadySteps(plan).map((step) => ({ id: step.id, title: step.title })),
376288
validation: validatePlanDag(plan),
377289
};
378290
}

packages/loopover-mcp/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@
4646
},
4747
"dependencies": {
4848
"@loopover/contract": "^0.1.0",
49-
"@loopover/engine": "^3.15.2",
49+
"@loopover/engine": "^3.16.0",
5050
"@modelcontextprotocol/sdk": "1.29.0",
5151
"posthog-node": "^5.46.1",
5252
"zod": "^4.4.3"
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
3.15.3
1+
3.16.0

packages/loopover-miner/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@
4848
},
4949
"dependencies": {
5050
"@loopover/contract": "^0.1.0",
51-
"@loopover/engine": "^3.15.2",
51+
"@loopover/engine": "^3.16.0",
5252
"@modelcontextprotocol/sdk": "1.29.0",
5353
"@sentry/node": "^10.67.0",
5454
"zod": "^4.4.3"

0 commit comments

Comments
 (0)