From c1d0f6d15d8449f7dda637f0ea7a2a337c4191f0 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 17 Jun 2026 17:39:08 -0700 Subject: [PATCH] feat(agent): multi-step plan DAG tools (#783) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 2: track a miner's multi-step plan ('close 1 stale PR → land 2 → open a new direct PR') with per-step state, retries, and resume. STATELESS — the harness holds the plan and passes it back each call, so gittensory keeps no record of the miner's plan (boundary-aligned) and resume is just re-sending it. - src/services/plan-dag.ts: pure DAG state machine — buildPlanDag (normalize + clamp + drop self/dup deps), validatePlanDag (dup ids, missing deps, cycles via DFS coloring), nextReadySteps (deps satisfied), markStepRunning, applyStepResult (completed/skipped terminal; failed retries until maxAttempts then fails), planProgress (counts + overall pending/running/completed/failed/ blocked). - src/mcp/server.ts: three stateless tools — gittensory_build_plan, gittensory_plan_status, gittensory_record_step_result. Tests: the full state machine + the MCP round-trip driving a 2-step plan to completion + retry-to-failure. New code 100% covered; full suite green (2117). --- src/mcp/server.ts | 86 +++++++++++++++++++++ src/services/plan-dag.ts | 133 +++++++++++++++++++++++++++++++++ test/unit/mcp-plan-dag.test.ts | 71 ++++++++++++++++++ test/unit/plan-dag.test.ts | 85 +++++++++++++++++++++ 4 files changed, 375 insertions(+) create mode 100644 src/services/plan-dag.ts create mode 100644 test/unit/mcp-plan-dag.test.ts create mode 100644 test/unit/plan-dag.test.ts diff --git a/src/mcp/server.ts b/src/mcp/server.ts index 7cf718bfc7..dd78ac427c 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -97,6 +97,7 @@ import { buildPostEligibilityCommentSpec, type LocalWriteActionSpec, } from "./local-write-tools"; +import { applyStepResult, buildPlanDag, nextReadySteps, planProgress, validatePlanDag, type PlanDag } from "../services/plan-dag"; import { loadRepoFocusManifest } from "../signals/focus-manifest-loader"; import { buildPredictedGateVerdict } from "../rules/predicted-gate"; import { buildIssueSlopAssessment, buildSlopAssessment, ISSUE_SLOP_RUBRIC_MARKDOWN, SLOP_RUBRIC_MARKDOWN } from "../signals/slop"; @@ -275,6 +276,48 @@ const localWriteActionOutputSchema = { boundary: z.string(), }; +// #783 plan DAG — STATELESS: the harness holds the plan and passes it back each call; these tools only advance +// the state machine, so gittensory keeps no record of the miner's plan. +const planStepStatusEnum = z.enum(["pending", "running", "completed", "failed", "skipped"]); +const rawPlanStepSchema = z + .object({ + id: z.string().min(1).max(100), + title: z.string().min(1).max(300), + actionClass: z.string().min(1).max(60).optional(), + dependsOn: z.array(z.string().min(1).max(100)).max(50).optional(), + maxAttempts: z.number().int().min(1).max(10).optional(), + }) + .strict(); +const planStepSchema = z + .object({ + id: z.string().min(1).max(100), + title: z.string().min(1).max(300), + actionClass: z.string().min(1).max(60).optional(), + dependsOn: z.array(z.string().min(1).max(100)).max(50), + status: planStepStatusEnum, + attempts: z.number().int().min(0), + maxAttempts: z.number().int().min(1).max(10), + lastError: z.string().max(2000).nullable().optional(), + }) + .strict(); +const planDagSchema = z.object({ steps: z.array(planStepSchema).max(100) }).strict(); +const buildPlanShape = { steps: z.array(rawPlanStepSchema).min(1).max(100) }; +const planStatusShape = { plan: planDagSchema }; +const recordStepResultShape = { + plan: planDagSchema, + stepId: z.string().min(1).max(100), + outcome: z.enum(["completed", "failed", "skipped"]), + error: z.string().max(2000).optional(), +}; +const planViewOutputSchema = { + plan: planDagSchema.optional(), + progress: z + .object({ total: z.number(), completed: z.number(), failed: z.number(), running: z.number(), pending: z.number(), skipped: z.number(), status: z.string() }) + .optional(), + readySteps: z.array(z.object({ id: z.string(), title: z.string() })).optional(), + validation: z.object({ valid: z.boolean(), errors: z.array(z.string()) }).optional(), +}; + const localBranchAnalysisShape = { login: z.string().min(1).max(SCENARIO_MAX_BRANCH_REF_CHARS), repoFullName: z.string().min(3).max(SCENARIO_MAX_REPO_FULL_NAME_CHARS), @@ -1106,6 +1149,23 @@ export class GittensoryMcp { async (input) => this.toolResult(this.localWriteSpec(buildDeleteBranchSpec(input))), ); + // #783 multi-step plan DAG — stateless: pass the plan back each call. + server.registerTool( + "gittensory_build_plan", + { description: "Normalize raw steps into a validated multi-step plan DAG (per-step state + retries). Returns the plan to hold and pass back to the other plan tools.", inputSchema: buildPlanShape, outputSchema: planViewOutputSchema }, + async (input) => this.toolResult(this.buildPlan(input)), + ); + server.registerTool( + "gittensory_plan_status", + { description: "Return a plan's progress, validation, and the steps ready to run now (all dependencies met).", inputSchema: planStatusShape, outputSchema: planViewOutputSchema }, + async (input) => this.toolResult(this.planStatusTool(input)), + ); + server.registerTool( + "gittensory_record_step_result", + { description: "Record a step's outcome (completed / failed / skipped). A failure retries until maxAttempts is exhausted. Returns the advanced plan + the next ready steps.", inputSchema: recordStepResultShape, outputSchema: planViewOutputSchema }, + async (input) => this.toolResult(this.recordStepResult(input)), + ); + server.registerTool( "gittensory_explain_score_breakdown", { @@ -1867,6 +1927,32 @@ export class GittensoryMcp { return { summary: `${spec.action}: ${spec.description} ${spec.boundary}`, data: spec as unknown as Record }; } + // #783 plan DAG — pure, stateless transforms over the caller's plan. + private planView(plan: PlanDag): Record { + return { + plan: plan as unknown as Record, + progress: planProgress(plan), + readySteps: nextReadySteps(plan).map((step) => ({ id: step.id, title: step.title })), + validation: validatePlanDag(plan), + }; + } + + private buildPlan(input: z.infer>): ToolPayload { + const plan = buildPlanDag(input.steps); + const validation = validatePlanDag(plan); + return { summary: `Built a ${plan.steps.length}-step plan (${validation.valid ? "valid DAG" : `INVALID: ${validation.errors.join("; ")}`}).`, data: this.planView(plan) }; + } + + private planStatusTool(input: z.infer>): ToolPayload { + const plan = input.plan as PlanDag; + return { summary: `Plan status: ${planProgress(plan).status}.`, data: this.planView(plan) }; + } + + private recordStepResult(input: z.infer>): ToolPayload { + const plan = applyStepResult(input.plan as PlanDag, input.stepId, { outcome: input.outcome, ...(input.error !== undefined ? { error: input.error } : {}) }); + return { summary: `Recorded ${input.outcome} for step ${input.stepId}; plan is now ${planProgress(plan).status}.`, data: this.planView(plan) }; + } + private async explainScoreBreakdown(input: z.infer>): Promise { if (!input.contributorLogin) throw new Error("contributorLogin is required for score breakdown."); this.requireContributorAccess(input.contributorLogin); diff --git a/src/services/plan-dag.ts b/src/services/plan-dag.ts new file mode 100644 index 0000000000..a474bc7c92 --- /dev/null +++ b/src/services/plan-dag.ts @@ -0,0 +1,133 @@ +// #783 multi-step action DAG. A miner plan is a set of steps with dependencies ("close 1 stale PR → land 2 → +// open a new direct PR"); gittensory tracks per-step state + retries so the plan survives across MCP tool +// calls and resumes where it left off. PURE + deterministic — the harness performs each step's real work and +// reports the result back; this module only advances the state machine. + +export type PlanStepStatus = "pending" | "running" | "completed" | "failed" | "skipped"; + +export type PlanStep = { + id: string; + title: string; + actionClass?: string | undefined; + dependsOn: string[]; + status: PlanStepStatus; + attempts: number; + maxAttempts: number; + lastError?: string | null | undefined; +}; + +export type PlanDag = { steps: PlanStep[] }; + +export type PlanOverallStatus = "pending" | "running" | "completed" | "failed" | "blocked"; + +export type PlanProgress = { + total: number; + completed: number; + failed: number; + running: number; + pending: number; + skipped: number; + status: PlanOverallStatus; +}; + +const DEFAULT_MAX_ATTEMPTS = 1; + +/** Build a normalized DAG from raw step input: default status pending / attempts 0, clamp maxAttempts to [1,10], + * drop self-deps + duplicate dep ids. Pure. */ +export function buildPlanDag(steps: Array<{ id: string; title: string; actionClass?: string | undefined; dependsOn?: string[] | undefined; maxAttempts?: number | undefined }>): PlanDag { + return { + steps: steps.map((step) => ({ + id: step.id, + title: step.title, + ...(step.actionClass !== undefined ? { actionClass: step.actionClass } : {}), + dependsOn: [...new Set((step.dependsOn ?? []).filter((dep) => dep !== step.id))], + status: "pending" as PlanStepStatus, + attempts: 0, + maxAttempts: Math.min(10, Math.max(1, Math.trunc(step.maxAttempts ?? DEFAULT_MAX_ATTEMPTS))), + })), + }; +} + +/** Validate the DAG: unique ids, every dependency exists, and no cycles. Pure. */ +export function validatePlanDag(plan: PlanDag): { valid: boolean; errors: string[] } { + const errors: string[] = []; + const ids = plan.steps.map((step) => step.id); + const idSet = new Set(ids); + if (idSet.size !== ids.length) errors.push("duplicate step ids"); + for (const step of plan.steps) { + for (const dep of step.dependsOn) { + if (!idSet.has(dep)) errors.push(`step ${step.id} depends on unknown step ${dep}`); + } + } + // Cycle detection via DFS coloring. + const color = new Map(); + const byId = new Map(plan.steps.map((step) => [step.id, step])); + const hasCycle = (id: string): boolean => { + color.set(id, 1); + /* v8 ignore next -- hasCycle is only ever called with an id present in byId, so the [] fallback is defensive. */ + for (const dep of byId.get(id)?.dependsOn ?? []) { + const depColor = color.get(dep) ?? 0; + if (depColor === 1) return true; + if (depColor === 0 && byId.has(dep) && hasCycle(dep)) return true; + } + color.set(id, 2); + return false; + }; + for (const step of plan.steps) { + if ((color.get(step.id) ?? 0) === 0 && hasCycle(step.id)) { + errors.push("plan has a dependency cycle"); + break; + } + } + return { valid: errors.length === 0, errors }; +} + +const isDone = (status: PlanStepStatus): boolean => status === "completed" || status === "skipped"; + +/** The steps ready to run now: pending, with every dependency completed or skipped. Pure. */ +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"))); +} + +function mapStep(plan: PlanDag, stepId: string, update: (step: PlanStep) => PlanStep): PlanDag { + return { steps: plan.steps.map((step) => (step.id === stepId ? update(step) : step)) }; +} + +/** Mark a ready step as running (the harness has started it). No-op for an unknown/non-pending step. Pure. */ +export function markStepRunning(plan: PlanDag, stepId: string): PlanDag { + return mapStep(plan, stepId, (step) => (step.status === "pending" ? { ...step, status: "running" } : step)); +} + +/** + * Record the outcome of a step the harness ran. `completed` / `skipped` are terminal. `failed` increments the + * attempt count and retries (back to pending) until maxAttempts is exhausted, after which it stays failed. An + * unknown step id is a no-op. Pure. + */ +export function applyStepResult(plan: PlanDag, stepId: string, result: { outcome: "completed" | "failed" | "skipped"; error?: string | null | undefined }): PlanDag { + return mapStep(plan, stepId, (step) => { + if (result.outcome === "completed") return { ...step, status: "completed", lastError: null }; + if (result.outcome === "skipped") return { ...step, status: "skipped", lastError: null }; + const attempts = step.attempts + 1; + const exhausted = attempts >= step.maxAttempts; + return { ...step, attempts, status: exhausted ? "failed" : "pending", lastError: result.error ?? "step failed" }; + }); +} + +/** Aggregate progress + the overall plan status. Pure. */ +export function planProgress(plan: PlanDag): PlanProgress { + const count = (status: PlanStepStatus) => plan.steps.filter((step) => step.status === status).length; + const completed = count("completed"); + const skipped = count("skipped"); + const failed = count("failed"); + const running = count("running"); + const pending = count("pending"); + const total = plan.steps.length; + let status: PlanOverallStatus; + if (total > 0 && completed + skipped === total) status = "completed"; + else if (failed > 0) status = "failed"; + else if (running > 0) status = "running"; + else if (pending > 0 && nextReadySteps(plan).length === 0) status = "blocked"; + else status = "pending"; + return { total, completed, failed, running, pending, skipped, status }; +} diff --git a/test/unit/mcp-plan-dag.test.ts b/test/unit/mcp-plan-dag.test.ts new file mode 100644 index 0000000000..b6fd5adb2b --- /dev/null +++ b/test/unit/mcp-plan-dag.test.ts @@ -0,0 +1,71 @@ +import { Client } from "@modelcontextprotocol/sdk/client/index.js"; +import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js"; +import { describe, expect, it } from "vitest"; +import { GittensoryMcp } from "../../src/mcp/server"; +import { createTestEnv } from "../helpers/d1"; + +async function connect() { + const server = new GittensoryMcp(createTestEnv()).createServer(); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + await server.connect(serverTransport); + const client = new Client({ name: "gittensory-plan-test", version: "0.1.0" }, { capabilities: {} }); + await client.connect(clientTransport); + return client; +} + +type PlanView = { + plan: { steps: Array<{ id: string; status: string; attempts: number }> }; + progress: { status: string; total: number; completed: number }; + readySteps: Array<{ id: string }>; + validation: { valid: boolean; errors: string[] }; +}; + +describe("MCP plan DAG tools (#783)", () => { + it("build_plan → record_step_result drives a multi-step plan to completion (stateless, plan passed back)", async () => { + const client = await connect(); + const built = await client.callTool({ + name: "gittensory_build_plan", + arguments: { + steps: [ + { id: "a", title: "close stale PR" }, + { id: "b", title: "land PR 2", dependsOn: ["a"] }, + ], + }, + }); + expect(built.isError).toBeFalsy(); + let view = built.structuredContent as PlanView; + expect(view.validation).toEqual({ valid: true, errors: [] }); + expect(view.progress.status).toBe("pending"); + expect(view.readySteps.map((s) => s.id)).toEqual(["a"]); // only the root is ready + + const afterA = await client.callTool({ name: "gittensory_record_step_result", arguments: { plan: view.plan, stepId: "a", outcome: "completed" } }); + view = afterA.structuredContent as PlanView; + expect(view.readySteps.map((s) => s.id)).toEqual(["b"]); // b unblocked + expect(view.progress).toMatchObject({ status: "pending", completed: 1 }); // b ready but not yet running + + const afterB = await client.callTool({ name: "gittensory_record_step_result", arguments: { plan: view.plan, stepId: "b", outcome: "completed" } }); + view = afterB.structuredContent as PlanView; + expect(view.progress).toMatchObject({ status: "completed", completed: 2, total: 2 }); + }); + + it("plan_status surfaces validation errors for a bad DAG without throwing", async () => { + const client = await connect(); + const result = await client.callTool({ + name: "gittensory_plan_status", + arguments: { plan: { steps: [{ id: "a", title: "A", dependsOn: ["ghost"], status: "pending", attempts: 0, maxAttempts: 1 }] } }, + }); + const view = result.structuredContent as PlanView; + expect(view.validation.valid).toBe(false); + expect(view.validation.errors.join(" ")).toMatch(/unknown step ghost/); + }); + + it("record_step_result retries a failed step until maxAttempts", async () => { + const client = await connect(); + const plan = { steps: [{ id: "a", title: "A", dependsOn: [], status: "pending", attempts: 0, maxAttempts: 2 }] }; + const first = (await client.callTool({ name: "gittensory_record_step_result", arguments: { plan, stepId: "a", outcome: "failed", error: "boom" } })).structuredContent as PlanView; + expect(first.plan.steps[0]).toMatchObject({ status: "pending", attempts: 1 }); // retry + const second = (await client.callTool({ name: "gittensory_record_step_result", arguments: { plan: first.plan, stepId: "a", outcome: "failed" } })).structuredContent as PlanView; + expect(second.plan.steps[0]).toMatchObject({ status: "failed", attempts: 2 }); + expect(second.progress.status).toBe("failed"); + }); +}); diff --git a/test/unit/plan-dag.test.ts b/test/unit/plan-dag.test.ts new file mode 100644 index 0000000000..981f13a5e6 --- /dev/null +++ b/test/unit/plan-dag.test.ts @@ -0,0 +1,85 @@ +import { describe, expect, it } from "vitest"; +import { applyStepResult, buildPlanDag, markStepRunning, nextReadySteps, planProgress, validatePlanDag, type PlanDag } from "../../src/services/plan-dag"; + +const chain = () => + buildPlanDag([ + { id: "a", title: "close stale PR" }, + { id: "b", title: "land PR 2", dependsOn: ["a"] }, + { id: "c", title: "open direct PR", dependsOn: ["b"] }, + ]); + +describe("plan DAG (#783)", () => { + it("buildPlanDag normalizes defaults, clamps maxAttempts, and drops self/duplicate deps", () => { + const plan = buildPlanDag([{ id: "a", title: "A", actionClass: "close", dependsOn: ["a", "a"], maxAttempts: 99 }]); + expect(plan.steps[0]).toMatchObject({ status: "pending", attempts: 0, maxAttempts: 10, dependsOn: [], actionClass: "close" }); + expect(buildPlanDag([{ id: "x", title: "X", maxAttempts: 0 }]).steps[0]?.maxAttempts).toBe(1); + }); + + it("validatePlanDag flags duplicate ids, missing deps, and cycles", () => { + expect(validatePlanDag(buildPlanDag([{ id: "a", title: "A" }, { id: "a", title: "A2" }])).errors).toContain("duplicate step ids"); + expect(validatePlanDag(buildPlanDag([{ id: "a", title: "A", dependsOn: ["ghost"] }])).valid).toBe(false); + const cyclic: PlanDag = { + steps: [ + { id: "a", title: "A", dependsOn: ["b"], status: "pending", attempts: 0, maxAttempts: 1 }, + { id: "b", title: "B", dependsOn: ["a"], status: "pending", attempts: 0, maxAttempts: 1 }, + ], + }; + expect(validatePlanDag(cyclic).errors).toContain("plan has a dependency cycle"); + expect(validatePlanDag(chain())).toEqual({ valid: true, errors: [] }); + }); + + it("nextReadySteps returns only steps whose dependencies are all done", () => { + const plan = chain(); + expect(nextReadySteps(plan).map((s) => s.id)).toEqual(["a"]); + const afterA = applyStepResult(plan, "a", { outcome: "completed" }); + expect(nextReadySteps(afterA).map((s) => s.id)).toEqual(["b"]); + }); + + it("applyStepResult: completed/skipped are terminal; failed retries then fails terminally", () => { + let plan = buildPlanDag([{ id: "a", title: "A", maxAttempts: 2 }]); + plan = applyStepResult(plan, "a", { outcome: "failed", error: "boom" }); + expect(plan.steps[0]).toMatchObject({ status: "pending", attempts: 1, lastError: "boom" }); + plan = applyStepResult(plan, "a", { outcome: "failed" }); + expect(plan.steps[0]).toMatchObject({ status: "failed", attempts: 2, lastError: "step failed" }); + expect(applyStepResult(buildPlanDag([{ id: "x", title: "X" }]), "x", { outcome: "skipped" }).steps[0]?.status).toBe("skipped"); + // unknown id → no-op + expect(applyStepResult(buildPlanDag([{ id: "x", title: "X" }]), "nope", { outcome: "completed" }).steps[0]?.status).toBe("pending"); + }); + + it("markStepRunning marks a pending step running and is a no-op otherwise", () => { + const plan = buildPlanDag([{ id: "a", title: "A" }]); + expect(markStepRunning(plan, "a").steps[0]?.status).toBe("running"); + const completed = applyStepResult(plan, "a", { outcome: "completed" }); + expect(markStepRunning(completed, "a").steps[0]?.status).toBe("completed"); + }); + + it("planProgress tracks the lifecycle: pending → running → completed", () => { + let plan = chain(); + expect(planProgress(plan).status).toBe("pending"); + plan = markStepRunning(plan, "a"); + expect(planProgress(plan).status).toBe("running"); + plan = applyStepResult(plan, "a", { outcome: "completed" }); + plan = applyStepResult(plan, "b", { outcome: "completed" }); + plan = applyStepResult(plan, "c", { outcome: "skipped" }); + expect(planProgress(plan)).toMatchObject({ status: "completed", completed: 2, skipped: 1, total: 3 }); + }); + + it("planProgress reports failed when a step exhausts its retries", () => { + const plan = applyStepResult(chain(), "a", { outcome: "failed" }); // maxAttempts 1 → terminal + expect(planProgress(plan)).toMatchObject({ status: "failed", failed: 1 }); + }); + + it("planProgress reports blocked for a deadlocked (cyclic) plan with no ready steps", () => { + const deadlocked: PlanDag = { + steps: [ + { id: "a", title: "A", dependsOn: ["b"], status: "pending", attempts: 0, maxAttempts: 1 }, + { id: "b", title: "B", dependsOn: ["a"], status: "pending", attempts: 0, maxAttempts: 1 }, + ], + }; + expect(planProgress(deadlocked).status).toBe("blocked"); + }); + + it("planProgress on an empty plan is pending", () => { + expect(planProgress({ steps: [] }).status).toBe("pending"); + }); +});