diff --git a/.changeset/fn-8768-plan-review-approval.md b/.changeset/fn-8768-plan-review-approval.md new file mode 100644 index 0000000000..3bf95abd21 --- /dev/null +++ b/.changeset/fn-8768-plan-review-approval.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Resume approved plans immediately after Plan Review exhausts its revision budget. +category: fix +dev: Records an audited human Plan Review bypass and wakes scheduler and deferred workflow continuations. diff --git a/packages/core/src/__tests__/plan-approval.test.ts b/packages/core/src/__tests__/plan-approval.test.ts index 77661c8c03..bca43e624b 100644 --- a/packages/core/src/__tests__/plan-approval.test.ts +++ b/packages/core/src/__tests__/plan-approval.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { computePlanApprovalFingerprint, resolvePlanApprovalRequired, type PlanApprovalMode } from "../planner/plan-approval.js"; +import { computePlanApprovalFingerprint, isPlanReviewSatisfied, resolvePlanApprovalRequired, type PlanApprovalMode } from "../planner/plan-approval.js"; import { applyFrontendUxCriteria } from "../tasks/frontend-ux-policy.js"; import { applyOriginalDescription } from "../tasks/original-description-policy.js"; @@ -38,6 +38,38 @@ describe("resolvePlanApprovalRequired", () => { }); }); +describe("isPlanReviewSatisfied", () => { + it("accepts either a reviewer pass or an explicit operator bypass", () => { + expect(isPlanReviewSatisfied({ workflowStepId: "plan-review", workflowStepName: "Plan Review", status: "passed" })).toBe(true); + expect(isPlanReviewSatisfied({ + workflowStepId: "plan-review", + workflowStepName: "Plan Review", + status: "skipped", + bypassedBy: "operator", + bypassedAt: "2026-08-03T23:53:04.539Z", + bypassReason: "Approved after Plan Review did not converge", + bypassedFromStatus: "failed", + bypassedFromVerdict: "REVISE", + })).toBe(true); + }); + + it("rejects unrelated, failed, or unaudited skipped results", () => { + expect(isPlanReviewSatisfied({ workflowStepId: "code-review", workflowStepName: "Code Review", status: "passed" })).toBe(false); + expect(isPlanReviewSatisfied({ workflowStepId: "plan-review", workflowStepName: "Plan Review", status: "failed" })).toBe(false); + expect(isPlanReviewSatisfied({ workflowStepId: "plan-review", workflowStepName: "Plan Review", status: "skipped" })).toBe(false); + expect(isPlanReviewSatisfied({ + workflowStepId: "plan-review", + workflowStepName: "Plan Review", + status: "skipped", + bypassedBy: "operator", + bypassedAt: "2026-08-03T23:53:04.539Z", + bypassReason: "Malformed override", + bypassedFromStatus: "passed", + bypassedFromVerdict: "REVISE", + })).toBe(false); + }); +}); + /* * FNXC:PlanApproval 2026-07-04-22:41: * FN-7569 — computePlanApprovalFingerprint coverage: stable for identical content, normalizes only diff --git a/packages/core/src/index.gate.ts b/packages/core/src/index.gate.ts index a88673bc51..adf30a68d0 100644 --- a/packages/core/src/index.gate.ts +++ b/packages/core/src/index.gate.ts @@ -85,7 +85,7 @@ export { resolveEffectivePluginSettings, } from "./plugins/plugin-prompt-condition.js"; export type { PromptConditionEvaluationResult } from "./plugins/plugin-prompt-condition.js"; -export { computePlanApprovalFingerprint, resolvePlanApprovalRequired } from "./planner/plan-approval.js"; +export { computePlanApprovalFingerprint, isPlanReviewSatisfied, resolvePlanApprovalRequired } from "./planner/plan-approval.js"; export type { PlanApprovalMode } from "./planner/plan-approval.js"; export { isActiveNearDuplicateColumn, isNearDuplicateCanonicalInactive } from "./duplicates/near-duplicate-canonical.js"; export type { NearDuplicateCanonicalState } from "./duplicates/near-duplicate-canonical.js"; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 5af13b9328..8e78559c0c 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -93,7 +93,7 @@ export { resolveEffectivePluginSettings, } from "./plugins/plugin-prompt-condition.js"; export type { PromptConditionEvaluationResult } from "./plugins/plugin-prompt-condition.js"; -export { computePlanApprovalFingerprint, resolvePlanApprovalRequired } from "./planner/plan-approval.js"; +export { computePlanApprovalFingerprint, isPlanReviewSatisfied, resolvePlanApprovalRequired } from "./planner/plan-approval.js"; export type { PlanApprovalMode } from "./planner/plan-approval.js"; export { isActiveNearDuplicateColumn, isNearDuplicateCanonicalInactive } from "./duplicates/near-duplicate-canonical.js"; export type { NearDuplicateCanonicalState } from "./duplicates/near-duplicate-canonical.js"; diff --git a/packages/core/src/planner/plan-approval.ts b/packages/core/src/planner/plan-approval.ts index 338266bb5c..17b709b61f 100644 --- a/packages/core/src/planner/plan-approval.ts +++ b/packages/core/src/planner/plan-approval.ts @@ -6,9 +6,31 @@ import { } from "../tasks/original-description-policy.js"; import { FRONTEND_UX_CRITERIA_SECTION } from "../tasks/frontend-ux-policy.js"; import type { ProjectSettings } from "../types.js"; +import type { WorkflowStepResult } from "../types/workflow/workflow-steps.js"; +import { PLAN_REVIEW_GROUP_ID } from "../workflows/builtin-plan-review-group.js"; export type PlanApprovalMode = NonNullable; +/* +FNXC:PlanReviewApproval 2026-08-04-00:26: +Plan Review is terminal when the reviewer passed it or an operator durably accepted the final +failed REVISE after the revision cap. Require the full audited source state so a malformed skip +cannot silently open the execution gate. +*/ +export function isPlanReviewSatisfied(result: WorkflowStepResult): boolean { + if (result.workflowStepId !== PLAN_REVIEW_GROUP_ID) return false; + if (result.status === "passed") return true; + return result.status === "skipped" + && (result.bypassedFromStatus === "failed" || result.bypassedFromStatus === "advisory_failure") + && result.bypassedFromVerdict === "REVISE" + && typeof result.bypassedBy === "string" + && result.bypassedBy.trim().length > 0 + && typeof result.bypassedAt === "string" + && result.bypassedAt.trim().length > 0 + && typeof result.bypassReason === "string" + && result.bypassReason.trim().length > 0; +} + /** * FNXC:PlanApproval 2026-07-04-22:41: * FN-7569 — manual plan approval was not idempotent against unchanged plan content: an diff --git a/packages/core/src/task-store/async/async-workflow-workitems.ts b/packages/core/src/task-store/async/async-workflow-workitems.ts index c2ca6041f9..a80d4c1759 100644 --- a/packages/core/src/task-store/async/async-workflow-workitems.ts +++ b/packages/core/src/task-store/async/async-workflow-workitems.ts @@ -35,8 +35,10 @@ import type { WorkflowWorkItemState, WorkflowWorkItemTransitionPatch, WorkflowWorkItemUpsertInput, + WorkflowStepResult, } from "../../types.js"; import type { WorkflowWorkItemRow } from "../row-types.js"; +import { isPlanReviewSatisfied } from "../../planner/plan-approval.js"; /** * FNXC:TaskStoreWorkflowWorkItems 2026-06-24-08:35: @@ -319,8 +321,8 @@ export async function seedStrandedPlanReviewContinuation( projectScopeFor(schema.project.tasks.projectId, layer.projectId), eq(schema.project.tasks.id, input.taskId), )).limit(1); - const results = taskRows[0]?.workflowStepResults as Array<{ workflowStepId?: string; status?: string }> | null | undefined; - if (results?.some((result) => result.workflowStepId === "plan-review" && result.status === "passed")) return { seeded: false, reason: "plan-review-passed" as const }; + const results = taskRows[0]?.workflowStepResults as WorkflowStepResult[] | null | undefined; + if (results?.some(isPlanReviewSatisfied)) return { seeded: false, reason: "plan-review-passed" as const }; const item = await upsertWorkflowWorkItem(layer, input, tx); return { seeded: true, workItemId: item.id }; })); diff --git a/packages/core/src/task-store/project-store-ops.ts b/packages/core/src/task-store/project-store-ops.ts index 8140c6d31a..00461955d7 100644 --- a/packages/core/src/task-store/project-store-ops.ts +++ b/packages/core/src/task-store/project-store-ops.ts @@ -41,6 +41,7 @@ import {readTaskRowInTransaction} from "./async/async-persistence.js"; import {withTaskWorkflowSerialization} from "./async/async-workflow-workitems.js"; import {recordActivityLogEntry as recordActivityLogEntryAsync} from "./async/async-audit.js"; import {applyOriginalDescription} from "../tasks/original-description-policy.js"; +import {isPlanReviewSatisfied} from "../planner/plan-approval.js"; import {recordRunAuditEvent as recordRunAuditEventAsync} from "../postgres/data-layer.js"; import {listGoalCitations as listGoalCitationsAsync} from "./async/async-events.js"; import type {RunAuditEventRow} from "../task-store/row-types.js"; @@ -175,7 +176,7 @@ export async function atomicWriteTaskJsonWithAuditImpl(store: TaskStore, dir: st seeding. This prevents a pass from committing between that repair's locked predicate reads and its insert. */ - if (task.workflowStepResults?.some((result) => result.workflowStepId === "plan-review" && result.status === "passed")) { + if (task.workflowStepResults?.some(isPlanReviewSatisfied)) { return withTaskWorkflowSerialization(tx, layer.projectId, id, persist); } return persist(); diff --git a/packages/core/src/task-store/workflow-task-create-ops.ts b/packages/core/src/task-store/workflow-task-create-ops.ts index 411ff92a12..e2a10b50f0 100644 --- a/packages/core/src/task-store/workflow-task-create-ops.ts +++ b/packages/core/src/task-store/workflow-task-create-ops.ts @@ -35,6 +35,7 @@ import {__setTaskActivityLogLimitsForTesting} from "../task-store/comments.js"; import {withTaskBranchContextInSourceMetadata} from "../task-store/branch-context.js"; import {upsertTaskRowInTransaction, readTaskRowInTransaction, buildTaskInsertValues} from "../task-store/async/async-persistence.js"; import {preserveResolvedTaskWedgeEpisode} from "../task-store/persistence.js"; +import {isPlanReviewSatisfied} from "../planner/plan-approval.js"; import {listDueWorkflowWorkItems as listDueWorkflowWorkItemsAsync, withTaskWorkflowSerialization} from "../task-store/async/async-workflow-workitems.js"; import {getTaskMovedCountsByDay as getTaskMovedCountsByDayAsync} from "../task-store/async/async-audit.js"; import {getAllDocuments as getAllDocumentsAsync} from "../task-store/async/async-comments-attachments.js"; @@ -138,7 +139,7 @@ export async function atomicWriteTaskJsonImpl2(store: TaskStore, dir: string, ta sole terminal result writer, so a plan-review pass must take that same lock as the first transaction lock before its row write can commit. */ - if (task.workflowStepResults?.some((result) => result.workflowStepId === "plan-review" && result.status === "passed")) { + if (task.workflowStepResults?.some(isPlanReviewSatisfied)) { await withTaskWorkflowSerialization(tx, layer.projectId, id, persist); } else { await persist(); diff --git a/packages/dashboard/src/__tests__/plan-approval-status.pg.test.ts b/packages/dashboard/src/__tests__/plan-approval-status.pg.test.ts index 4b07afdc5a..7ee75061b4 100644 --- a/packages/dashboard/src/__tests__/plan-approval-status.pg.test.ts +++ b/packages/dashboard/src/__tests__/plan-approval-status.pg.test.ts @@ -54,6 +54,98 @@ pgDescribe("plan approval status persistence", () => { expect(response.body.approvedPlanFingerprint).toBe(persisted.approvedPlanFingerprint); }); + it.each(["failed", "advisory_failure"] as const)( + "durably bypasses an exhausted %s Plan Review before clearing its approval hold", + async (reviewStatus) => { + const task = await store.createTask({ description: "Approve after Plan Review did not converge" }); + await store.updateTask(task.id, { + status: "awaiting-approval", + awaitingApprovalReason: "plan-review-replan-cap", + workflowStepResults: [{ + workflowStepId: "plan-review", + workflowStepName: "Plan Review", + phase: "pre-merge", + source: "optional-group", + status: reviewStatus, + verdict: "REVISE", + output: "The plan still needs revision.", + priorAttempts: [{ + workflowStepId: "plan-review", + workflowStepName: "Plan Review", + status: "failed", + verdict: "REVISE", + output: "Earlier revision request.", + }], + }], + } as never); + + const taskDir = join(harness.rootDir, ".fusion", "tasks", task.id); + await mkdir(taskDir, { recursive: true }); + await writeFile(join(taskDir, "PROMPT.md"), "# Human-approved plan\n", "utf8"); + + const response = await request(createApp(), "POST", `/api/tasks/${task.id}/approve-plan`); + + expect(response.status).toBe(200); + const persisted = await store.getTask(task.id); + expect(persisted.status).toBeUndefined(); + expect(persisted.awaitingApprovalReason).toBeUndefined(); + expect(persisted.workflowStepResults).toContainEqual(expect.objectContaining({ + workflowStepId: "plan-review", + status: "skipped", + bypassedBy: "dashboard-operator", + bypassReason: "Approved after Plan Review did not converge", + bypassedFromStatus: reviewStatus, + bypassedFromVerdict: "REVISE", + priorAttempts: [expect.objectContaining({ output: "Earlier revision request." })], + })); + expect(persisted.workflowStepResults?.[0]?.verdict).toBeUndefined(); + }, + ); + + it("approves an exhausted Plan Review from a split workflow's review column", async () => { + const task = await store.createTask({ description: "Approve legacy split-column review" }); + await store.writeTaskWorkflowSelection(task.id, "builtin:legacy-coding", []); + await store.updateTask(task.id, { + status: "awaiting-approval", + awaitingApprovalReason: "plan-review-replan-cap", + workflowStepResults: [{ + workflowStepId: "plan-review", + workflowStepName: "Plan Review", + status: "failed", + verdict: "REVISE", + }], + } as never); + + const response = await request(createApp(), "POST", `/api/tasks/${task.id}/approve-plan`); + + expect(response.status).toBe(200); + const persisted = await store.getTask(task.id); + expect(persisted.column).toBe("todo"); + expect(persisted.status).toBeUndefined(); + expect(persisted.workflowStepResults).toContainEqual(expect.objectContaining({ + workflowStepId: "plan-review", + status: "skipped", + bypassedFromStatus: "failed", + bypassedFromVerdict: "REVISE", + })); + }); + + it("keeps the approval hold when cap metadata has no failed REVISE result", async () => { + const task = await store.createTask({ description: "Malformed exhausted review state" }); + await store.updateTask(task.id, { + status: "awaiting-approval", + awaitingApprovalReason: "plan-review-replan-cap", + workflowStepResults: [], + } as never); + + const response = await request(createApp(), "POST", `/api/tasks/${task.id}/approve-plan`); + + expect(response.status).toBe(409); + const persisted = await store.getTask(task.id); + expect(persisted.status).toBe("awaiting-approval"); + expect(persisted.awaitingApprovalReason).toBe("plan-review-replan-cap"); + }); + it("clears a prior fingerprint when the approved plan cannot be read", async () => { const task = await store.createTask({ description: "Approve without a readable plan" }); await store.updateTask(task.id, { diff --git a/packages/dashboard/src/routes/register-task-workflow-routes.ts b/packages/dashboard/src/routes/register-task-workflow-routes.ts index b10eaccdb6..6eb99997a7 100644 --- a/packages/dashboard/src/routes/register-task-workflow-routes.ts +++ b/packages/dashboard/src/routes/register-task-workflow-routes.ts @@ -64,6 +64,7 @@ import { columnsWithFlag, resolveReboundTarget, resolveColumnFlags, + PLAN_REVIEW_GROUP_ID, TransitionRejectionError, ArchivedTaskDocumentPublicationRejectedError, TaskDocumentPreconditionFailedError, @@ -4018,6 +4019,22 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork firing; it started firing on everything. */ const approveIntakeColumn = await resolveIntakeColumnForTask(scopedStore, task.id); + let approveColumn = approveIntakeColumn; + /* + FNXC:PlanReviewApproval 2026-08-04-00:26: + An exhausted Plan Review is parked in the review node's column. That column is not always + the workflow intake column (`builtin:legacy-coding` uses todo vs triage), so the operator's + terminal approval must be accepted where the failed review actually ran. + */ + if (task.awaitingApprovalReason === "plan-review-replan-cap") { + try { + const ir = await resolveWorkflowIrForTask(scopedStore, task.id); + approveColumn = ir.nodes.find((node) => node.id === PLAN_REVIEW_GROUP_ID)?.column + ?? approveIntakeColumn; + } catch { + // Preserve the existing intake fallback when workflow resolution is unavailable. + } + } /* The resolved column ONLY — the legacy-`triage` disjunct this comment used to justify is gone (PR #2614 review — greptile: the comment outlived the code). @@ -4027,8 +4044,8 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork no test in either direction. A guard that accepts a column no workflow declares is not caution, it is an unreachable branch that reads like a requirement. */ - if (task.column !== approveIntakeColumn) { - throw badRequest(`Task must be in the '${approveIntakeColumn}' column to approve plan`); + if (task.column !== approveColumn) { + throw badRequest(`Task must be in the '${approveColumn}' column to approve plan`); } if (task.status !== "awaiting-approval") { throw badRequest("Task must have status 'awaiting-approval' to approve plan"); @@ -4039,9 +4056,6 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork // awaitingApprovalReason === "release-authorization" is gone too, so tasks parked by // the old gate can now be approved normally instead of staying stuck with no exit. - // Log the approval - await scopedStore.logEntry(task.id, "Plan approved by user"); - /* * FNXC:PlanApproval 2026-07-04-22:41: * FN-7569 — persist a fingerprint of the exact PROMPT.md the operator just approved @@ -4062,6 +4076,48 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork // No PROMPT.md to fingerprint (unusual for an awaiting-approval task) — leave unset. } + /* + FNXC:PlanReviewApproval 2026-08-04-00:26: + Manual approval after the revision cap is durable evidence that the final REVISE was + accepted. Persist the audited bypass with the hold clear so no consumer can observe only + half of the operator decision and enqueue another Plan Review. + */ + let approvedWorkflowStepResults: Task["workflowStepResults"] | undefined; + if (task.awaitingApprovalReason === "plan-review-replan-cap") { + const results = [...(task.workflowStepResults ?? [])]; + let reviewIndex = -1; + for (let index = results.length - 1; index >= 0; index -= 1) { + const result = results[index]; + if ( + result.workflowStepId === PLAN_REVIEW_GROUP_ID + && (result.status === "failed" || result.status === "advisory_failure") + && result.verdict === "REVISE" + ) { + reviewIndex = index; + break; + } + } + if (reviewIndex === -1) { + throw conflict("Cannot approve exhausted Plan Review: no failed REVISE result is available to override"); + } + + const prior = results[reviewIndex]; + const bypassed = { + ...prior, + status: "skipped" as const, + bypassedBy: "dashboard-operator", + bypassedAt: new Date().toISOString(), + bypassReason: "Approved after Plan Review did not converge", + bypassedFromStatus: prior.status, + bypassedFromVerdict: prior.verdict, + }; + delete bypassed.verdict; + results[reviewIndex] = bypassed; + approvedWorkflowStepResults = results; + } + + await scopedStore.logEntry(task.id, "Plan approved by user"); + // Move to todo and clear status const reboundColumn = await resolveReboundColumnForTask(scopedStore, task.id); await scopedStore.moveTask(task.id, reboundColumn); @@ -4075,6 +4131,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork const updated = await scopedStore.updateTask(task.id, { status: null, approvedPlanFingerprint: approvedPlanFingerprint ?? null, + ...(approvedWorkflowStepResults ? { workflowStepResults: approvedWorkflowStepResults } : {}), }); res.json(updated); diff --git a/packages/engine/src/__tests__/plan-approval-hold-invariant.test.ts b/packages/engine/src/__tests__/plan-approval-hold-invariant.test.ts index bf535281ba..422a6535fa 100644 --- a/packages/engine/src/__tests__/plan-approval-hold-invariant.test.ts +++ b/packages/engine/src/__tests__/plan-approval-hold-invariant.test.ts @@ -65,6 +65,7 @@ ADDED IN REVIEW ROUND 1 (PR #2491), because correctly HOLDING a card is not free [describe #5] */ import { beforeEach, describe, expect, it, vi } from "vitest"; +import { EventEmitter } from "node:events"; import type { Task, TaskStore, WorkflowIr, WorkflowWorkItem } from "@fusion/core"; import { AWAITING_APPROVAL_PAUSE_REASON, PLAN_REVIEW_GROUP_ID } from "@fusion/core"; @@ -78,6 +79,8 @@ import { PARKED_CONTINUATION_DEFER_MS, resolveParkedContinuationDeferral, resolvePlanningContinuationCandidate, + wakeApprovedPlanningContinuations, + InProcessRuntime, type DuePlanningContinuationDrainDeps, } from "../runtimes/in-process-runtime.js"; import { schedulerLog } from "../logger.js"; @@ -495,6 +498,81 @@ describe("#4 an operator-parked item leaves the due window instead of starving t }); }); +describe("#4b an approval decision removes the human-wait delay", () => { + it("clears retryAfter on runnable planning continuations and kicks the drain", async () => { + const transition = vi.fn().mockResolvedValue(undefined); + const kick = vi.fn(); + const retryAfter = new Date(Date.now() + PARKED_CONTINUATION_DEFER_MS).toISOString(); + + await expect(wakeApprovedPlanningContinuations({ + taskId: "FN-1", + list: async () => [ + dueItem({ retryAfter }), + dueItem({ id: "capacity", waitReason: "capacity", retryAfter }), + ], + transition, + kick, + warn: vi.fn(), + })).resolves.toBe(1); + + expect(transition).toHaveBeenCalledWith("wi-1", "runnable", { + expectedState: "runnable", + retryAfter: null, + }); + expect(transition).not.toHaveBeenCalledWith("capacity", expect.anything(), expect.anything()); + expect(kick).toHaveBeenCalledOnce(); + }); + + it("keeps releasing after one transition fails and always kicks the drain", async () => { + const retryAfter = new Date(Date.now() + PARKED_CONTINUATION_DEFER_MS).toISOString(); + const transition = vi.fn() + .mockRejectedValueOnce(new Error("lost CAS")) + .mockResolvedValueOnce(undefined); + const warn = vi.fn(); + const kick = vi.fn(); + + await expect(wakeApprovedPlanningContinuations({ + taskId: "FN-1", + list: async () => [dueItem({ id: "first", retryAfter }), dueItem({ id: "second", retryAfter })], + transition, + kick, + warn, + })).resolves.toBe(1); + + expect(transition).toHaveBeenCalledTimes(2); + expect(warn).toHaveBeenCalledWith(expect.stringContaining("first")); + expect(kick).toHaveBeenCalledOnce(); + }); + + it("wires an approval task update through the runtime to the deferred continuation", async () => { + const retryAfter = new Date(Date.now() + PARKED_CONTINUATION_DEFER_MS).toISOString(); + const transitionWorkflowWorkItem = vi.fn().mockResolvedValue(undefined); + const store = Object.assign(new EventEmitter(), { + listWorkflowWorkItemsForTask: vi.fn().mockResolvedValue([dueItem({ retryAfter })]), + transitionWorkflowWorkItem, + }); + const runtime = new InProcessRuntime({ + projectId: "test-project", + projectName: "Test", + workingDirectory: "/test/project", + isolationMode: "in-process", + }, {} as never); + (runtime as any).taskStore = store; + const kick = vi.spyOn(runtime as any, "kickWorkflowContinuationProcessor").mockImplementation(() => undefined); + (runtime as any).setupEventForwarding(); + + store.emit("task:updated", task({ status: "awaiting-approval" })); + store.emit("task:updated", task({ status: null, approvedPlanFingerprint: "approved" })); + + await vi.waitFor(() => expect(transitionWorkflowWorkItem).toHaveBeenCalledWith( + "wi-1", + "runnable", + { expectedState: "runnable", retryAfter: null }, + )); + expect(kick).toHaveBeenCalledOnce(); + }); +}); + // ───────────────────────────────────────────────────────────────────────────── // #5 — the drain PASS itself: the deferral is applied, and it is a compare-and-set // ───────────────────────────────────────────────────────────────────────────── diff --git a/packages/engine/src/__tests__/pre-release-plan-review.test.ts b/packages/engine/src/__tests__/pre-release-plan-review.test.ts index 458c772205..0732e9cae1 100644 --- a/packages/engine/src/__tests__/pre-release-plan-review.test.ts +++ b/packages/engine/src/__tests__/pre-release-plan-review.test.ts @@ -73,6 +73,29 @@ describe("pre-release Plan Review readiness", () => { await expect(isUnplannedForExecution(store, task, workflow())).resolves.toBe(true); }); + it("treats an operator-bypassed Plan Review as satisfied without another continuation", async () => { + const task = { + id: "T-HUMAN", + column: "todo", + enabledWorkflowSteps: ["plan-review"], + workflowStepResults: [{ + workflowStepId: "plan-review", + workflowStepName: "Plan Review", + phase: "pre-merge", + source: "optional-group", + status: "skipped", + bypassedBy: "operator", + bypassedAt: "2026-08-03T23:53:04.539Z", + bypassReason: "Approved after Plan Review did not converge", + bypassedFromStatus: "failed", + bypassedFromVerdict: "REVISE", + }], + } as any; + const store = { listWorkflowWorkItemsForTask: async () => [] } as any; + + await expect(isUnplannedForExecution(store, task, workflow())).resolves.toBe(false); + }); + it("does not filter active continuations to task kind", async () => { const task = { id: "T-5", column: "todo" } as any; const store = { diff --git a/packages/engine/src/__tests__/scheduler-planning-finished-wake.test.ts b/packages/engine/src/__tests__/scheduler-planning-finished-wake.test.ts index 447ecc0c55..fef9a1907d 100644 --- a/packages/engine/src/__tests__/scheduler-planning-finished-wake.test.ts +++ b/packages/engine/src/__tests__/scheduler-planning-finished-wake.test.ts @@ -174,3 +174,67 @@ describe("Scheduler wakes on the planning -> dispatchable transition", () => { expect(schedule).not.toHaveBeenCalled(); }); }); + +describe("Scheduler wakes on the approval-held -> dispatchable transition", () => { + it("schedules immediately when plan approval clears in a hold column", async () => { + const { emit, schedule } = createScheduler(); + + emit("task:updated", createTask({ status: "awaiting-approval" })); + expect(schedule).not.toHaveBeenCalled(); + + emit("task:updated", createTask({ status: null })); + await flushAsyncHandlers(); + + expect(schedule).toHaveBeenCalledTimes(1); + }); + + it("uses the durable approval fingerprint when this process missed the hold event", async () => { + const { emit, schedule } = createScheduler(); + + emit("task:updated", createTask({ + status: null, + approvedPlanFingerprint: "approved-plan", + })); + await flushAsyncHandlers(); + + expect(schedule).toHaveBeenCalledTimes(1); + }); + + it("uses audited Plan Review evidence when approval could not fingerprint PROMPT.md", async () => { + const { emit, schedule } = createScheduler(); + + emit("task:updated", createTask({ + status: null, + approvedPlanFingerprint: undefined, + workflowStepResults: [{ + workflowStepId: "plan-review", + workflowStepName: "Plan Review", + status: "skipped", + bypassedBy: "dashboard-operator", + bypassedAt: "2026-08-04T00:26:00.000Z", + bypassReason: "Approved after Plan Review did not converge", + bypassedFromStatus: "failed", + bypassedFromVerdict: "REVISE", + }], + })); + await flushAsyncHandlers(); + + expect(schedule).toHaveBeenCalledTimes(1); + }); + + it("does not wake when approval remains held or clears into a pause/non-hold lane", async () => { + for (const terminal of [ + { status: "awaiting-approval" }, + { status: null, paused: true }, + { status: null, userPaused: true }, + { status: null, column: "in-review" }, + ]) { + const { emit, schedule } = createScheduler(); + emit("task:updated", createTask({ status: "awaiting-approval" })); + emit("task:updated", createTask(terminal)); + await flushAsyncHandlers(); + expect(schedule, JSON.stringify(terminal)).not.toHaveBeenCalled(); + } + }); + +}); diff --git a/packages/engine/src/execution/hold-release.ts b/packages/engine/src/execution/hold-release.ts index 66cfc7e1ea..2a39c66ef3 100644 --- a/packages/engine/src/execution/hold-release.ts +++ b/packages/engine/src/execution/hold-release.ts @@ -51,6 +51,7 @@ import { isWorkflowOptionalGroupEnabled, resolveEffectiveAutoMerge, isTaskBlockedOnApproval, + isPlanReviewSatisfied, type TaskStore, type Task, type WorkflowIr, @@ -212,10 +213,8 @@ export async function isUnplannedForExecution(store: TaskStore, task: Task, ir: if (preReleaseReview && preReleaseReviewEnabled && preReleaseReview.column === task.column) { // Compatibility for tasks planned before durable continuations existed and // for narrow store adapters that expose only the legacy review result. - const legacyPassed = task.workflowStepResults?.some( - (result) => result.workflowStepId === PLAN_REVIEW_GROUP_ID && result.status === "passed", - ); - if (!legacyPassed) { + const legacySatisfied = task.workflowStepResults?.some(isPlanReviewSatisfied); + if (!legacySatisfied) { if (typeof store.listWorkflowWorkItemsForTask !== "function") return true; // FNXC:StrandedHoldContinuation 2026-07-26-15:45: // FN-8592 defines graph idleness over every active continuation kind; diff --git a/packages/engine/src/plan-review-continuation.ts b/packages/engine/src/plan-review-continuation.ts index 09c9b91ef1..0ad08eb75d 100644 --- a/packages/engine/src/plan-review-continuation.ts +++ b/packages/engine/src/plan-review-continuation.ts @@ -2,8 +2,8 @@ import { ACTIVE_WORKFLOW_WORK_ITEM_STATES, computeWorkflowIrPin, isTaskBlockedOnApproval, + isPlanReviewSatisfied, isUnplannedSeedPrompt, - PLAN_REVIEW_GROUP_ID, type Task, type TaskStore, type WorkflowIr, @@ -107,7 +107,7 @@ export function evaluateStrandedHoldContinuation(input: { const review = resolvePreReleasePlanReviewNode(input.ir); if (!review || review.column !== input.task.column) return { stranded: false, candidate: false, reason: "no-pre-release-review" }; if (input.continuations.some((item) => ACTIVE_WORKFLOW_WORK_ITEM_STATES.includes(item.state))) return { stranded: false, candidate: false, reason: "active-continuation" }; - if (input.stepResults?.some((result) => result.workflowStepId === PLAN_REVIEW_GROUP_ID && result.status === "passed")) return { stranded: false, candidate: false, reason: "plan-review-passed" }; + if (input.stepResults?.some(isPlanReviewSatisfied)) return { stranded: false, candidate: false, reason: "plan-review-passed" }; if (input.promptContent === null) return { stranded: false, candidate: false, reason: "prompt-missing" }; if (isUnplannedSeedPrompt(input.promptContent, input.task.id, input.task.title, input.task.description)) return { stranded: false, candidate: false, reason: "seed-prompt" }; if (input.task.status === "planning" || input.task.status === "needs-replan") return { stranded: false, candidate: false, reason: "triage-owned" }; diff --git a/packages/engine/src/runtimes/in-process-runtime.ts b/packages/engine/src/runtimes/in-process-runtime.ts index 45dfa18b84..c50046ea96 100644 --- a/packages/engine/src/runtimes/in-process-runtime.ts +++ b/packages/engine/src/runtimes/in-process-runtime.ts @@ -23,6 +23,7 @@ import { AsyncCentralClaimStore, ChatStore, isEphemeralAgent, + isPlanReviewSatisfied, isTaskBlockedOnApproval, resolveWorkflowIrForTask, resolveTaskLifecycleColumns, @@ -295,6 +296,43 @@ export function resolveParkedContinuationDeferral( }; } +/* +FNXC:PlanReviewApproval 2026-08-04-00:26: +An operator decision must remove the one-minute human-wait deferral immediately. Clear every +runnable planning continuation for the task, preserve CAS ownership, and wake the drain even when +inspection fails so the normal classifier remains authoritative. +*/ +export async function wakeApprovedPlanningContinuations(deps: { + taskId: string; + list: (taskId: string) => Promise; + transition: ( + itemId: string, + state: WorkflowWorkItemState, + patch: { expectedState: WorkflowWorkItemState; retryAfter: null }, + ) => Promise; + kick: () => void; + warn: (message: string) => void; +}): Promise { + let released = 0; + try { + const items = await deps.list(deps.taskId); + for (const item of items) { + if (item.state !== "runnable" || item.waitReason !== "planning" || !item.retryAfter) continue; + try { + await deps.transition(item.id, item.state, { expectedState: item.state, retryAfter: null }); + released += 1; + } catch (error) { + deps.warn(`Failed to clear approval deferral for workflow work item ${item.id}: ${error instanceof Error ? error.message : String(error)}`); + } + } + } catch (error) { + deps.warn(`Failed to inspect approval-deferred workflow work for ${deps.taskId}: ${error instanceof Error ? error.message : String(error)}`); + } finally { + deps.kick(); + } + return released; +} + /** The FIFO due-poll batch size. Named because the starvation the deferral above * prevents is a property of this bound, so the two belong in one place. */ export const DUE_PLANNING_CONTINUATION_BATCH_LIMIT = 20; @@ -743,6 +781,13 @@ export class InProcessRuntime private workflowContinuationTimer?: ReturnType; private workflowContinuationDrainActive = false; private workflowContinuationDrainSince = 0; + /* + FNXC:PlanReviewApproval 2026-08-04-00:26: + Track the event edge and the durable approval marker. The marker covers engine restarts and + cross-process updates that did not deliver the earlier awaiting-approval event. + */ + private approvalHeldTaskIds = new Set(); + private approvalReleasedTaskIds = new Set(); private messageStore?: MessageStore; /** FNXC:TaskDeleteNotice 2026-07-26-16:10: identity-guarded teardown for the delete-notice mailbox seam. */ private unregisterTaskDeleteNoticeMailbox?: () => void; @@ -2692,6 +2737,30 @@ export class InProcessRuntime // Forward task:updated events this.taskStore.on("task:updated", (task: Task) => { this.recordActivity(); + if (task.status === "awaiting-approval") { + this.approvalHeldTaskIds.add(task.id); + this.approvalReleasedTaskIds.delete(task.id); + } else if ( + !task.status + && !task.paused + && !task.userPaused + && ( + this.approvalHeldTaskIds.delete(task.id) + || ( + (Boolean(task.approvedPlanFingerprint) || task.workflowStepResults?.some(isPlanReviewSatisfied) === true) + && !this.approvalReleasedTaskIds.has(task.id) + ) + ) + ) { + this.approvalReleasedTaskIds.add(task.id); + void wakeApprovedPlanningContinuations({ + taskId: task.id, + list: (taskId) => this.taskStore.listWorkflowWorkItemsForTask(taskId), + transition: (itemId, state, patch) => this.taskStore.transitionWorkflowWorkItem(itemId, state, patch), + kick: () => this.kickWorkflowContinuationProcessor(), + warn: (message) => runtimeLog.warn(message), + }); + } this.emit("task:updated", task); }); @@ -2702,6 +2771,8 @@ export class InProcessRuntime */ this.taskStore.on("task:deleted", (task: Task, meta?: { githubIssueAction?: GithubIssueAction; observed?: boolean; outboxEventId?: string }) => { this.recordActivity(); + this.approvalHeldTaskIds.delete(task.id); + this.approvalReleasedTaskIds.delete(task.id); this.emit("task:deleted", task, meta); }); diff --git a/packages/engine/src/scheduler.ts b/packages/engine/src/scheduler.ts index 658dd5dc52..76b8b8f088 100644 --- a/packages/engine/src/scheduler.ts +++ b/packages/engine/src/scheduler.ts @@ -4,6 +4,7 @@ import { compareTasksByPriorityThenAgeAndId, HIGH_FANOUT_BLOCKER_TODO_THRESHOLD, nonExecutableDuplicateRedirectReason, + isPlanReviewSatisfied, type TaskStore, type Task, type MissionStore, @@ -917,6 +918,13 @@ export class Scheduler { * task:moved, so this is the only signal that the card just became executable. */ private planningTaskIds = new Set(); + /* + FNXC:PlanReviewApproval 2026-08-04-00:26: + Wake dispatch on the observed hold edge or the durable approval marker. The latter covers a + restart or cross-process update that did not deliver the earlier awaiting-approval event. + */ + private approvalHeldTaskIds = new Set(); + private approvalReleasedTaskIds = new Set(); /** Tracks mission-linked tasks observed with status=failed before moveTask clears status/error. */ private failedTaskIds = new Set(); /** Tracks tasks blocked by unavailable-node policy to deduplicate block log entries. */ @@ -1339,6 +1347,37 @@ export class Scheduler { })(); } + if (task.status === "awaiting-approval") { + this.approvalHeldTaskIds.add(task.id); + this.approvalReleasedTaskIds.delete(task.id); + } else if ( + !task.status + && !task.paused + && !task.userPaused + && ( + this.approvalHeldTaskIds.delete(task.id) + || ( + (Boolean(task.approvedPlanFingerprint) || task.workflowStepResults?.some(isPlanReviewSatisfied) === true) + && !this.approvalReleasedTaskIds.has(task.id) + ) + ) + ) { + this.approvalReleasedTaskIds.add(task.id); + void (async () => { + const approvalParked = await resolveTaskParkedColumns(this.store, task.id); + if ( + this.running + && !task.status + && !task.paused + && !task.userPaused + && approvalParked.wake.has(task.column) + ) { + schedulerLog.log(`Task ${task.id} plan approval cleared — triggering scheduling`); + void this.schedule(); + } + })(); + } + if (!this.options.prMonitor) return; // DELIBERATE-LITERAL — runtime bridges drop lanes; never replace this unknown fallback with the sync resolver. if (eventLanes ? task.column !== eventLanes.review : task.column !== "in-review") return; @@ -1364,6 +1403,8 @@ export class Scheduler { // FNXC:CodingIdeasWorkflow 2026-07-25-13:10: drop planning tracking with the other per-task // sets so a deleted-mid-planning id cannot leak or fire a stale wake if the id is reused. this.planningTaskIds.delete(task.id); + this.approvalHeldTaskIds.delete(task.id); + this.approvalReleasedTaskIds.delete(task.id); this.failedTaskIds.delete(task.id); this.recentEngineTodoRequeues.delete(task.id); this.wasNodeDispatchValidationBlocked.delete(task.id); diff --git a/packages/engine/src/triage.ts b/packages/engine/src/triage.ts index ee04a82de8..d2a0ec487f 100644 --- a/packages/engine/src/triage.ts +++ b/packages/engine/src/triage.ts @@ -42,6 +42,7 @@ import { workflowHasColumn, getStepParser, computePlanApprovalFingerprint, + isPlanReviewSatisfied, extractIntentSignature, findNearDuplicates, isNearDuplicateCanonicalInactive, resolveColumnFlags, @@ -1259,11 +1260,13 @@ export class TriageProcessor { return evicted; } - /** True when Plan Review already recorded a passed verdict on this task. */ - private hasPassedPlanReview(task: Pick): boolean { - return task.workflowStepResults?.some( - (result) => result.workflowStepId === PLAN_REVIEW_GROUP_ID && result.status === "passed", - ) === true; + /* + FNXC:PlanReviewApproval 2026-08-04-00:26: + Recovery treats an audited operator acceptance as terminal Plan Review evidence, without + fabricating a reviewer pass or allowing an unaudited skip to release the task. + */ + private hasSatisfiedPlanReview(task: Pick): boolean { + return task.workflowStepResults?.some(isPlanReviewSatisfied) === true; } /** @@ -1279,7 +1282,7 @@ export class TriageProcessor { async recoverApprovedTask(task: Task): Promise { const recoverableStatus = task.status === "planning" - || (task.status == null && this.hasPassedPlanReview(task)); + || (task.status == null && this.hasSatisfiedPlanReview(task)); /* FNXC:WorkflowLifecycleColumns 2026-07-29-09:05 (U11): the INTAKE lane, not the literal. Converting only the `todo` sites left this one rejecting every card whose workflow renames its planner column, so the release below was diff --git a/packages/engine/src/workflows/workflow-graph-executor.ts b/packages/engine/src/workflows/workflow-graph-executor.ts index fca0e0ec43..24444793c8 100644 --- a/packages/engine/src/workflows/workflow-graph-executor.ts +++ b/packages/engine/src/workflows/workflow-graph-executor.ts @@ -9,7 +9,7 @@ import type { WorkflowNodeExtensionResult, WorkflowStepResult, } from "@fusion/core"; -import { BUILTIN_CODING_WORKFLOW_IR, PLAN_REVIEW_GROUP_ID, WorkflowIrError, getWorkflowExtensionRegistry, resolveMaxReworkCycles, isExperimentalFeatureEnabled, GRAPH_NATIVE_POST_MERGE_FLAG, isCompletionSummaryNode, classifyReviewLease, isWorkflowOptionalGroupEnabled } from "@fusion/core"; +import { BUILTIN_CODING_WORKFLOW_IR, PLAN_REVIEW_GROUP_ID, WorkflowIrError, getWorkflowExtensionRegistry, resolveMaxReworkCycles, isExperimentalFeatureEnabled, GRAPH_NATIVE_POST_MERGE_FLAG, isCompletionSummaryNode, classifyReviewLease, isWorkflowOptionalGroupEnabled, isPlanReviewSatisfied } from "@fusion/core"; import { isNonPlanDefectPlanReviewFailure } from "../errors/transient-error-detector.js"; import { isSessionContentionError } from "../errors/transient-error-patterns.js"; import { isRequiredArtifactReadFailedValue, parseRequiredArtifactMissingValue } from "../execution/required-workflow-artifacts.js"; @@ -819,12 +819,10 @@ export class WorkflowGraphExecutor { */ if ( node.id === PLAN_REVIEW_GROUP_ID - && task.workflowStepResults?.some( - (result) => result.workflowStepId === PLAN_REVIEW_GROUP_ID && result.status === "passed", - ) + && task.workflowStepResults?.some(isPlanReviewSatisfied) ) { context[`node:${node.id}:outcome`] = "success"; - this.deps.logTaskEntry?.("[pre-merge] Workflow step already passed: Plan Review"); + this.deps.logTaskEntry?.("[pre-merge] Workflow step already satisfied: Plan Review"); return await traverseChildren(node, { outcome: "success", value: "already-passed" }); } const repairedPlanReview = node.id === PLAN_REVIEW_GROUP_ID