From 3f313567dd800688cfde066e007321dac30176bb Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 5 Jun 2026 22:42:36 -0700 Subject: [PATCH] feat(pr): built-in PR workflow template, end-to-end (U9) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ships BUILTIN_PR_WORKFLOW_IR (builtin:pr-workflow) wiring the lifecycle end to end: start -> pr-create -> await-review hold -> (changes-requested) pr-respond -[rework]-> await-review -> (approved) auto-merge gate -> pr-merge -> end, with conflict/failed holds and manual force-merge/close edges. Completes U6's validator side: validateV2 permits a top-level kind:rework edge whose loop-head opts in via config.reworkRegion (non- rework cycles still rejected). Additive (default builtin:coding untouched); behind workflowGraphExecutor. Legacy PrCommentHandler/PrMonitor retirement deferred (they remain the flag-off path's PR handling) — recorded in the plan. Fast e2e test; 41 core IR/builtin tests green. Co-Authored-By: Claude Opus 4.8 (1M context) --- ...feat-unified-pr-entity-review-loop-plan.md | 1 + .../src/__tests__/builtin-workflows.test.ts | 43 ++- .../core/src/__tests__/workflow-ir.test.ts | 33 ++ packages/core/src/builtin-pr-workflow-ir.ts | 173 +++++++++ packages/core/src/builtin-workflows.ts | 35 ++ packages/core/src/index.ts | 1 + packages/core/src/workflow-ir.ts | 24 +- .../src/__tests__/pr-workflow-e2e.test.ts | 365 ++++++++++++++++++ 8 files changed, 665 insertions(+), 10 deletions(-) create mode 100644 packages/core/src/builtin-pr-workflow-ir.ts create mode 100644 packages/engine/src/__tests__/pr-workflow-e2e.test.ts diff --git a/docs/plans/2026-06-05-001-feat-unified-pr-entity-review-loop-plan.md b/docs/plans/2026-06-05-001-feat-unified-pr-entity-review-loop-plan.md index 093b38a6e5..b65246ec5e 100644 --- a/docs/plans/2026-06-05-001-feat-unified-pr-entity-review-loop-plan.md +++ b/docs/plans/2026-06-05-001-feat-unified-pr-entity-review-loop-plan.md @@ -321,6 +321,7 @@ Diagrams render authoritative content alongside the prose; on disagreement the p - Webhook ingestion as an alternative to polling (the reconcile's external-event firing is webhook-ready; a public endpoint is future). - PR node kinds contributed by plugins (today node kinds are built-in only; a plugin node-kind registry is a separate effort). - The legacy (non-graph, `workflowGraphExecutor` off) path keeps the current working branch-group/per-task PR flow unchanged — the node lifecycle is the graph-executor path. Converging the legacy path onto nodes is out of scope. +- **Cutover-deferral record (U9, scoped):** U9 shipped its headline deliverable — the built-in PR workflow template (`builtin:pr-workflow`, `packages/core/src/builtin-pr-workflow-ir.ts`), wiring `pr-create → await-review → pr-respond → auto-merge gate → pr-merge → end` end to end behind the `workflowGraphExecutor` flag — **additively**, as a NEW built-in alongside the unchanged default `builtin:coding`. The retirement half of U9 (removing/gutting `PrCommentHandler`, `PrMonitor`, `pr-monitor-gh.ts`, and re-pointing `syncGroupPr` body-sync at the entity) is **deferred until the graph executor is the default**: those modules ARE the legacy flag-off path's PR handling and are wired into `scheduler.ts` + `project-engine.ts`, so removing them now would break the default flag-off path that this Scope-Boundaries note commits to keeping unchanged. The R20 scheduler-invariant test stays green (scheduler untouched). When the executor becomes the default, retire the legacy comment/monitor path and physically drop the frozen legacy `branch_groups` PR columns then. - Post-merge feedback notifications; GitHub-native auto-merge fallback; capturing gh rate-limit/pagination learnings into `docs/solutions/` once the reconcile ships. --- diff --git a/packages/core/src/__tests__/builtin-workflows.test.ts b/packages/core/src/__tests__/builtin-workflows.test.ts index 78afed3ed2..d81fc2c89e 100644 --- a/packages/core/src/__tests__/builtin-workflows.test.ts +++ b/packages/core/src/__tests__/builtin-workflows.test.ts @@ -3,14 +3,14 @@ import { describe, it, expect, beforeEach, afterEach } from "vitest"; import { BUILTIN_WORKFLOWS, getBuiltinWorkflow, isBuiltinWorkflowId } from "../builtin-workflows.js"; import { BUILTIN_CODING_WORKFLOW_IR } from "../builtin-coding-workflow-ir.js"; import { compileWorkflowToSteps } from "../workflow-compiler.js"; -import { DEFAULT_WORKFLOW_COLUMN_IDS, parseWorkflowIr } from "../workflow-ir.js"; +import { DEFAULT_WORKFLOW_COLUMN_IDS, parseWorkflowIr, serializeWorkflowIr } from "../workflow-ir.js"; import { createTaskStoreTestHarness } from "./store-test-helpers.js"; describe("built-in workflows", () => { // Graph-only built-ins (step inversion, KTD-9) model branching/foreach/rework // structure the linear compiler cannot lower to a step list — they run only // under the workflow graph executor. They still must parse as valid IR. - const GRAPH_ONLY_BUILTIN_IDS = new Set(["builtin:stepwise-coding"]); + const GRAPH_ONLY_BUILTIN_IDS = new Set(["builtin:stepwise-coding", "builtin:pr-workflow"]); it("every built-in has a valid IR; linear built-ins compile without error", () => { expect(BUILTIN_WORKFLOWS.length).toBeGreaterThanOrEqual(4); @@ -39,6 +39,45 @@ describe("built-in workflows", () => { expect(template.nodes.some((n) => n.config?.seam === "step-execute")).toBe(true); }); + it("includes the PR lifecycle built-in wiring the PR nodes end to end (U9)", () => { + const pr = getBuiltinWorkflow("builtin:pr-workflow"); + expect(pr).toBeDefined(); + const ir = parseWorkflowIr(pr!.ir); + if (ir.version !== "v2") throw new Error("expected v2"); + + // The three PR node kinds plus the await holds are all present. + const kinds = ir.nodes.map((n) => n.kind); + expect(kinds).toContain("pr-create"); + expect(kinds).toContain("pr-respond"); + expect(kinds).toContain("pr-merge"); + expect(ir.nodes.filter((n) => n.kind === "hold").length).toBeGreaterThanOrEqual(3); + + // The auto-merge gate (U6) routes after approval. + expect(ir.nodes.some((n) => n.kind === "gate" && (n.config as { gate?: string })?.gate === "auto-merge")).toBe(true); + + // await-review is the bounded-rework region head; pr-respond loops back to it. + const awaitReview = ir.nodes.find((n) => n.id === "await-review"); + expect((awaitReview?.config as { reworkRegion?: boolean })?.reworkRegion).toBe(true); + expect((awaitReview?.config as { release?: string })?.release).toBe("external-event"); + expect( + ir.edges.some((e) => e.from === "pr-respond" && e.to === "await-review" && e.kind === "rework"), + ).toBe(true); + + // The create→await-review→gate→merge→end spine exists. + expect(ir.edges.some((e) => e.from === "pr-create" && e.to === "await-review")).toBe(true); + expect(ir.edges.some((e) => e.from === "await-review" && e.to === "gate")).toBe(true); + expect(ir.edges.some((e) => e.from === "gate" && e.to === "pr-merge")).toBe(true); + expect(ir.edges.some((e) => e.from === "pr-merge" && e.to === "end")).toBe(true); + }); + + it("the PR built-in IR round-trips through serialize → parse unchanged (U9)", () => { + const pr = getBuiltinWorkflow("builtin:pr-workflow")!; + const serialized = serializeWorkflowIr(pr.ir); + const reparsed = parseWorkflowIr(serialized); + // Re-serializing the reparsed IR yields the identical bytes (stable round-trip). + expect(serializeWorkflowIr(reparsed)).toBe(serialized); + }); + it("default workflow column ids equal the legacy enum values, in legacy order (KTD-1)", () => { expect(BUILTIN_CODING_WORKFLOW_IR.version).toBe("v2"); if (BUILTIN_CODING_WORKFLOW_IR.version !== "v2") throw new Error("expected v2"); diff --git a/packages/core/src/__tests__/workflow-ir.test.ts b/packages/core/src/__tests__/workflow-ir.test.ts index 017678b613..62409d4b7a 100644 --- a/packages/core/src/__tests__/workflow-ir.test.ts +++ b/packages/core/src/__tests__/workflow-ir.test.ts @@ -265,6 +265,39 @@ describe("parseWorkflowIr — hold release kinds", () => { }); }); +describe("parseWorkflowIr — top-level rework region (U6/U9)", () => { + const cols = [{ id: "c", name: "C", traits: [] }]; + + // start → head(reworkRegion) → body → rework back to head; head also has a + // forward `outcome:rework-exhausted` edge out of the loop. This is the PR + // review-loop shape (await-review → pr-respond → rework back), generalized. + function reworkIr(headConfig: Record | undefined): WorkflowIrV2 { + return v2( + cols, + [ + { id: "start", kind: "start", column: "c" }, + { id: "head", kind: "hold", column: "c", config: { release: "external-event", ...headConfig } }, + { id: "body", kind: "prompt", column: "c" }, + { id: "end", kind: "end", column: "c" }, + ], + [ + { from: "start", to: "head" }, + { from: "head", to: "body", condition: "outcome:go" }, + { from: "head", to: "end", condition: "outcome:rework-exhausted" }, + { from: "body", to: "head", condition: "outcome:again", kind: "rework" }, + ], + ); + } + + it("accepts a top-level rework edge into a reworkRegion head", () => { + expect(() => parseWorkflowIr(reworkIr({ reworkRegion: true, maxReworkCycles: 5 }))).not.toThrow(); + }); + + it("rejects a top-level rework edge whose head is not a reworkRegion", () => { + expect(() => parseWorkflowIr(reworkIr(undefined))).toThrow(/only legal inside a foreach template/); + }); +}); + describe("parseWorkflowIr — split/join parallelism (KTD-11)", () => { const cols = [{ id: "c", name: "C", traits: [] }]; diff --git a/packages/core/src/builtin-pr-workflow-ir.ts b/packages/core/src/builtin-pr-workflow-ir.ts new file mode 100644 index 0000000000..1f774eaebf --- /dev/null +++ b/packages/core/src/builtin-pr-workflow-ir.ts @@ -0,0 +1,173 @@ +import type { WorkflowIr } from "./workflow-ir-types.js"; +import { parseWorkflowIr } from "./workflow-ir.js"; + +/** + * The built-in **PR** workflow (U9) — the unified PR-entity lifecycle wired end to + * end as first-class workflow-graph nodes and edges (R3/R20). It is the headline + * "wire it end to end" deliverable: a task/group routed through this workflow gets + * the full create → await-review → respond → auto-merge gate → merge → end + * lifecycle with no hand-authoring. + * + * It mirrors the way `builtin-stepwise-coding-workflow-ir` authors a v2 IR + * directly (the `linear` helper in `builtin-workflows.ts` only builds simple + * pipelines). Like every built-in it is read-only, and like the stepwise built-in + * it is **graph-only**: the PR node kinds (`pr-create`/`pr-respond`/`pr-merge`), + * the hold-based await columns, and the top-level rework loop are interpreter-only + * — it requires the `workflowGraphExecutor` flag at run time. + * + * start + * → pr-create (in-progress) + * outcome:open → await-review (hold, external-event release) + * outcome:failed → failed (hold, manual release) --retry--> pr-create + * → await-review (hold; the bounded-rework REGION HEAD) + * event changes-requested → pr-respond + * event approved → auto-merge gate + * event conflict → await-rebase (hold) --conflict-cleared--> await-review + * manual force-merge → pr-merge + * manual close → end (closed) + * → pr-respond + * --rework: pushed (bounded by maxReworkCycles)--> await-review + * outcome:rework-exhausted → await-review-hold (manual) --> await-review + * → gate (auto-merge?) + * outcome:auto-on → pr-merge + * outcome:auto-off → await-review (park for manual merge) + * → pr-merge + * outcome:merged-requested → end (reconcile corroborates `merged`) + * outcome:stale-head → await-review (re-evaluate against new head) + * → end + * + * **Await states are hold columns with `external-event`/`manual` release (U4).** + * The node handlers are fast/idempotent/fail-closed; the long waits (for review, + * for merge readiness, for a cleared conflict) are the holds. The node-agnostic + * GitHub reconcile (U4) fires the `github:pr-` external-event releases that + * advance whichever card is parked in an await hold — the scheduler has zero PR + * knowledge (R20). + * + * **The review→respond loop is the bounded rework cycle (U6).** `await-review` is + * the top-level rework region head (`config.reworkRegion: true`, + * `config.maxReworkCycles`); the `pr-respond --rework--> await-review` edge loops + * up to the cap and then routes `outcome:rework-exhausted` to a manual hold so the + * card parks instead of looping forever (R8). + * + * NOTE (cutover deferral): this is shipped ADDITIVELY — a NEW built-in alongside + * the default `builtin-coding-workflow`, which is unchanged. Full retirement of + * the legacy comment/monitor PR path (`PrCommentHandler`, `PrMonitor`, + * `pr-monitor-gh.ts`) is deferred until the graph executor is the default; the + * flag-OFF path keeps the current working branch-group/per-task PR flow unchanged. + * See the plan's "Deferred to follow-up work" note. + */ +const RAW_BUILTIN_PR_WORKFLOW_IR: WorkflowIr = { + version: "v2", + name: "builtin-pr", + columns: [ + { id: "triage", name: "Triage", traits: [{ trait: "intake" }] }, + { + id: "in-progress", + name: "In progress", + traits: [{ trait: "wip" }, { trait: "timing" }], + }, + { + id: "await-review", + name: "Awaiting review", + // The PR-await dwell column. The reconcile fires external-event releases that + // move whatever card is parked here; the generic hold-release sweep does the + // move (no PR knowledge in the substrate). + traits: [{ trait: "merge-blocker" }, { trait: "stall-detection" }], + }, + { id: "done", name: "Done", traits: [{ trait: "complete" }] }, + { id: "archived", name: "Archived", traits: [{ trait: "archived" }] }, + ], + nodes: [ + { id: "start", kind: "start", column: "triage" }, + // pr-create: open (or reuse) the PR and write the entity (creating → open), + // or record `failed` (routable, never thrown). It is also a (bounded) rework + // region head so the manual retry edge from the failed hold is a legal + // loop-back (the only other top-level cycle besides the review loop). + { id: "pr-create", kind: "pr-create", column: "in-progress", config: { reworkRegion: true, maxReworkCycles: 5 } }, + // Failed-creation hold: a human releases it (manual) to retry pr-create. + { id: "failed", kind: "hold", column: "in-progress", config: { release: "manual" } }, + // await-review: the long wait for a review event. ALSO the bounded-rework + // region head — the pr-respond rework edge loops back here up to the cap. + // External-event release is fired by the U4 reconcile (github:pr-); + // manual release covers user-controlled approve/force-merge/close edges. + { + id: "await-review", + kind: "hold", + column: "await-review", + config: { release: "external-event", reworkRegion: true, maxReworkCycles: 5 }, + }, + // pr-respond: the review-response run body (U5). Loops back to await-review on + // a push (the bounded rework edge). + { id: "pr-respond", kind: "pr-respond", column: "in-progress" }, + // Rework-exhaustion escalation: at the cap, park on a manual hold (a human + // releases it back to await-review) instead of looping forever (R8). + { + id: "await-review-hold", + kind: "hold", + column: "await-review", + config: { release: "manual" }, + }, + // Auto-merge gate (U6, R10): routes outcome:auto-on → pr-merge, + // outcome:auto-off → park back on await-review for a manual merge. + { id: "gate", kind: "gate", column: "await-review", config: { gate: "auto-merge" } }, + // await-rebase: the conflict dwell column. The reconcile fires + // github:pr-conflict-cleared to release it back to await-review. + { + id: "await-rebase", + kind: "hold", + column: "await-review", + config: { release: "external-event" }, + }, + // pr-merge: tool-side merge with expectedHeadOid. Does NOT write `merged` — + // the reconcile corroborates the terminal state. + { id: "pr-merge", kind: "pr-merge", column: "await-review" }, + { id: "end", kind: "end", column: "done" }, + ], + // Edge note: every loop-back to a region head (`await-review` or `pr-create`) + // is a `kind: "rework"` edge — the only legal top-level cycle (U6). Forward + // edges leaving a region head (changes-requested, approved, conflict, + // rework-exhausted, …) are plain. The executor re-runs the head under its + // bounded budget when a rework edge fires; at the cap it routes + // `outcome:rework-exhausted` out of the loop. + edges: [ + { from: "start", to: "pr-create" }, + // pr-create outcomes. + { from: "pr-create", to: "await-review", condition: "outcome:open" }, + { from: "pr-create", to: "failed", condition: "outcome:failed" }, + // pr-create node-level hard failure (source resolution) also parks on failed. + { from: "pr-create", to: "failed", condition: "failure" }, + // Manual retry from the failed hold loops back to pr-create (rework region: + // pr-create), bounded by pr-create's maxReworkCycles. + { from: "failed", to: "pr-create", condition: "success", kind: "rework" }, + // await-review release events (fired by the U4 reconcile / user controls). + // These LEAVE the region head, so they are plain forward edges. + { from: "await-review", to: "pr-respond", condition: "outcome:changes-requested" }, + { from: "await-review", to: "gate", condition: "outcome:approved" }, + { from: "await-review", to: "await-rebase", condition: "outcome:conflict" }, + { from: "await-review", to: "pr-merge", condition: "outcome:force-merge" }, + { from: "await-review", to: "end", condition: "outcome:close" }, + // Rework exhaustion (await-review is the region head): park on a manual hold. + { from: "await-review", to: "await-review-hold", condition: "outcome:rework-exhausted" }, + // pr-respond → bounded rework back to await-review (the review loop). Bounded + // by await-review's maxReworkCycles; at the cap the head routes rework-exhausted. + { from: "pr-respond", to: "await-review", condition: "outcome:fixed", kind: "rework" }, + { from: "pr-respond", to: "await-review", condition: "outcome:disagreed-only", kind: "rework" }, + // await-review-hold manual release → back to await-review (rework loop-back). + { from: "await-review-hold", to: "await-review", condition: "success", kind: "rework" }, + // Conflict cleared → back to await-review (rework loop-back). + { from: "await-rebase", to: "await-review", condition: "outcome:conflict-cleared", kind: "rework" }, + // auto-merge gate routing. auto-on goes forward to pr-merge; auto-off parks + // back on await-review for a manual merge (rework loop-back). + { from: "gate", to: "pr-merge", condition: "outcome:auto-on" }, + { from: "gate", to: "await-review", condition: "outcome:auto-off", kind: "rework" }, + // pr-merge outcomes: merged-requested ends (reconcile corroborates `merged`); + // a stale-head race re-evaluates against the new head via await-review. + { from: "pr-merge", to: "end", condition: "outcome:merged-requested" }, + { from: "pr-merge", to: "await-review", condition: "outcome:stale-head", kind: "rework" }, + // Defensive: a non-actionable / no-entity merge parks rather than dead-ends. + { from: "pr-merge", to: "await-review", condition: "outcome:not-actionable", kind: "rework" }, + { from: "pr-merge", to: "await-review", condition: "failure", kind: "rework" }, + ], +}; + +export const BUILTIN_PR_WORKFLOW_IR = parseWorkflowIr(RAW_BUILTIN_PR_WORKFLOW_IR); diff --git a/packages/core/src/builtin-workflows.ts b/packages/core/src/builtin-workflows.ts index 62e734cf44..6c8541d5b5 100644 --- a/packages/core/src/builtin-workflows.ts +++ b/packages/core/src/builtin-workflows.ts @@ -1,3 +1,4 @@ +import { BUILTIN_PR_WORKFLOW_IR } from "./builtin-pr-workflow-ir.js"; import { BUILTIN_STEPWISE_CODING_WORKFLOW_IR } from "./builtin-stepwise-coding-workflow-ir.js"; import type { WorkflowDefinition } from "./workflow-definition-types.js"; import type { WorkflowIr } from "./workflow-ir-types.js"; @@ -166,6 +167,40 @@ export const BUILTIN_WORKFLOWS: WorkflowDefinition[] = [ createdAt: BUILTIN_TS, updatedAt: BUILTIN_TS, }, + // The PR workflow (U9) — the unified PR-entity lifecycle wired end to end as + // first-class graph nodes/edges: pr-create → await-review (hold) → pr-respond + // (bounded rework loop) → auto-merge gate → pr-merge → end, with the await + // states modeled as hold columns the U4 reconcile advances via external-event + // releases. Authored directly as a v2 IR (the `linear` helper only builds + // simple pipelines); read-only like every built-in. Requires the + // `workflowGraphExecutor` flag at run time (pr-* node kinds, holds, and the + // top-level rework loop are interpreter-only). + // + // ADDITIVE: this is a NEW built-in alongside the unchanged default + // `builtin:coding`. Full retirement of the legacy comment/monitor PR path is + // deferred until the graph executor is the default (see the plan's "Deferred to + // follow-up work"). + { + id: "builtin:pr-workflow", + name: "PR lifecycle (built-in)", + description: + "The unified PR lifecycle as graph nodes: create the PR, await review, respond to changes (bounded rework loop), gate on auto-merge, then merge — with GitHub reconciliation advancing the await holds. Requires the workflow graph executor.", + ir: BUILTIN_PR_WORKFLOW_IR, + layout: { + start: { x: 60, y: 160 }, + "pr-create": { x: 230, y: 160 }, + failed: { x: 230, y: 320 }, + "await-review": { x: 400, y: 160 }, + "pr-respond": { x: 400, y: 320 }, + "await-review-hold": { x: 570, y: 320 }, + gate: { x: 570, y: 160 }, + "await-rebase": { x: 740, y: 320 }, + "pr-merge": { x: 740, y: 160 }, + end: { x: 910, y: 160 }, + }, + createdAt: BUILTIN_TS, + updatedAt: BUILTIN_TS, + }, ]; const BUILTIN_BY_ID = new Map(BUILTIN_WORKFLOWS.map((wf) => [wf.id, wf])); diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 7fba542ed3..e604db4eef 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -78,6 +78,7 @@ export { } from "./workflow-ir-types.js"; export { BUILTIN_CODING_WORKFLOW_IR } from "./builtin-coding-workflow-ir.js"; export { BUILTIN_STEPWISE_CODING_WORKFLOW_IR } from "./builtin-stepwise-coding-workflow-ir.js"; +export { BUILTIN_PR_WORKFLOW_IR } from "./builtin-pr-workflow-ir.js"; // ── Trait model (U2) ───────────────────────────────────────────────── export type { diff --git a/packages/core/src/workflow-ir.ts b/packages/core/src/workflow-ir.ts index 6a3cc5eff8..8fccc49dc1 100644 --- a/packages/core/src/workflow-ir.ts +++ b/packages/core/src/workflow-ir.ts @@ -782,15 +782,23 @@ function validateV2(ir: WorkflowIrV2): void { validateCodeNodes(ir.nodes); validateFields(ir.fields); - // Rework edges are legal only intra-template; any rework edge at the top level - // is rejected (template rework edges are validated inside validateForeach and - // never appear in ir.edges). + // Rework edges are legal intra-template (foreach, KTD-5) and — since U6 + // generalized the bounded-rework mechanism to the top-level walk — for a + // designated top-level rework region (the PR review loop: await-review → + // pr-respond → rework back to await-review). A top-level rework edge is legal + // ONLY when its target (the loop head) explicitly opts in via + // `config.reworkRegion === true`; the executor seeds the bound from that head's + // `config.maxReworkCycles` (shared default + clamp). This keeps every other + // top-level back-edge rejected (validateNoIllegalCycles below still throws for + // non-rework cycles), so the relaxation is narrow and opt-in. for (const edge of ir.edges) { - if (isReworkEdge(edge)) { - throw new WorkflowIrError( - `rework edge '${edge.from}' -> '${edge.to}' is only legal inside a foreach template`, - ); - } + if (!isReworkEdge(edge)) continue; + const head = nodesById.get(edge.to); + if (head?.config?.reworkRegion === true) continue; + throw new WorkflowIrError( + `rework edge '${edge.from}' -> '${edge.to}' is only legal inside a foreach template ` + + `or into a top-level rework region head (config.reworkRegion: true)`, + ); } validateNoIllegalCycles(ir.nodes, outgoing); diff --git a/packages/engine/src/__tests__/pr-workflow-e2e.test.ts b/packages/engine/src/__tests__/pr-workflow-e2e.test.ts new file mode 100644 index 0000000000..3a36b7e021 --- /dev/null +++ b/packages/engine/src/__tests__/pr-workflow-e2e.test.ts @@ -0,0 +1,365 @@ +/** + * U9 — built-in PR workflow end-to-end (FAST, faked GitHub + faked agent). + * + * Proves the headline "wire it end to end" deliverable: a task routed through the + * built-in PR workflow graph (`BUILTIN_PR_WORKFLOW_IR`) flows through the full + * node lifecycle — create → await-review → (changes-requested) respond → + * (approved) auto-merge gate → merge → end — with the U4 reconcile firing the + * external-event releases that advance the await holds. + * + * The executor cannot itself park at a hold (holds are dwell columns the runtime + * parks/resumes the card at; the executor has no hold handler). So this drives the + * lifecycle in the same SEGMENTS the runtime does, resuming the graph at each next + * node, and uses the real {@link PrReconciler} to prove a GitHub state change fires + * the matching `github:pr-` release between segments. The PR node handlers + * (pr-create / pr-respond / pr-merge / auto-merge gate) run with injected fakes — + * the engine never touches a real GitHub client. + * + * It also pins that the built-in IR parses/validates and round-trips. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { mkdtempSync } from "node:fs"; +import { rm } from "node:fs/promises"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; +import { + BUILTIN_PR_WORKFLOW_IR, + TaskStore, + parseWorkflowIr, + serializeWorkflowIr, +} from "@fusion/core"; +import type { PrEntity, TaskDetail, WorkflowIr } from "@fusion/core"; + +import { WorkflowGraphExecutor } from "../workflow-graph-executor.js"; +import type { + PrMergeCallResult, + PrNodeDeps, + PrRespondCallResult, + PrSourceDescriptor, +} from "../pr-nodes.js"; +import { + PrReconciler, + type PrReconcileFetchResult, + type PrReconcileGithubOps, +} from "../pr-reconcile.js"; + +const settingsOn = () => ({ experimentalFeatures: { workflowGraphExecutor: true } }); + +const SOURCE: PrSourceDescriptor = { + sourceType: "task", + sourceId: "T-1", + repo: "owner/repo", + headBranch: "fusion/t-1", +}; + +const TASK = { id: "T-1" } as TaskDetail; + +/** A focused sub-IR mirroring a segment of the built-in graph, so the executor + * resumes at a single runnable node and stops at the next hold/end — exactly the + * way the runtime resumes a parked card. */ +function segment(name: string, nodes: WorkflowIr["nodes"], edges: WorkflowIr["edges"]): WorkflowIr { + return { version: "v1", name, nodes, edges }; +} + +describe("built-in PR workflow — static validity (U9)", () => { + it("parses/validates as a v2 IR with the PR node lifecycle", () => { + const ir = parseWorkflowIr(BUILTIN_PR_WORKFLOW_IR); + expect(ir.version).toBe("v2"); + const kinds = ir.nodes.map((n) => n.kind); + expect(kinds).toContain("pr-create"); + expect(kinds).toContain("pr-respond"); + expect(kinds).toContain("pr-merge"); + expect(kinds).toContain("hold"); + // The bounded review loop is a top-level rework edge into the region head. + expect( + ir.edges.some((e) => e.from === "pr-respond" && e.to === "await-review" && e.kind === "rework"), + ).toBe(true); + }); + + it("round-trips serialize → parse unchanged", () => { + const serialized = serializeWorkflowIr(BUILTIN_PR_WORKFLOW_IR); + expect(serializeWorkflowIr(parseWorkflowIr(serialized))).toBe(serialized); + }); +}); + +describe("built-in PR workflow — node lifecycle end to end (U9)", () => { + let rootDir: string; + let globalDir: string; + let store: TaskStore; + + beforeEach(async () => { + rootDir = mkdtempSync(join(tmpdir(), "fusion-pr-e2e-")); + globalDir = mkdtempSync(join(tmpdir(), "fusion-pr-e2e-global-")); + store = new TaskStore(rootDir, globalDir, { inMemoryDb: true }); + await store.init(); + }); + + afterEach(async () => { + store.close(); + await rm(rootDir, { recursive: true, force: true, maxRetries: 5, retryDelay: 50 }); + await rm(globalDir, { recursive: true, force: true, maxRetries: 5, retryDelay: 50 }); + }); + + // ── Fakes ────────────────────────────────────────────────────────────────── + + /** A scriptable fake reconcile GitHub-ops returning a chosen deep-fetch state. */ + function makeReconcileOps(fetch: () => PrReconcileFetchResult): { + ops: PrReconcileGithubOps; + fetchCalls: number; + } { + const state = { fetchCalls: 0 }; + return { + get fetchCalls() { + return state.fetchCalls; + }, + ops: { + probe: async () => ({ changed: true, etag: "etag" }), + fetchPrState: async () => { + state.fetchCalls += 1; + return fetch(); + }, + }, + }; + } + + function makeReconciler(ops: PrReconcileGithubOps): { reconciler: PrReconciler; fired: string[] } { + const fired: string[] = []; + const reconciler = new PrReconciler({ + store, + ops, + releaseByEvent: async (taskId: string, tag: string) => { + fired.push(`${taskId}::${tag}`); + return { released: true }; + }, + setTimer: () => 0 as unknown as ReturnType, + clearTimer: () => {}, + }); + return { reconciler, fired }; + } + + function prDeps(overrides: Partial = {}): PrNodeDeps { + return { + getStore: () => store, + resolvePrSource: () => SOURCE, + createPr: async () => ({ prNumber: 7, prUrl: "https://github.com/owner/repo/pull/7", headOid: "head-1" }), + mergePr: async () => ({ status: "merged-requested" }) as PrMergeCallResult, + ...overrides, + }; + } + + it("drives create → await-review → respond → gate → merge → end with reconcile-fired releases", async () => { + const respond = vi.fn(async (): Promise => ({ value: "fixed" })); + const mergePr = vi.fn(async () => ({ status: "merged-requested" }) as PrMergeCallResult); + const deps = prDeps({ respond, mergePr }); + + // ── Segment 1: start → pr-create → (await-review). The executor stops where + // the built-in would park at the await-review hold. ──────────────────────── + const createExec = new WorkflowGraphExecutor({ prNodes: deps }); + const createResult = await createExec.run( + TASK, + settingsOn(), + segment( + "seg-create", + [ + { id: "start", kind: "start" }, + { id: "pr-create", kind: "pr-create" }, + { id: "await-review", kind: "end" }, // hold stand-in (the parking point) + ], + [ + { from: "start", to: "pr-create" }, + { from: "pr-create", to: "await-review", condition: "outcome:open" }, + ], + ), + ); + expect(createResult.outcome).toBe("success"); + expect(createResult.visitedNodeIds).toContain("pr-create"); + const opened = store.getActivePrEntityBySource("task", "T-1"); + expect(opened?.state).toBe("open"); + expect(opened?.prNumber).toBe(7); + // Verified so the gate/respond hard-gate (R19) does not block it. + store.updatePrEntity(opened!.id, { unverified: false }); + + // ── Reconcile fires changes-requested → the await-review hold releases to + // pr-respond. ──────────────────────────────────────────────────────────── + const cr = makeReconcileOps(() => ({ exists: true, prState: "open", prNumber: 7, reviewDecision: "CHANGES_REQUESTED" })); + const r1 = makeReconciler(cr.ops); + const fired1 = await r1.reconciler.reconcileRepoOnce("owner/repo"); + expect(fired1.map((t) => t.event)).toContain("changes-requested"); + expect(r1.fired).toContain("T-1::github:pr-changes-requested"); + + // ── Segment 2: pr-respond runs the (faked) review-response and loops back to + // the await-review hold (the bounded rework edge). ─────────────────────────── + const respondExec = new WorkflowGraphExecutor({ prNodes: deps }); + const respondResult = await respondExec.run( + TASK, + settingsOn(), + segment( + "seg-respond", + [ + { id: "start", kind: "start" }, + { id: "pr-respond", kind: "pr-respond" }, + { id: "await-review", kind: "end" }, // loop back to the await hold + ], + [ + { from: "start", to: "pr-respond" }, + { from: "pr-respond", to: "await-review", condition: "outcome:fixed" }, + ], + ), + ); + expect(respondResult.visitedNodeIds).toContain("pr-respond"); + expect(respond).toHaveBeenCalledTimes(1); + // The rework-cycle counter advanced (R8 cap backing, persisted). + expect(store.getActivePrEntityBySource("task", "T-1")?.responseRounds).toBe(1); + + // ── Reconcile fires approved → the await-review hold releases to the gate. ── + const ap = makeReconcileOps(() => ({ + exists: true, + prState: "open", + prNumber: 7, + headOid: "head-1", // a real deep-fetch returns the corroborated head OID + reviewDecision: "APPROVED", + checksRollup: "success", + mergeable: "clean", + })); + const r2 = makeReconciler(ap.ops); + const fired2 = await r2.reconciler.reconcileRepoOnce("owner/repo"); + expect(fired2.map((t) => t.event)).toContain("approved"); + expect(r2.fired).toContain("T-1::github:pr-approved"); + + // Opt in to auto-merge so the gate routes auto-on → pr-merge. + const approved = store.getActivePrEntityBySource("task", "T-1")!; + store.updatePrEntity(approved.id, { autoMerge: true }); + expect(approved.reviewDecision).toBe("APPROVED"); + + // ── Segment 3: gate (auto-merge) → pr-merge → end. ────────────────────────── + const mergeExec = new WorkflowGraphExecutor({ prNodes: deps }); + const mergeResult = await mergeExec.run( + TASK, + settingsOn(), + segment( + "seg-gate-merge", + [ + { id: "start", kind: "start" }, + { id: "gate", kind: "gate", config: { gate: "auto-merge" } }, + { id: "pr-merge", kind: "pr-merge" }, + { id: "await-review", kind: "end" }, // auto-off would park here + { id: "end", kind: "end" }, + ], + [ + { from: "start", to: "gate" }, + { from: "gate", to: "pr-merge", condition: "outcome:auto-on" }, + { from: "gate", to: "await-review", condition: "outcome:auto-off" }, + { from: "pr-merge", to: "end", condition: "outcome:merged-requested" }, + ], + ), + ); + expect(mergeResult.outcome).toBe("success"); + // `end` nodes are terminal sinks the executor never adds to visitedNodeIds. + expect(mergeResult.visitedNodeIds).toEqual(["start", "gate", "pr-merge"]); + expect(mergePr).toHaveBeenCalledTimes(1); + expect(mergePr).toHaveBeenCalledWith(expect.objectContaining({ expectedHeadOid: "head-1" })); + // pr-merge does NOT write the terminal state — reconcile corroborates it. + expect(store.getActivePrEntityBySource("task", "T-1")?.state).toBe("open"); + + // ── Reconcile fires merged → entity goes terminal and drops from the poll + // set (the run ends). ─────────────────────────────────────────────────────── + const mg = makeReconcileOps(() => ({ exists: true, prState: "merged", prNumber: 7 })); + const r3 = makeReconciler(mg.ops); + const fired3 = await r3.reconciler.reconcileRepoOnce("owner/repo"); + expect(fired3.map((t) => t.event)).toEqual(["merged"]); + expect(r3.fired).toContain("T-1::github:pr-merged"); + expect(store.getActivePrEntityBySource("task", "T-1")).toBeNull(); + expect(store.listActivePrEntities()).toHaveLength(0); + }); + + it("auto-merge OFF parks for manual merge instead of reaching pr-merge", async () => { + // Seed an open, approved-but-not-opted-in entity. + const entity = store.ensurePrEntityForSource({ ...SOURCE, state: "open" }); + store.updatePrEntity(entity.id, { + state: "open", + unverified: false, + reviewDecision: "APPROVED", + checksRollup: "success", + mergeable: "clean", + autoMerge: false, // not opted in → gate must route auto-off + headOid: "head-1", + }); + + const mergePr = vi.fn(async () => ({ status: "merged-requested" }) as PrMergeCallResult); + // `park` is a script (a runnable parking sink) so the executor visits it — + // an `end` node is a terminal sink the executor never adds to visitedNodeIds. + const park = vi.fn(async () => ({ outcome: "success" as const })); + const exec = new WorkflowGraphExecutor({ prNodes: prDeps({ mergePr }), handlers: { script: park } }); + const result = await exec.run( + TASK, + settingsOn(), + segment( + "seg-auto-off", + [ + { id: "start", kind: "start" }, + { id: "gate", kind: "gate", config: { gate: "auto-merge" } }, + { id: "pr-merge", kind: "pr-merge" }, + { id: "park", kind: "script" }, + { id: "end", kind: "end" }, + ], + [ + { from: "start", to: "gate" }, + { from: "gate", to: "pr-merge", condition: "outcome:auto-on" }, + { from: "gate", to: "park", condition: "outcome:auto-off" }, + { from: "park", to: "end" }, + ], + ), + ); + expect(result.visitedNodeIds).toContain("park"); + expect(result.visitedNodeIds).not.toContain("pr-merge"); + expect(mergePr).not.toHaveBeenCalled(); + }); + + it("the bounded review loop runs the built-in IR's rework region to its cap", async () => { + // Run the real built-in IR's review region as a top-level rework loop: a + // respond that always returns `fixed` keeps the rework edge firing until the + // await-review head's maxReworkCycles budget exhausts and routes out. This + // pins the built-in's rework wiring against the executor's bound enforcement. + const entity = store.ensurePrEntityForSource({ ...SOURCE, state: "open" }); + store.updatePrEntity(entity.id, { state: "open", unverified: false, headOid: "h" }); + + const respond = vi.fn(async (): Promise => ({ value: "fixed" })); + // Exhaustion routes to a runnable parking sink (not an `end`), so the run's + // terminal outcome is that sink's success — mirroring the foreach exhaustion + // posture (the head's exhaustion result is `failure` only to deselect the + // success loop edge; the exhausted target then runs). + const parked = vi.fn(async () => ({ outcome: "success" as const })); + const exec = new WorkflowGraphExecutor({ prNodes: prDeps({ respond }), handlers: { script: parked } }); + + // Mirror the built-in's region head config (reworkRegion + a small cap) so the + // executor seeds the same bounded budget. + const cap = 3; + const ir = segment( + "review-loop", + [ + { id: "start", kind: "start" }, + { id: "await-review", kind: "gate", config: { reworkRegion: true, maxReworkCycles: cap } }, + { id: "pr-respond", kind: "pr-respond" }, + { id: "parked", kind: "script" }, + { id: "end", kind: "end" }, + ], + [ + { from: "start", to: "await-review" }, + // Region head's forward edges: keep looping (success) vs exit (exhausted). + { from: "await-review", to: "pr-respond", condition: "success" }, + { from: "await-review", to: "parked", condition: "outcome:rework-exhausted" }, + { from: "parked", to: "end" }, + { from: "pr-respond", to: "await-review", condition: "outcome:fixed", kind: "rework" }, + ], + ); + + const result = await exec.run(TASK, settingsOn(), ir); + expect(result.outcome).toBe("success"); + expect(result.visitedNodeIds).toContain("parked"); + // Initial pass + `cap` rework re-entries → pr-respond runs cap+1 times, then + // the head's budget exhausts and routes out of the loop exactly once. + expect(respond).toHaveBeenCalledTimes(cap + 1); + expect(parked).toHaveBeenCalledTimes(1); + expect(store.getActivePrEntityBySource("task", "T-1")?.responseRounds).toBe(cap + 1); + }); +});