diff --git a/.changeset/fn-175-review-gated-workflow.md b/.changeset/fn-175-review-gated-workflow.md new file mode 100644 index 0000000000..4e2fa7a520 --- /dev/null +++ b/.changeset/fn-175-review-gated-workflow.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": minor +--- + +summary: Add an opt-in coding workflow with review-owned verification gates. +category: feature +dev: Review rejection appends structured remediation steps without reopening completed implementation work. diff --git a/docs/architecture.md b/docs/architecture.md index 72d20043a0..a028ee372d 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -2479,3 +2479,7 @@ Workspace review and landing share one canonical base-to-task-branch binary-diff ### Review convergence invariant Automatic review remediation archives rather than erases failure history on the bounce, graph-failure resume, and remediation-owned replan paths; explicit operator retry clears it. A carrier is non-blocking and cannot satisfy a gate without complete arbitration provenance. Every automatic remediation stop point uses the convergence ladder before a human park. A dispute remains open until adjudicated, preserving its blocking power, implementer obligation, reviewer visibility, and supersession eligibility. Arbitration releases one fenced adjudicated gate only and refuses partial rulings with binding findings. Review-convergence audit emission is bounded best-effort telemetry: it cannot alter, delay past its bound, or abort a ladder, arbitration, or dispute lifecycle outcome. + +### Review-gated remediation safety + +The review-gated coding workflow preserves implementation steps across review rejection. Its replay authority is appended remediation provenance, not step-name matching. Zero-diff finalization recognizes legacy verification names, structurally marked remediation steps, and required gate results so a missing or failed Verification cannot be finalized as a no-op. diff --git a/docs/dashboard-guide.md b/docs/dashboard-guide.md index 5a8a892ae5..44da197768 100644 --- a/docs/dashboard-guide.md +++ b/docs/dashboard-guide.md @@ -2601,3 +2601,7 @@ Settings separates automatic installation from automatic restart. Following an o ### Review finding resolutions The Review tab renders the `dispute-upheld` badge for an adjudicated finding. It is a terminal resolution and cannot be selected for a further revision; it is distinct from the existing Superseded badge. + +### Review-gated task progress + +Cards using the review-gated workflow show implementation progress while work is in progress. In review, their progress block includes Verification, Code Review, and Documentation & Delivery after the implementation steps, separated visually; the running gate supplies the card badge. diff --git a/docs/run-audit.md b/docs/run-audit.md index 6fe8ddb19a..6b2591ea38 100644 --- a/docs/run-audit.md +++ b/docs/run-audit.md @@ -96,3 +96,8 @@ All `recordRunAuditEventWithinTransaction(tx, ...)` calls and the `recordRunAudi ### Review convergence events `task:review-finding-disputed`, `task:review-convergence-escalation`, `task:review-arbitration`, and `task:review-convergence-human-escalation` record review-cycle progression. Their metadata contains only ids, counts, and fixed outcomes; dispute rationales, findings, reviewer feedback, and arbiter output are never recorded. All five emission sites use the FN-9175 bounded best-effort seam, so hostile telemetry cannot alter or block the ladder, arbitration release, or dispute result. + +| Event | Metadata | +| --- | --- | +| `review-remediation-appended` | Task id, gate id, wave, and count only. | +| `review-remediation-parked` | Task id and fixed park outcome only. | diff --git a/docs/workflow-steps.md b/docs/workflow-steps.md index 56d7d6ce95..5525b45faf 100644 --- a/docs/workflow-steps.md +++ b/docs/workflow-steps.md @@ -1036,3 +1036,9 @@ Automatic remediation retains a failed review as a non-blocking `skipped` carrie The next reviewer receives the same-gate attempt history and open findings. An implementer may call `fn_review_dispute(findingId, rationale)`: this records a contested-but-open annotation, so the finding remains blocking and visible. The reviewer must supersede it or rebut it with `rebutsDisputedFindingId`; only a terminal same-gate verdict that does neither marks it `dispute-upheld`. Repeated unchanged review input routes through the shared convergence ladder: one bounded escalation/replan, then arbitration, then a loud human escalation only after the automatic stages are spent. A non-declining stage performs a real re-dispatch and its requester succeeds rather than terminalizing the remediation node. Arbitration may release only its exact fenced failed gate; it cannot release sibling gates, and a split with binding findings remains blocking. + +## Review-gated coding + +`builtin:review-gated-coding` is an opt-in coding workflow. Its task steps contain implementation work only. Verification, Code Review, and Documentation & Delivery run in that order as review-column gates. A failed Verification or Code Review appends `Fix: ` remediation steps to the end of the task list; their durable `remediation` provenance records the gate, finding, affected file, and wave. Step names are never used to classify remediation. + +The workflow allows at most three remediation waves. Missing actionable findings, out-of-scope findings, duplicate-only remediation, and an exhausted wave budget park the task for a human rather than returning it to implementation without work. `parse-steps` uses `preserveRemediationSteps` to stop before replacement writes when live remediation exists, while `implementationOnlySteps` only audits gate-like plan steps and never deletes them. diff --git a/packages/core/src/__tests__/builtin-review-gated-coding-workflow.test.ts b/packages/core/src/__tests__/builtin-review-gated-coding-workflow.test.ts new file mode 100644 index 0000000000..6708b2ed7f --- /dev/null +++ b/packages/core/src/__tests__/builtin-review-gated-coding-workflow.test.ts @@ -0,0 +1,48 @@ +import { describe, expect, it } from "vitest"; +import { resolveRequiredPreMergeStepIds } from "../merge/required-pre-merge-steps.js"; +import { BUILTIN_REVIEW_GATED_CODING_WORKFLOW_IR } from "../workflows/builtin-review-gated-coding-workflow-ir.js"; +import { BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR } from "../workflows/builtin-stepwise-final-review-coding-workflow-ir.js"; +import { getBuiltinWorkflow } from "../workflows/builtin-workflows.js"; +import { parseWorkflowIr, serializeWorkflowIr } from "../workflows/workflow-ir.js"; +import { resolveWorkflowOptionalSteps } from "../workflows/workflow-optional-steps.js"; + +describe("builtin:review-gated-coding", () => { + it("is a selectable validated workflow with review-owned gates", () => { + const workflow = getBuiltinWorkflow("builtin:review-gated-coding"); + expect(workflow?.name).toBe("Coding (review-gated)"); + expect(parseWorkflowIr(serializeWorkflowIr(BUILTIN_REVIEW_GATED_CODING_WORKFLOW_IR))) + .toEqual(BUILTIN_REVIEW_GATED_CODING_WORKFLOW_IR); + + expect(resolveWorkflowOptionalSteps(BUILTIN_REVIEW_GATED_CODING_WORKFLOW_IR)).toEqual([ + { templateId: "plan-review", name: "Plan Review", description: "", phase: "pre-merge", defaultOn: true }, + { templateId: "verification", name: "Verification", description: "", phase: "pre-merge", defaultOn: true }, + { templateId: "code-review", name: "Code Review", description: "", phase: "pre-merge", defaultOn: true }, + { templateId: "documentation-delivery", name: "Documentation & Delivery", description: "", phase: "pre-merge", defaultOn: true }, + { templateId: "post-merge-verification", name: "Post-merge verification", description: "", phase: "post-merge", defaultOn: false }, + ]); + expect(resolveRequiredPreMergeStepIds(BUILTIN_REVIEW_GATED_CODING_WORKFLOW_IR, undefined)) + .toEqual(new Set(["plan-review", "verification", "code-review", "documentation-delivery"])); + }); + + it("routes failures through explicit remediation before replaying verification", () => { + const ir = BUILTIN_REVIEW_GATED_CODING_WORKFLOW_IR; + const parse = ir.nodes.find((node) => node.id === "parse"); + const planReview = ir.nodes.find((node) => node.id === "plan-review"); + const planReviewTemplate = planReview?.config.template as { nodes?: Array<{ config?: Record }> }; + + expect(parse?.config).toMatchObject({ implementationOnlySteps: true, preserveRemediationSteps: true }); + expect(planReviewTemplate.nodes?.[0]?.config).toMatchObject({ requireImplementationOnlySteps: true }); + expect(ir.edges).toEqual(expect.arrayContaining([ + { from: "verification", to: "verification-remediation", condition: "failure" }, + { from: "code-review", to: "code-review-remediation-steps", condition: "failure" }, + { from: "verification-remediation", to: "verification", condition: "success", kind: "rework" }, + { from: "code-review-remediation-steps", to: "verification", condition: "success", kind: "rework" }, + ])); + }); + + it("leaves the default coding IR's plan review contract unchanged", () => { + const planReview = BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR.nodes.find((node) => node.id === "plan-review"); + const template = planReview?.config.template as { nodes?: Array<{ config?: Record }> }; + expect(template.nodes?.[0]?.config?.requireImplementationOnlySteps).toBeUndefined(); + }); +}); diff --git a/packages/core/src/__tests__/no-commits-finalize-guard.test.ts b/packages/core/src/__tests__/no-commits-finalize-guard.test.ts index f779f292ca..01b071c021 100644 --- a/packages/core/src/__tests__/no-commits-finalize-guard.test.ts +++ b/packages/core/src/__tests__/no-commits-finalize-guard.test.ts @@ -135,6 +135,21 @@ describe("evaluateNoCommitsNoOpFinalize", () => { expect(result.reason).toContain("Deploy notes"); }); + it("blocks a skipped remediation step structurally even when its name has no gate word", () => { + expect(evaluateNoCommitsNoOpFinalize({ + noCommitsExpected: true, + steps: [{ name: "Fix: inverted condition", status: "skipped", remediation: { wave: 1, gate: "Code Review", gateStepId: "code-review", detail: "inverted condition" } }], + })).toMatchObject({ blocked: true }); + }); + + it("requires each supplied verification gate to have a passing result", () => { + const task = { noCommitsExpected: false, steps: [{ name: "Implement", status: "done" as const }], workflowStepResults: [] }; + expect(evaluateNoCommitsNoOpFinalize(task, { requiredVerificationStepIds: new Set(["verification"]) })) + .toMatchObject({ blocked: true }); + expect(evaluateNoCommitsNoOpFinalize({ ...task, workflowStepResults: [{ workflowStepId: "verification", status: "passed" }] }, { requiredVerificationStepIds: new Set(["verification"]) })) + .toMatchObject({ blocked: false }); + }); + it("does not block skip-free ordinary tasks (all-done handled by lineage proof)", () => { expect(evaluateNoCommitsNoOpFinalize({ noCommitsExpected: false, diff --git a/packages/core/src/__tests__/remediation-steps.test.ts b/packages/core/src/__tests__/remediation-steps.test.ts new file mode 100644 index 0000000000..5b73232985 --- /dev/null +++ b/packages/core/src/__tests__/remediation-steps.test.ts @@ -0,0 +1,64 @@ +import { describe, expect, it } from "vitest"; +import { + formatRemediationStepName, + hasOpenEquivalentRemediationStep, + remediationWaveCount, + type Task, + type TaskStep, +} from "../index.js"; +import { appendRemediationStepsImpl } from "../task-store/remediation-step-ops.js"; + +const remediation = (detail: string, status: TaskStep["status"] = "pending", wave = 1): TaskStep => ({ + name: formatRemediationStepName({ detail }), + status, + remediation: { wave, gate: "Code Review", gateStepId: "code-review", filePath: "src/example.ts", detail }, +}); + +function fakeStore(steps: TaskStep[]) { + const task = { id: "FN-175", steps } as Task; + return { + task, + store: { + async updateTaskAtomic(_id: string, update: (current: Task) => { steps?: TaskStep[] } | null) { + const patch = await update(task); + if (patch?.steps) task.steps = patch.steps; + return task; + }, + }, + }; +} + +describe("review remediation steps", () => { + it("appends without rewriting the existing prefix", async () => { + const prefix = [{ name: "Implementation", status: "done" as const, dependsOn: [] }]; + const { store, task } = fakeStore(prefix); + const result = await appendRemediationStepsImpl(store as never, task.id, [remediation("missing undefined case")]); + expect(task.steps.slice(0, prefix.length)).toEqual(prefix); + expect(result).toMatchObject({ appendedCount: 1, wave: 1 }); + }); + + it("deduplicates only open equivalent remediation", async () => { + const { store, task } = fakeStore([remediation("missing undefined case")]); + expect((await appendRemediationStepsImpl(store as never, task.id, [remediation("missing undefined case")])).appendedCount).toBe(0); + task.steps[0]!.status = "done"; + expect((await appendRemediationStepsImpl(store as never, task.id, [remediation("missing undefined case")])).appendedCount).toBe(1); + }); + + it("counts durable waves and formats collision-free names", () => { + expect(remediationWaveCount([])).toBe(0); + expect(remediationWaveCount([remediation("one", "done", 1), remediation("three", "pending", 3)])).toBe(3); + for (const detail of ["inverted condition", "undefined case", "resolver error"]) { + const name = formatRemediationStepName({ detail }); + expect(name).toMatch(/^Fix: /); + expect(name).not.toMatch(/(^|[^a-z])(testing|verification|documentation|delivery)([^a-z]|$)/i); + expect(name).not.toMatch(/test|verif|qa|review/i); + } + }); + + it("recognizes open equivalence structurally", () => { + const existing = remediation("missing undefined case"); + expect(hasOpenEquivalentRemediationStep([existing], remediation("missing undefined case"))).toBe(true); + existing.status = "done"; + expect(hasOpenEquivalentRemediationStep([existing], remediation("missing undefined case"))).toBe(false); + }); +}); diff --git a/packages/core/src/index.gate.ts b/packages/core/src/index.gate.ts index 35c0b5c388..e0441f1b61 100644 --- a/packages/core/src/index.gate.ts +++ b/packages/core/src/index.gate.ts @@ -835,6 +835,15 @@ export { type NoOpCompletionMarker, type NoOpCompletionMarkerKind, } from "./merge/no-op-completion-marker.js"; +export { + formatRemediationStepName, + isRemediationStep, + remediationWaveCount, + hasOpenEquivalentRemediationStep, + remediationDeclaredFiles, +} from "./tasks/remediation-steps.js"; +export type { RemediationStepInput } from "./tasks/remediation-steps.js"; +export type { AppendRemediationStepsOptions, AppendRemediationStepsResult } from "./task-store/remediation-step-ops.js"; export { evaluateNoCommitsNoOpFinalize } from "./merge/no-commits-finalize-guard.js"; export type { NoCommitsNoOpFinalizeEvaluation } from "./merge/no-commits-finalize-guard.js"; export { diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index d287a3c4c9..bb0a4356ca 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -360,6 +360,7 @@ export { export type { WorkflowPromptDefault, WorkflowPromptOverrides } from "./workflows/workflow-prompt-overrides.js"; export { BUILTIN_STEPWISE_CODING_WORKFLOW_IR } from "./workflows/builtin-stepwise-coding-workflow-ir.js"; export { BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR } from "./workflows/builtin-stepwise-final-review-coding-workflow-ir.js"; +export { BUILTIN_REVIEW_GATED_CODING_WORKFLOW_IR } from "./workflows/builtin-review-gated-coding-workflow-ir.js"; export { BUILTIN_PR_WORKFLOW_IR } from "./workflows/builtin-pr-workflow-ir.js"; export { BUILTIN_LEAD_GENERATION_WORKFLOW_IR } from "./workflows/builtin-lead-generation-workflow-ir.js"; export { @@ -982,6 +983,15 @@ export { type NoOpCompletionMarker, type NoOpCompletionMarkerKind, } from "./merge/no-op-completion-marker.js"; +export { + formatRemediationStepName, + isRemediationStep, + remediationWaveCount, + hasOpenEquivalentRemediationStep, + remediationDeclaredFiles, +} from "./tasks/remediation-steps.js"; +export type { RemediationStepInput } from "./tasks/remediation-steps.js"; +export type { AppendRemediationStepsOptions, AppendRemediationStepsResult } from "./task-store/remediation-step-ops.js"; export { evaluateNoCommitsNoOpFinalize } from "./merge/no-commits-finalize-guard.js"; export type { NoCommitsNoOpFinalizeEvaluation } from "./merge/no-commits-finalize-guard.js"; export { evaluateCompletedPromotionFailureProvenance, CLEAN_COMPLETION_MARKERS } from "./merge/completed-promotion-failure-provenance.js"; diff --git a/packages/core/src/merge/no-commits-finalize-guard.ts b/packages/core/src/merge/no-commits-finalize-guard.ts index d6e7798b24..4065c34da0 100644 --- a/packages/core/src/merge/no-commits-finalize-guard.ts +++ b/packages/core/src/merge/no-commits-finalize-guard.ts @@ -1,3 +1,4 @@ +import { isRemediationStep } from "../tasks/remediation-steps.js"; import type { Task } from "../types.js"; export interface NoCommitsNoOpFinalizeEvaluation { @@ -20,12 +21,19 @@ export interface NoCommitsNoOpFinalizeEvaluation { const VERIFICATION_STEP_NAME = /test|verif|qa|review/i; export function evaluateNoCommitsNoOpFinalize( - task: Pick, + task: Pick, + options: { requiredVerificationStepIds?: ReadonlySet } = {}, ): NoCommitsNoOpFinalizeEvaluation { const steps = task.steps ?? []; const doneCount = steps.filter((step) => step.status === "done").length; const incompleteCount = steps.length - doneCount; const noCommitsExpected = task.noCommitsExpected === true; + const missingRequiredGate = [...(options.requiredVerificationStepIds ?? [])].find((id) => + !task.workflowStepResults?.some((result) => result.workflowStepId === id && result.status === "passed"), + ); + if (missingRequiredGate) { + return { blocked: true, reason: `required verification gate '${missingRequiredGate}' has no passing result`, doneCount, incompleteCount }; + } const skippedSteps = steps.filter((step) => step.status === "skipped"); const hasCompletedVerification = steps.some((step) => @@ -35,7 +43,7 @@ export function evaluateNoCommitsNoOpFinalize( // FN-8141: skipped step + empty diff. Applies to ALL tasks regardless of `noCommitsExpected`. if (skippedSteps.length > 0) { const verificationSkipped = skippedSteps.filter((step) => - VERIFICATION_STEP_NAME.test(step.name ?? ""), + VERIFICATION_STEP_NAME.test(step.name ?? "") || isRemediationStep(step), ); // A skipped verification/QA/review step over an empty diff blocks unconditionally: diff --git a/packages/core/src/store.ts b/packages/core/src/store.ts index d28d629170..cddf0acfd1 100644 --- a/packages/core/src/store.ts +++ b/packages/core/src/store.ts @@ -11,7 +11,7 @@ import { evaluateSpecDrift, hasPriorLockDivergence, type DriftReport } from "./p import * as schema from "./postgres/schema/index.js"; import { type FSWatcher } from "node:fs"; import { readFile } from "node:fs/promises"; -import type { Task, TaskDetail, TaskCreateInput, TaskAttachment, AgentLogEntry, BoardConfig, Column, ColumnId, CheckoutClaimPrecondition, MergeResult, Settings, GlobalSettings, ProjectSettings, ActivityLogEntry, ActivityEventType, TaskDocument, TaskDocumentRevision, TaskDocumentCreateInput, ArchivedTaskDocumentAdditionInput, ArchivedTaskDocumentAdditionResult, TaskDocumentWithTask, Artifact, ArtifactCreateInput, ArtifactType, ArtifactWithTask, InboxTask, TaskLogEntry, RunMutationContext, RunAuditEvent, RunAuditEventInput, RunAuditEventFilter, ArchivedTaskEntry, ArchiveAgentLogMode, TaskPriority, WorkflowStepTemplate, Agent, AutostashOrphanRecord, TaskCommitAssociation, CommitAssociationDiffBackfillReport, GithubIssueAction, MergeQueueEntry, MergeQueueEnqueueOptions, MergeQueueAcquireOptions, MergeQueueReleaseOutcome, HandoffToReviewOptions, GoalCitation, GoalCitationFilter, GoalCitationInput, GoalCitationSurface, BranchGroup, BranchGroupCreateInput, BranchGroupUpdate, TaskBranchAssignmentMode, MergeRequestRecord, MergeRequestState, MergeRequestWorkflowProjectionOptions, CompletionHandoffMarker, WorkflowWorkItem, WorkflowWorkItemDueFilter, WorkflowWorkItemKind, WorkflowWorkItemState, WorkflowWorkItemTransitionPatch, WorkflowWorkItemUpsertInput, PrEntity, PrEntityCreateInput, PrEntityUpdate, PrThreadState, PrThreadOutcome, PluginActivation, PluginActivationInput } from "./types.js"; +import type { Task, TaskDetail, TaskCreateInput, TaskAttachment, AgentLogEntry, BoardConfig, Column, ColumnId, CheckoutClaimPrecondition, MergeResult, Settings, GlobalSettings, ProjectSettings, ActivityLogEntry, ActivityEventType, TaskDocument, TaskDocumentRevision, TaskDocumentCreateInput, ArchivedTaskDocumentAdditionInput, ArchivedTaskDocumentAdditionResult, TaskDocumentWithTask, Artifact, ArtifactCreateInput, ArtifactType, ArtifactWithTask, InboxTask, TaskLogEntry, RunMutationContext, RunAuditEvent, RunAuditEventInput, RunAuditEventFilter, ArchivedTaskEntry, ArchiveAgentLogMode, TaskPriority, WorkflowStepTemplate, Agent, AutostashOrphanRecord, TaskCommitAssociation, CommitAssociationDiffBackfillReport, GithubIssueAction, MergeQueueEntry, MergeQueueEnqueueOptions, MergeQueueAcquireOptions, MergeQueueReleaseOutcome, HandoffToReviewOptions, GoalCitation, GoalCitationFilter, GoalCitationInput, GoalCitationSurface, BranchGroup, BranchGroupCreateInput, BranchGroupUpdate, TaskBranchAssignmentMode, MergeRequestRecord, MergeRequestState, MergeRequestWorkflowProjectionOptions, CompletionHandoffMarker, WorkflowWorkItem, WorkflowWorkItemDueFilter, WorkflowWorkItemKind, WorkflowWorkItemState, WorkflowWorkItemTransitionPatch, WorkflowWorkItemUpsertInput, PrEntity, PrEntityCreateInput, PrEntityUpdate, PrThreadState, PrThreadOutcome, PluginActivation, PluginActivationInput, TaskStep } from "./types.js"; /* FNXC:SpecLock 2026-08-09-21:01: @@ -149,6 +149,7 @@ import { createWorkflowStepImpl, updateWorkflowStepImpl, updateWorkflowDefinitio import { initImpl, setupActivityLogListenersImpl, reconcileOrphanedTaskDirsImpl, watchImpl, migrateAgentLogEntriesImpl, migrateMovedSettingsImpl, recoverStaleTransitionPendingImpl, migrateLegacyWorkflowStepsImpl, emitTaskLifecycleEventSafelyImpl } from "./task-store/lifecycle-ops.js"; import { TaskDeletedOutboxConsumer } from "./task-store/task-deleted-outbox-consumer.js"; import { updateStepImpl, startStepImpl, acquireMergeQueueLeaseImpl, mergeTaskImpl } from "./task-store/merge-queue-ops.js"; +import { appendRemediationStepsImpl, type AppendRemediationStepsOptions, type AppendRemediationStepsResult } from "./task-store/remediation-step-ops.js"; import { addCommentImpl, publishArchivedTaskDocumentAdditionImpl, upsertTaskDocumentImpl } from "./task-store/comments-ops.js"; import { deleteTaskImpl, archiveTaskImpl, type DeleteTaskIfResult } from "./task-store/archive-lifecycle.js"; import type { TaskDeleteAuditContext } from "./task-delete-attribution.js"; @@ -2041,6 +2042,9 @@ export class TaskStore extends EventEmitter { async updateTaskAtomic( id: string, updater: ( current: Task, ) => Parameters[1] | null | undefined | Promise[1] | null | undefined>, runContext?: RunMutationContext, ): Promise { return updateTaskAtomicImpl(this, id, updater, runContext); } + async appendRemediationSteps(taskId: string, steps: readonly TaskStep[], options?: AppendRemediationStepsOptions): Promise { + return appendRemediationStepsImpl(this, taskId, steps, options); + } /** Dismisses one active AI merge finding with an operator-provided audit reason. */ async dismissAiMergeReviewFinding(taskId: string, findingId: string, reason: string, actor = "operator"): Promise { const trimmed = reason.trim(); diff --git a/packages/core/src/task-store/remediation-step-ops.ts b/packages/core/src/task-store/remediation-step-ops.ts new file mode 100644 index 0000000000..17ee679692 --- /dev/null +++ b/packages/core/src/task-store/remediation-step-ops.ts @@ -0,0 +1,45 @@ +import type { TaskStore } from "../store.js"; +import type { Task, TaskStep } from "../types.js"; +import { hasOpenEquivalentRemediationStep, remediationWaveCount } from "../tasks/remediation-steps.js"; + +export interface AppendRemediationStepsOptions { + wave?: number; +} + +export interface AppendRemediationStepsResult { + task: Task; + appended: TaskStep[]; + appendedCount: number; + wave: number; +} + +/** + * FNXC:ReviewGatedCoding 2026-08-23-04:52: + * Remediation can arrive while an execution session owns the same task. Append under the task's + * atomic mutation so existing implementation steps are never reordered, rewritten, or lost. + */ +export async function appendRemediationStepsImpl( + store: Pick, + taskId: string, + candidates: readonly TaskStep[], + options: AppendRemediationStepsOptions = {}, +): Promise { + let appended: TaskStep[] = []; + let wave = 0; + const task = await store.updateTaskAtomic(taskId, (current) => { + const existing = current.steps ?? []; + wave = options.wave ?? remediationWaveCount(existing) + 1; + appended = candidates + .filter((candidate) => candidate.remediation !== undefined) + .filter((candidate) => !hasOpenEquivalentRemediationStep([...existing, ...appended], candidate)) + .map((candidate) => ({ + ...candidate, + status: "pending", + remediation: { ...candidate.remediation!, wave: candidate.remediation?.wave ?? wave }, + ...(candidate.dependsOn ? { dependsOn: [...candidate.dependsOn] } : {}), + })); + if (appended.length === 0) return null; + return { steps: [...existing, ...appended] }; + }); + return { task, appended, appendedCount: appended.length, wave }; +} diff --git a/packages/core/src/tasks/remediation-steps.ts b/packages/core/src/tasks/remediation-steps.ts new file mode 100644 index 0000000000..7d4202c019 --- /dev/null +++ b/packages/core/src/tasks/remediation-steps.ts @@ -0,0 +1,48 @@ +import type { TaskStep } from "../types/task/task-log.js"; + +export interface RemediationStepInput { + name?: string; + remediation: NonNullable; + dependsOn?: number[]; +} + +/** + * FNXC:ReviewGatedCoding 2026-08-23-04:52: + * Review remediation names deliberately omit their gate. `Fix (Verification): …` collides with + * legacy lexical replay/evidence rules, while the durable remediation provenance is the sole + * authority for gate identity. + */ +export function formatRemediationStepName(input: { detail?: string; name?: string }): string { + const detail = (input.detail ?? input.name ?? "review finding").replace(/\s+/g, " ").trim(); + return `Fix: ${detail || "review finding"}`; +} + +/** Structural provenance, rather than a step name, classifies appended review work. */ +export function isRemediationStep(step: TaskStep): step is TaskStep & { remediation: NonNullable } { + return step.remediation !== undefined; +} + +export function remediationWaveCount(steps: readonly TaskStep[]): number { + return steps.reduce((highest, step) => Math.max(highest, step.remediation?.wave ?? 0), 0); +} + +const normalize = (value: string | undefined): string => (value ?? "").replace(/\\/g, "/").trim().replace(/\s+/g, " ").toLowerCase(); + +/** Only open equivalent work is deduplicated; a recurrence after completion is new work. */ +export function hasOpenEquivalentRemediationStep( + steps: readonly TaskStep[], + candidate: Pick, +): boolean { + const remediation = candidate.remediation; + if (!remediation) return false; + return steps.some((step) => + isRemediationStep(step) + && (step.status === "pending" || step.status === "in-progress") + && normalize(step.remediation.filePath) === normalize(remediation.filePath) + && normalize(step.remediation.detail) === normalize(remediation.detail), + ); +} + +export function remediationDeclaredFiles(steps: readonly TaskStep[]): string[] { + return [...new Set(steps.flatMap((step) => step.remediation?.declaredFiles ?? []).map((file) => file.trim()).filter(Boolean))].sort(); +} diff --git a/packages/core/src/types/task/task-log.ts b/packages/core/src/types/task/task-log.ts index f81c6ac31a..73545423c0 100644 --- a/packages/core/src/types/task/task-log.ts +++ b/packages/core/src/types/task/task-log.ts @@ -10,6 +10,21 @@ export type StepStatus = "pending" | "in-progress" | "done" | "skipped"; export interface TaskStep { name: string; status: StepStatus; + /** + * FNXC:ReviewGatedCoding 2026-08-23-04:52: + * JSONB task steps need no migration for this additive provenance. Review-gated consumers use + * this durable field—not a human-readable name—to identify appended remediation work. + */ + remediation?: { + wave: number; + gate: string; + gateStepId: string; + findingId?: string; + filePath?: string; + line?: number; + detail?: string; + declaredFiles?: string[]; + }; /** * Step-inversion (KTD-11): 0-indexed indices of steps this step depends on, * parsed from the PROMPT.md `### Step N (depends: 1,2): Title` annotation diff --git a/packages/core/src/workflows/builtin-documentation-delivery-group.ts b/packages/core/src/workflows/builtin-documentation-delivery-group.ts new file mode 100644 index 0000000000..0329094ebe --- /dev/null +++ b/packages/core/src/workflows/builtin-documentation-delivery-group.ts @@ -0,0 +1,32 @@ +import type { WorkflowIrNode } from "./workflow-ir-types.js"; + +export const DOCUMENTATION_DELIVERY_GROUP_ID = "documentation-delivery"; + +const DOCUMENTATION_DELIVERY_PROMPT = `Document and deliver the accepted implementation exactly once. Update relevant operator documentation, save a concise delivery note with fn_task_document_write(key="docs", ...), register visual or media deliverables with fn_artifact_register when present, and record only genuine out-of-scope follow-ups.`; + +/** Documentation runs after passing verification and code review, never as an implementation step. */ +export function documentationDeliveryOptionalGroupNode(column: string): WorkflowIrNode { + return { + id: DOCUMENTATION_DELIVERY_GROUP_ID, + kind: "optional-group", + column, + config: { + name: "Documentation & Delivery", + defaultOn: true, + template: { + nodes: [{ + id: "documentation-delivery-step", + kind: "prompt", + config: { + name: "Documentation & Delivery", + prompt: DOCUMENTATION_DELIVERY_PROMPT, + toolMode: "coding", + gateMode: "gate", + workflowAction: "documentation-delivery", + }, + }], + edges: [], + }, + }, + }; +} diff --git a/packages/core/src/workflows/builtin-plan-review-group.ts b/packages/core/src/workflows/builtin-plan-review-group.ts index cd163aea8d..da221b1304 100644 --- a/packages/core/src/workflows/builtin-plan-review-group.ts +++ b/packages/core/src/workflows/builtin-plan-review-group.ts @@ -65,7 +65,7 @@ column the preceding node established — `todo` in practice, the same lane by a /** Build the `plan-review` optional-group node placed between planning and execution. */ export function planReviewOptionalGroupNode( column?: string, - options: { defaultOn?: boolean; maxRevisions?: number | "unbounded"; requireExternalIntegrationEvidence?: boolean } = {}, + options: { defaultOn?: boolean; maxRevisions?: number | "unbounded"; requireExternalIntegrationEvidence?: boolean; requireImplementationOnlySteps?: boolean } = {}, ): WorkflowIrNode { const promptConfig: Record = { name: PLAN_REVIEW_NAME, @@ -74,6 +74,15 @@ export function planReviewOptionalGroupNode( toolMode: "readonly", gateMode: "gate", }; + if (options.requireImplementationOnlySteps === true) { + /* + * FNXC:ReviewGatedCoding 2026-08-23-04:52: + * A reviewer can distinguish implementation work from a legitimate name containing + * "verification"; parser regexes cannot, so only this workflow opts into the criterion. + */ + promptConfig.prompt = `${PLAN_REVIEW_PROMPT}\n\n## Review-gated implementation steps\nREVISE when the proposed task-step list includes testing, verification, documentation, or delivery work. Those are review-column gates in this workflow, not implementation steps.`; + promptConfig.requireImplementationOnlySteps = true; + } if (options.requireExternalIntegrationEvidence === true) { /* * FNXC:PlanValidation 2026-06-30-08:56: diff --git a/packages/core/src/workflows/builtin-review-gated-coding-workflow-ir.ts b/packages/core/src/workflows/builtin-review-gated-coding-workflow-ir.ts new file mode 100644 index 0000000000..d0c62c16fd --- /dev/null +++ b/packages/core/src/workflows/builtin-review-gated-coding-workflow-ir.ts @@ -0,0 +1,60 @@ +import type { WorkflowIr } from "./workflow-ir-types.js"; +import { parseWorkflowIr } from "./workflow-ir.js"; +import { BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR } from "./builtin-stepwise-final-review-coding-workflow-ir.js"; +import { verificationOptionalGroupNode } from "./builtin-verification-gate-group.js"; +import { documentationDeliveryOptionalGroupNode } from "./builtin-documentation-delivery-group.js"; +import { codeReviewRemediationStepsNode, verificationRemediationNode } from "./builtin-workflow-remediation-nodes.js"; +import { builtinPromptConfig } from "./builtin-workflow-prompts.js"; + +const clone = (ir: WorkflowIr): WorkflowIr => JSON.parse(JSON.stringify(ir)) as WorkflowIr; + +/** + * FNXC:ReviewGatedCoding 2026-08-23-04:52: + * This selectable workflow derives from, but never mutates, the default coding IR. Its review + * gates are structural nodes; task.steps remains implementation work plus appended provenance. + */ +const RAW_BUILTIN_REVIEW_GATED_CODING_WORKFLOW_IR: WorkflowIr = (() => { + const ir = clone(BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR); + ir.name = "builtin-review-gated-coding"; + + const plan = ir.nodes.find((node) => node.id === "plan"); + if (plan) plan.config = builtinPromptConfig("planning-implementation-only", "Plan"); + const planReview = ir.nodes.find((node) => node.id === "plan-review"); + const planTemplate = planReview?.config?.template as { nodes?: Array<{ config?: Record }> } | undefined; + if (planTemplate?.nodes?.[0]?.config) planTemplate.nodes[0].config.requireImplementationOnlySteps = true; + const parse = ir.nodes.find((node) => node.id === "parse"); + if (parse) parse.config = { ...parse.config, implementationOnlySteps: true, preserveRemediationSteps: true }; + + const removed = new Set(["browser-verification", "browser-verification-remediation", "code-review-remediation"]); + ir.nodes = ir.nodes.filter((node) => !removed.has(node.id)); + ir.edges = ir.edges.filter((edge) => !removed.has(edge.from) && !removed.has(edge.to)); + + const codeReviewIndex = ir.nodes.findIndex((node) => node.id === "code-review"); + if (codeReviewIndex < 0) throw new Error("review-gated coding requires the inherited code-review gate"); + ir.nodes.splice(codeReviewIndex, 0, verificationOptionalGroupNode("in-review")); + const completionIndex = ir.nodes.findIndex((node) => node.id === "completion-summary"); + if (completionIndex < 0) throw new Error("review-gated coding requires completion summary"); + ir.nodes.splice(completionIndex, 0, documentationDeliveryOptionalGroupNode("in-review")); + ir.nodes.push(verificationRemediationNode(), codeReviewRemediationStepsNode()); + + ir.edges = ir.edges.filter((edge) => !( + (edge.from === "steps" && edge.to === "code-review") + || (edge.from === "completion-summary" && edge.to === "code-review") + || (edge.from === "code-review" && edge.to === "completion-summary") + || (edge.from === "code-review" && edge.to === "merge-gate") + )); + ir.edges.push( + { from: "steps", to: "verification", condition: "success" }, + { from: "verification", to: "code-review", condition: "success" }, + { from: "code-review", to: "documentation-delivery", condition: "success" }, + { from: "documentation-delivery", to: "completion-summary", condition: "success" }, + { from: "completion-summary", to: "merge-gate", condition: "success" }, + { from: "verification", to: "verification-remediation", condition: "failure" }, + { from: "code-review", to: "code-review-remediation-steps", condition: "failure" }, + { from: "verification-remediation", to: "verification", condition: "success", kind: "rework" }, + { from: "code-review-remediation-steps", to: "verification", condition: "success", kind: "rework" }, + ); + return ir; +})(); + +export const BUILTIN_REVIEW_GATED_CODING_WORKFLOW_IR = parseWorkflowIr(RAW_BUILTIN_REVIEW_GATED_CODING_WORKFLOW_IR); diff --git a/packages/core/src/workflows/builtin-verification-gate-group.ts b/packages/core/src/workflows/builtin-verification-gate-group.ts new file mode 100644 index 0000000000..0ad74eae1b --- /dev/null +++ b/packages/core/src/workflows/builtin-verification-gate-group.ts @@ -0,0 +1,26 @@ +import type { WorkflowIrNode } from "./workflow-ir-types.js"; + +export const VERIFICATION_GROUP_ID = "verification"; + +/** + * FNXC:ReviewGatedCoding 2026-08-23-04:52: + * Verification is a deterministic review-column measurement. The nested gate is deliberately not + * a prompt: only command exit codes may decide whether it passes. + */ +export function verificationOptionalGroupNode(column: string): WorkflowIrNode { + return { + id: VERIFICATION_GROUP_ID, + kind: "optional-group", + column, + config: { + name: "Verification", + defaultOn: true, + reworkRegion: true, + maxReworkCycles: 3, + template: { + nodes: [{ id: "verification-step", kind: "gate", config: { name: "Verification", workflowAction: "deterministic-verification" } }], + edges: [], + }, + }, + }; +} diff --git a/packages/core/src/workflows/builtin-workflow-prompts.ts b/packages/core/src/workflows/builtin-workflow-prompts.ts index 9724382e01..5f48c43c2a 100644 --- a/packages/core/src/workflows/builtin-workflow-prompts.ts +++ b/packages/core/src/workflows/builtin-workflow-prompts.ts @@ -10,6 +10,8 @@ export const BUILTIN_SEAM_PROMPTS: Record = { execute: DEFAULT_EXECUTOR_PROMPT, planning: DEFAULT_TRIAGE_PROMPT, "planning-fast": DEFAULT_TRIAGE_FAST_PROMPT, + /* Review-gated tasks keep test and delivery work in review-column gates. */ + "planning-implementation-only": `${DEFAULT_TRIAGE_PROMPT}\n\n## Review-gated step contract\nProduce implementation steps only. Do not add Testing & Verification or Documentation & Delivery steps; those run as review-column gates after implementation.`, "step-execute": DEFAULT_EXECUTOR_PROMPT, review: DEFAULT_REVIEWER_PROMPT, merge: DEFAULT_MERGER_PROMPT, diff --git a/packages/core/src/workflows/builtin-workflow-remediation-nodes.ts b/packages/core/src/workflows/builtin-workflow-remediation-nodes.ts index 2313fc6f5a..62d8a21329 100644 --- a/packages/core/src/workflows/builtin-workflow-remediation-nodes.ts +++ b/packages/core/src/workflows/builtin-workflow-remediation-nodes.ts @@ -8,6 +8,8 @@ Review-gate remediation must be visible in the workflow graph instead of living export const PLAN_REPLAN_NODE_ID = "plan-replan"; export const BROWSER_VERIFICATION_REMEDIATION_NODE_ID = "browser-verification-remediation"; export const CODE_REVIEW_REMEDIATION_NODE_ID = "code-review-remediation"; +export const VERIFICATION_REMEDIATION_NODE_ID = "verification-remediation"; +export const REVIEW_GATED_CODE_REVIEW_REMEDIATION_NODE_ID = "code-review-remediation-steps"; export function planReplanNode(column = "triage"): WorkflowIrNode { return { @@ -37,6 +39,24 @@ export function browserVerificationRemediationNode(column = "in-progress"): Work }; } +export function verificationRemediationNode(column = "in-progress"): WorkflowIrNode { + return { + id: VERIFICATION_REMEDIATION_NODE_ID, + kind: "prompt", + column, + config: { name: "Verification remediation", workflowAction: "review-remediation-steps", forWorkflowStepId: "verification", toolMode: "readonly" }, + }; +} + +export function codeReviewRemediationStepsNode(column = "in-progress"): WorkflowIrNode { + return { + id: REVIEW_GATED_CODE_REVIEW_REMEDIATION_NODE_ID, + kind: "prompt", + column, + config: { name: "Code review remediation", workflowAction: "review-remediation-steps", forWorkflowStepId: "code-review", toolMode: "readonly" }, + }; +} + export function codeReviewRemediationNode(column = "in-progress"): WorkflowIrNode { return { id: CODE_REVIEW_REMEDIATION_NODE_ID, diff --git a/packages/core/src/workflows/builtin-workflows.ts b/packages/core/src/workflows/builtin-workflows.ts index d410a2699a..cbaaae2ad4 100644 --- a/packages/core/src/workflows/builtin-workflows.ts +++ b/packages/core/src/workflows/builtin-workflows.ts @@ -6,6 +6,7 @@ import { BUILTIN_MARKETING_WORKFLOW_IR } from "./builtin-marketing-workflow-ir.j 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 { BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR } from "./builtin-stepwise-final-review-coding-workflow-ir.js"; +import { BUILTIN_REVIEW_GATED_CODING_WORKFLOW_IR } from "./builtin-review-gated-coding-workflow-ir.js"; import { BUILTIN_WORKFLOW_SETTINGS } from "./builtin-workflow-settings.js"; import { builtinPromptConfig } from "./builtin-workflow-prompts.js"; import { browserVerificationOptionalGroupNode } from "./builtin-browser-verification-group.js"; @@ -548,6 +549,19 @@ export const BUILTIN_WORKFLOWS: WorkflowDefinition[] = [ createdAt: BUILTIN_TS, updatedAt: BUILTIN_TS, }, + { + id: "builtin:review-gated-coding", + name: "Coding (review-gated)", + description: "Coding pipeline with deterministic verification, code review, and delivery gates in review.", + kind: "workflow", + ir: BUILTIN_REVIEW_GATED_CODING_WORKFLOW_IR, + layout: { + start: { x: 60, y: 160 }, plan: { x: 230, y: 160 }, "plan-review": { x: 400, y: 160 }, "plan-replan": { x: 400, y: 320 }, "plan-review-no-op": { x: 570, y: 320 }, parse: { x: 570, y: 160 }, steps: { x: 740, y: 160 }, + verification: { x: 910, y: 160 }, "verification-remediation": { x: 910, y: 320 }, "code-review": { x: 1080, y: 160 }, "code-review-remediation-steps": { x: 1080, y: 320 }, "documentation-delivery": { x: 1250, y: 160 }, "completion-summary": { x: 1420, y: 160 }, "merge-gate": { x: 1590, y: 160 }, "branch-group-member-integration": { x: 1760, y: 80 }, "branch-group-promotion": { x: 1930, y: 80 }, "merge-attempt": { x: 2100, y: 160 }, "merge-retry": { x: 2270, y: 80 }, "recovery-router": { x: 2270, y: 240 }, "merge-manual-hold": { x: 1760, y: 240 }, "post-merge-verification": { x: 2440, y: 160 }, end: { x: 2610, y: 160 }, "review-pending-handoff": { x: 740, y: 320 }, + }, + createdAt: BUILTIN_TS, + updatedAt: BUILTIN_TS, + }, /* * FNXC:CodingIdeasWorkflow 2026-07-04-09:40: * The Coding (Ideas) variant adds a manual "Ideas" intake in front of the default stepwise pipeline. New cards land in "ideas" (autoTriage off) and are not planned until an operator promotes them into the merged "todo" planner column; from there the graph is identical to the default Coding workflow. diff --git a/packages/core/src/workflows/index.ts b/packages/core/src/workflows/index.ts index 36ab825461..58f04b4ca6 100644 --- a/packages/core/src/workflows/index.ts +++ b/packages/core/src/workflows/index.ts @@ -11,6 +11,9 @@ export * from "./builtin-completion-summary-node.js"; export * from "./builtin-lead-generation-workflow-ir.js"; export * from "./builtin-marketing-workflow-ir.js"; export * from "./builtin-plan-review-group.js"; +export * from "./builtin-review-gated-coding-workflow-ir.js"; +export * from "./builtin-verification-gate-group.js"; +export * from "./builtin-documentation-delivery-group.js"; export * from "./builtin-post-merge-group.js"; export * from "./builtin-pr-workflow-ir.js"; export * from "./builtin-stepwise-coding-workflow-ir.js"; diff --git a/packages/dashboard/app/components/TaskCard.css b/packages/dashboard/app/components/TaskCard.css index 69c948b3c1..f525bec407 100644 --- a/packages/dashboard/app/components/TaskCard.css +++ b/packages/dashboard/app/components/TaskCard.css @@ -1428,6 +1428,11 @@ the established compact mobile interaction rhythm while the card remains the pri overflow-y: auto; } +.card-review-gates-separator { + border-top: var(--border-width) solid color-mix(in srgb, var(--border) 70%, transparent); + margin-block: var(--space-xs); +} + .card-step-item { display: flex; align-items: center; diff --git a/packages/dashboard/app/components/TaskCard.tsx b/packages/dashboard/app/components/TaskCard.tsx index 4dc470891e..1fe7ac4d0a 100644 --- a/packages/dashboard/app/components/TaskCard.tsx +++ b/packages/dashboard/app/components/TaskCard.tsx @@ -1740,8 +1740,8 @@ function TaskCardComponent({ In-progress card progress is WIP implementation only. Plan Review (Todo) and Code Review / other review-lane gates must not appear as checklist rows or inflate completed/total while the card is in In progress; badges still use full progress helpers (isPlanReviewRunning / running step labels). */ const unifiedProgress = useMemo( - () => getUnifiedTaskProgress(task, { scope: "implementation" }), - [task.steps, task.enabledWorkflowSteps, task.workflowStepResults], + () => getUnifiedTaskProgress(task, { scope: task.column === "in-review" ? "full" : "implementation" }), + [task.column, task.steps, task.enabledWorkflowSteps, task.workflowStepResults], ); /* FNXC:TaskCardProgress 2026-06-29-02:26: @@ -4168,7 +4168,8 @@ function TaskCardComponent({ {showSteps && (
- {unifiedProgress.items.map((step) => { + {unifiedProgress.items.map((step, index) => { + const beginsReviewGates = step.source === "workflow" && index > 0 && unifiedProgress.items[index - 1]?.source === "step"; /* FNXC:WorkflowSteps 2026-06-25-00:00: The dot color is keyed by the unified status, which now distinguishes the two @@ -4181,7 +4182,9 @@ function TaskCardComponent({ Workflow-sourced rows remain visible through their step names and status dots, but task cards intentionally omit the redundant `workflow` text badge so expanded step lists stay focused on progress. */ return ( -
+
+ {beginsReviewGates && ); })}
diff --git a/packages/dashboard/app/utils/__tests__/taskProgress.review-gates.test.ts b/packages/dashboard/app/utils/__tests__/taskProgress.review-gates.test.ts new file mode 100644 index 0000000000..cdb00827a8 --- /dev/null +++ b/packages/dashboard/app/utils/__tests__/taskProgress.review-gates.test.ts @@ -0,0 +1,19 @@ +import { describe, expect, it } from "vitest"; +import { getRunningWorkflowStepLabel, getUnifiedTaskProgress } from "../taskProgress"; + +describe("review-gated progress", () => { + const task = { + steps: [{ name: "Implement", status: "done" as const }], + enabledWorkflowSteps: ["verification", "code-review", "documentation-delivery"], + workflowStepResults: [{ workflowStepId: "verification", workflowStepName: "Verification", phase: "pre-merge" as const, source: "optional-group" as const, status: "pending" as const, startedAt: "2026-08-23T00:00:00.000Z" }], + }; + + it("excludes review gates from implementation progress and orders them after steps in full progress", () => { + expect(getUnifiedTaskProgress(task, { scope: "implementation" }).items.map((item) => item.name)).toEqual(["Implement"]); + expect(getUnifiedTaskProgress(task).items.map((item) => item.name)).toEqual(["Implement", "Verification", "Code Review", "Documentation Delivery"]); + }); + + it("uses the persisted verification name for the running-gate badge", () => { + expect(getRunningWorkflowStepLabel(task)).toBe("Verification"); + }); +}); diff --git a/packages/dashboard/app/utils/taskProgress.ts b/packages/dashboard/app/utils/taskProgress.ts index 57cf6e5973..a80d9192a3 100644 --- a/packages/dashboard/app/utils/taskProgress.ts +++ b/packages/dashboard/app/utils/taskProgress.ts @@ -96,6 +96,8 @@ const NON_IMPLEMENTATION_WORKFLOW_STEP_IDS = new Set([ "plan-review", "plan-replan", "code-review", + "verification", + "documentation-delivery", "browser-verification", "post-merge-verification", "completion-summary", diff --git a/packages/engine/src/__tests__/review-gated-remediation-steps.test.ts b/packages/engine/src/__tests__/review-gated-remediation-steps.test.ts new file mode 100644 index 0000000000..b79b19c6d8 --- /dev/null +++ b/packages/engine/src/__tests__/review-gated-remediation-steps.test.ts @@ -0,0 +1,43 @@ +import { describe, expect, it } from "vitest"; +import { deriveRemediationSteps } from "../executor/derive-remediation-steps.js"; + +describe("deriveRemediationSteps", () => { + const base = { gateStepId: "code-review", wave: 1, prompt: "## File Scope\n- `src/**`", changedFiles: [] } as const; + + it("turns each blocking code-review finding into provenance-backed work", () => { + const result = deriveRemediationSteps({ + ...base, + gate: "Code Review", + findings: [{ id: "finding-1", title: "wrong guard", body: "Reverse the guard", filePath: "src/guard.ts", line: 8, severity: "critical" }], + }); + expect(result.steps).toEqual([expect.objectContaining({ + name: "Fix: Reverse the guard", + status: "pending", + remediation: expect.objectContaining({ gate: "Code Review", findingId: "finding-1", filePath: "src/guard.ts", line: 8 }), + })]); + expect(result.steps[0].name).not.toMatch(/test|verif|qa|review/i); + }); + + it("does not create work for non-blocking or out-of-scope findings", () => { + const nonBlocking = deriveRemediationSteps({ + ...base, gate: "Code Review", + findings: [{ id: "note", title: "note", body: "note", filePath: "src/a.ts", severity: "medium" }], + }); + expect(nonBlocking.steps).toEqual([]); + + const upstream = deriveRemediationSteps({ + ...base, gate: "Code Review", + findings: [{ id: "upstream", title: "upstream", body: "outside", filePath: "other/a.ts", severity: "critical" }], + }); + expect(upstream).toMatchObject({ steps: [], reason: "upstream-out-of-scope" }); + expect(upstream.outOfScope).toEqual([{ filePath: "other/a.ts", detail: "outside" }]); + }); + + it("derives distinct verification files and a fallback when output has none", () => { + const files = deriveRemediationSteps({ ...base, gate: "Verification", gateStepId: "verification", verificationCommandLabel: "testCommand", verificationOutput: "src/a.ts:3 failed\nsrc/b.ts:6 failed" }); + expect(files.steps.map((step) => step.remediation?.filePath)).toEqual(["src/a.ts", "src/b.ts"]); + const fallback = deriveRemediationSteps({ ...base, gate: "Verification", gateStepId: "verification", verificationCommandLabel: "buildCommand", verificationOutput: "failed" }); + expect(fallback.steps).toHaveLength(1); + expect(fallback.steps[0].name).toBe("Fix: Fix failing buildCommand"); + }); +}); diff --git a/packages/engine/src/__tests__/review-gated-step-preservation.test.ts b/packages/engine/src/__tests__/review-gated-step-preservation.test.ts new file mode 100644 index 0000000000..6824e6f6b7 --- /dev/null +++ b/packages/engine/src/__tests__/review-gated-step-preservation.test.ts @@ -0,0 +1,32 @@ +import { describe, expect, it } from "vitest"; +import type { TaskDetail, TaskStep, WorkflowIrNode } from "@fusion/core"; +import { ParseStepsNodeRunner } from "../workflow-node-runners/parse-steps-runner.js"; + +const node = (config: Record): WorkflowIrNode => ({ id: "parse", kind: "parse-steps", config }); +const task = (steps: TaskStep[] = []) => ({ id: "FN-175", steps } as TaskDetail); + +describe("review-gated parse-step preservation", () => { + it("preserves live remediation before an empty parse can replace task steps", async () => { + const writeSteps = async () => { throw new Error("must not write"); }; + const runner = new ParseStepsNodeRunner({ + readArtifact: async () => "", + writeSteps, + getLiveTask: async () => task([{ name: "Fix: guard", status: "pending", remediation: { wave: 1, gate: "Code Review", gateStepId: "code-review", detail: "guard" } }]), + }); + await expect(runner.run(node({ artifact: "PROMPT.md", parser: "step-headings", preserveRemediationSteps: true }), { task: task(), context: {} })) + .resolves.toMatchObject({ outcome: "success", value: "preserved-remediation-steps" }); + }); + + it("audits but never filters implementation names containing gate words", async () => { + const writes: TaskStep[][] = []; + const audits: string[] = []; + const runner = new ParseStepsNodeRunner({ + readArtifact: async () => "### Step 1: Wire documentation link resolver\n### Step 2: Testing & Verification", + writeSteps: async (_task, steps) => { writes.push(steps); }, + audit: (reason) => audits.push(reason), + }); + await runner.run(node({ artifact: "PROMPT.md", parser: "step-headings", implementationOnlySteps: true }), { task: task(), context: {} }); + expect(writes).toEqual([[{ name: "Wire documentation link resolver", status: "pending" }, { name: "Testing & Verification", status: "pending" }]]); + expect(audits).toContain("implementation-only-leakage"); + }); +}); diff --git a/packages/engine/src/__tests__/review-gated-verification-gate.test.ts b/packages/engine/src/__tests__/review-gated-verification-gate.test.ts new file mode 100644 index 0000000000..cc837c2a9b --- /dev/null +++ b/packages/engine/src/__tests__/review-gated-verification-gate.test.ts @@ -0,0 +1,50 @@ +import { describe, expect, it, vi } from "vitest"; +import type { Settings, TaskStore, WorkflowIrNode } from "@fusion/core"; +import { runDeterministicVerificationGate } from "../workflow-node-runners/verification-gate.js"; + +const node: WorkflowIrNode = { id: "verification-step", kind: "gate", column: "in-review", config: { workflowAction: "deterministic-verification" } }; +const task = { id: "FN-175" }; +const settings = (overrides: Partial = {}) => ({ testCommand: "pnpm test", buildCommand: "pnpm build", ...overrides }) as Settings; + +function result(overrides: Record = {}) { + return { command: "pnpm test", exitCode: 0, stdout: "", stderr: "", success: true, ...overrides }; +} + +describe("runDeterministicVerificationGate", () => { + it("passes only after every configured command exits zero", async () => { + const runCommand = vi.fn().mockResolvedValue(result()); + const gate = await runDeterministicVerificationGate( + { store: {} as TaskStore, runCommand: runCommand as never }, node, task, settings(), "/worktree", + ); + expect(gate).toMatchObject({ outcome: "success", value: "passed" }); + expect(runCommand).toHaveBeenCalledTimes(2); + }); + + it("fails on a non-zero result and preserves its command label", async () => { + const runCommand = vi.fn().mockResolvedValue(result({ exitCode: 1, success: false, stderr: "failure tail" })); + const gate = await runDeterministicVerificationGate( + { store: {} as TaskStore, runCommand: runCommand as never }, node, task, settings(), "/worktree", + ); + expect(gate).toMatchObject({ outcome: "failure", value: "failed" }); + expect(String(gate.contextPatch.output)).toContain("testCommand"); + expect(runCommand).toHaveBeenCalledTimes(1); + }); + + it("fails closed when no command is configured", async () => { + const runCommand = vi.fn(); + const gate = await runDeterministicVerificationGate( + { store: {} as TaskStore, runCommand: runCommand as never }, node, task, settings({ testCommand: undefined, buildCommand: undefined }), "/worktree", + ); + expect(gate).toMatchObject({ outcome: "failure", value: "no-verification-command-configured" }); + expect(runCommand).not.toHaveBeenCalled(); + }); + + it("preserves infrastructure failure classification instead of accepting claimed text", async () => { + const runCommand = vi.fn().mockResolvedValue(result({ exitCode: null, success: false, timedOut: true, stdout: "verification passed" })); + const gate = await runDeterministicVerificationGate( + { store: {} as TaskStore, runCommand: runCommand as never }, node, task, settings({ buildCommand: undefined }), "/worktree", + ); + expect(gate).toMatchObject({ outcome: "failure", value: "verification-infrastructure-failure" }); + expect(String(gate.contextPatch.output)).toContain("timed-out"); + }); +}); diff --git a/packages/engine/src/executor/append-review-remediation-steps.ts b/packages/engine/src/executor/append-review-remediation-steps.ts new file mode 100644 index 0000000000..186d2a67bb --- /dev/null +++ b/packages/engine/src/executor/append-review-remediation-steps.ts @@ -0,0 +1,97 @@ +import { AWAITING_APPROVAL_PAUSE_REASON, remediationDeclaredFiles, remediationWaveCount, type Task, type TaskStore } from "@fusion/core"; +import { deriveRemediationSteps } from "./derive-remediation-steps.js"; +import type { RequestPreMergeOptionalStepFixInfo } from "./request-pre-merge-optional-step-fix.js"; + +export type AppendReviewRemediationStepsDeps = { + store: TaskStore; + readTaskArtifact: (taskId: string, key: string) => Promise; + sendTaskBackForFix: (...args: any[]) => Promise; +}; + +/** + * FNXC:ReviewGatedRemediation 2026-08-23-05:14: + * A review-gated rejection appends named provenance work before it can bounce. This deliberately + * refuses a blind return to implementation: no candidate, out-of-scope evidence, duplicate-only + * work, or the fourth wave is a human hold rather than an empty executor dispatch. + */ +export async function appendReviewRemediationSteps( + deps: AppendReviewRemediationStepsDeps, + task: Task, + info: RequestPreMergeOptionalStepFixInfo, +): Promise { + const gate = info.nodeId === "verification" ? "Verification" : info.nodeId === "code-review" ? "Code Review" : undefined; + if (!gate) return false; + const wave = remediationWaveCount(task.steps ?? []) + 1; + if (wave > 3) return park(deps.store, task.id, "review-remediation-wave-exhausted"); + const prompt = await deps.readTaskArtifact(task.id, "PROMPT.md"); + const derived = deriveRemediationSteps({ + gate, + gateStepId: info.nodeId!, + wave, + findings: info.findings, + verificationOutput: info.feedback, + verificationCommandLabel: gate === "Verification" ? info.stepName : undefined, + prompt, + changedFiles: task.modifiedFiles, + }); + if (derived.reason === "upstream-out-of-scope") { + await deps.store.logEntry(task.id, "Review remediation is out of scope — awaiting human action", derived.outOfScope.map((item) => item.filePath).filter(Boolean).join(", ")); + return park(deps.store, task.id, "review-remediation-upstream-out-of-scope"); + } + if (derived.steps.length === 0) return park(deps.store, task.id, "review-remediation-no-actionable-findings"); + const appended = await deps.store.appendRemediationSteps(task.id, derived.steps, { wave }); + const live = await deps.store.getTask(task.id); + if (appended.appendedCount === 0 || !live.steps.some((step) => step.status === "pending")) { + return park(deps.store, task.id, "review-remediation-no-pending-work"); + } + await widenPromptFileScope(deps.store, task.id, prompt, remediationDeclaredFiles(appended.appended)); + await deps.sendTaskBackForFix( + live, + live.worktree ?? "", + info.feedback, + info.stepName, + `Review gate ${gate} requested named remediation`, + true, + false, + undefined, + info.findings, + undefined, + "none", + ); + return true; +} + +async function park(store: TaskStore, taskId: string, reason: string): Promise { + await store.updateTask(taskId, { + status: "awaiting-approval", + paused: true, + pausedReason: AWAITING_APPROVAL_PAUSE_REASON, + awaitingApprovalReason: "code-review-non-convergence", + }); + await store.logEntry(taskId, "Review remediation requires human action", reason); + return false; +} + +/** + * FNXC:ReviewGatedRemediation 2026-08-23-05:23: + * A remediation accepted from the branch diff may be outside the original prompt scope. Persist its + * declared files before the bounce so the executor and scope-aware squash merge see the same contract. + */ +async function widenPromptFileScope(store: TaskStore, taskId: string, prompt: string | undefined, files: readonly string[]): Promise { + const additions = [...new Set(files.map((file) => file.trim()).filter(Boolean))]; + if (additions.length === 0 || !prompt) return; + const heading = /^##\s+File Scope\s*$/m.exec(prompt); + if (!heading || heading.index === undefined) return; + const sectionStart = heading.index + heading[0].length; + const rest = prompt.slice(sectionStart); + const nextHeading = rest.search(/^##\s/m); + const sectionEnd = nextHeading === -1 ? prompt.length : sectionStart + nextHeading; + const section = prompt.slice(sectionStart, sectionEnd); + const existing = new Set((section.match(/`([^`]+)`/g) ?? []).map((entry) => entry.slice(1, -1))); + const missing = additions.filter((file) => !existing.has(file)); + if (missing.length === 0) return; + const trimmed = section.replace(/\s+$/, ""); + const insertion = missing.map((file) => `- \`${file}\``).join("\n"); + const replacement = trimmed.length === 0 ? `\n\n${insertion}\n` : `${trimmed}\n${insertion}\n`; + await store.updateTask(taskId, { prompt: prompt.slice(0, sectionStart) + replacement + prompt.slice(sectionEnd) }); +} diff --git a/packages/engine/src/executor/build-parse-steps-deps.ts b/packages/engine/src/executor/build-parse-steps-deps.ts index a5eee47601..46ac73cee0 100644 --- a/packages/engine/src/executor/build-parse-steps-deps.ts +++ b/packages/engine/src/executor/build-parse-steps-deps.ts @@ -24,6 +24,7 @@ export function buildParseStepsDeps( writeSteps: async (task, steps: TaskStep[]): Promise => { await deps.store.updateTask(task.id, { steps }); }, + getLiveTask: (taskId) => deps.store.getTask(taskId), hasExpandedForeach: async (task): Promise => { const store = deps.store as unknown as { loadWorkflowRunStepInstancesAsync?: (taskId: string, runId: string) => Promise; diff --git a/packages/engine/src/executor/cleanup-merge-state.ts b/packages/engine/src/executor/cleanup-merge-state.ts index cb2d6bde26..17aaddc5f4 100644 --- a/packages/engine/src/executor/cleanup-merge-state.ts +++ b/packages/engine/src/executor/cleanup-merge-state.ts @@ -21,7 +21,7 @@ export async function cleanupMergeStateForReverification( deps: CleanupMergeStateDeps, task: Task, logMessage: string, - options?: { preserveVerificationFailureCount?: boolean }, + options?: { preserveVerificationFailureCount?: boolean; stepReopenPolicy?: "reopen-trailing" | "none" }, ): Promise { const preservedWorkflowStepResults = preservePreExecutionWorkflowStepResults(task); await deps.store.updateTask(task.id, { @@ -35,7 +35,12 @@ export async function cleanupMergeStateForReverification( const refreshedTask = await deps.store.getTask(task.id); const steps = refreshedTask.steps ?? []; - if (steps.length > 0) { + /* + * FNXC:ReviewGatedRemediation 2026-08-23-05:10: + * Appended provenance steps are the only replay authority for review-gated work. Skipping both + * legacy reopen paths prevents lexical matches from reopening completed implementation steps. + */ + if (options?.stepReopenPolicy !== "none" && steps.length > 0) { const allStepsComplete = isTaskWorkComplete(refreshedTask); if (allStepsComplete) { await deps.reopenLastStepForRevision(task.id, refreshedTask); diff --git a/packages/engine/src/executor/deps-bags.ts b/packages/engine/src/executor/deps-bags.ts index a5046f6f69..c584b150df 100644 --- a/packages/engine/src/executor/deps-bags.ts +++ b/packages/engine/src/executor/deps-bags.ts @@ -669,11 +669,18 @@ export function buildRequestPreMergeOptionalStepFixDeps(host: any): any { ...facadeFields(host, ["store", "workflowLifecycleMovesInFlight"]), ...facadeMethods(host, [ "getRunContextFor", "recoverMissingRequiredArtifacts", "parkPlanReviewReplanCapExhausted", - "clearPausedAborted", "sendTaskBackForFix", + "clearPausedAborted", "readTaskArtifact", "appendReviewRemediationSteps", "sendTaskBackForFix", ]), }; } +export function buildAppendReviewRemediationStepsDeps(host: any): any { + return { + ...facadeFields(host, ["store"]), + ...facadeMethods(host, ["readTaskArtifact", "sendTaskBackForFix"]), + }; +} + export function buildHandleLoopDetectedDeps(host: any): any { return { ...facadeFields(host, ["store", "activeSessions", "loopRecoveryState"]), diff --git a/packages/engine/src/executor/derive-remediation-steps.ts b/packages/engine/src/executor/derive-remediation-steps.ts new file mode 100644 index 0000000000..ccd2855b18 --- /dev/null +++ b/packages/engine/src/executor/derive-remediation-steps.ts @@ -0,0 +1,76 @@ +import { formatRemediationStepName, isBlockingFinding, type ReviewBlockingSeverity, type TaskStep, type WorkflowReviewFinding } from "@fusion/core"; +import { extractFileScope, matchesScope } from "../merge/merger-file-scope.js"; + +export interface DeriveRemediationStepsInput { + gate: "Code Review" | "Verification"; + gateStepId: string; + wave: number; + findings?: WorkflowReviewFinding[]; + blockingSeverity?: ReviewBlockingSeverity; + verificationOutput?: string; + verificationCommandLabel?: string; + prompt?: string; + changedFiles?: readonly string[]; +} + +export interface DerivedRemediationSteps { + steps: TaskStep[]; + outOfScope: Array<{ filePath?: string; detail: string }>; + reason?: "upstream-out-of-scope"; +} + +const fileReference = /(?:^|[\s(])([\w@./-]+\.(?:[cm]?[jt]sx?|json|md|css|html|yml|yaml))(?::(\d+))?/gm; +const normalized = (value: string) => value.replace(/\\/g, "/").trim(); + +function verificationCandidates(input: DeriveRemediationStepsInput): Array<{ filePath?: string; line?: number; detail: string }> { + const detail = input.verificationCommandLabel ?? "verification command"; + const seen = new Set(); + const candidates: Array<{ filePath?: string; line?: number; detail: string }> = []; + for (const match of input.verificationOutput?.matchAll(fileReference) ?? []) { + const filePath = normalized(match[1]); + if (!seen.has(filePath)) { + seen.add(filePath); + candidates.push({ filePath, ...(match[2] ? { line: Number(match[2]) } : {}), detail: `Fix failing ${detail}: ${filePath}` }); + } + } + return candidates.length > 0 ? candidates : [{ detail: `Fix failing ${detail}` }]; +} + +/** + * FNXC:ReviewGatedRemediation 2026-08-23-05:06: + * Gate findings become explicit append-only steps with provenance. Scope filtering happens before + * append: an unrelated failure is upstream work, never invented remediation for this task. + */ +export function deriveRemediationSteps(input: DeriveRemediationStepsInput): DerivedRemediationSteps { + const candidates: Array<{ filePath?: string; line?: number; detail: string; findingId?: string }> = input.gate === "Code Review" + ? (input.findings ?? []) + .filter((finding) => isBlockingFinding(finding, input.blockingSeverity ?? "critical")) + .map((finding) => ({ filePath: finding.filePath, line: finding.line, detail: finding.body || finding.title, findingId: finding.id })) + : verificationCandidates(input); + const declaredScope = extractFileScope(input.prompt ?? ""); + const changedFiles = new Set((input.changedFiles ?? []).map(normalized)); + const steps: TaskStep[] = []; + const outOfScope: Array<{ filePath?: string; detail: string }> = []; + for (const candidate of candidates) { + const filePath = candidate.filePath ? normalized(candidate.filePath) : undefined; + const allowed = !filePath || matchesScope(filePath, declaredScope) || changedFiles.has(filePath); + if (!allowed) { + outOfScope.push({ filePath, detail: candidate.detail }); + continue; + } + steps.push({ + name: formatRemediationStepName({ detail: candidate.detail }), + status: "pending", + remediation: { + wave: input.wave, + gate: input.gate, + gateStepId: input.gateStepId, + ...(candidate.findingId ? { findingId: candidate.findingId } : {}), + ...(filePath ? { filePath, declaredFiles: [filePath] } : {}), + ...(candidate.line ? { line: candidate.line } : {}), + detail: candidate.detail, + }, + }); + } + return { steps, outOfScope, ...(candidates.length > 0 && steps.length === 0 ? { reason: "upstream-out-of-scope" } : {}) }; +} diff --git a/packages/engine/src/executor/free-reexports.ts b/packages/engine/src/executor/free-reexports.ts index 8de2d418f3..b8c76e53ac 100644 --- a/packages/engine/src/executor/free-reexports.ts +++ b/packages/engine/src/executor/free-reexports.ts @@ -139,6 +139,7 @@ export { resetStepsIfWorkLost as resetStepsIfWorkLostFree } from "./reset-steps- export { routeRetryableRemediationGraphFailureToPreMergeFix as routeRetryableRemediationGraphFailureToPreMergeFixFree } from "./route-retryable-remediation.js"; export { buildForeachWorktreeDeps as buildForeachWorktreeDepsFree } from "./build-foreach-worktree-deps.js"; export { requestPreMergeOptionalStepFix as requestPreMergeOptionalStepFixFree } from "./request-pre-merge-optional-step-fix.js"; +export { appendReviewRemediationSteps as appendReviewRemediationStepsFree } from "./append-review-remediation-steps.js"; export { createSpawnAgentTool as createSpawnAgentToolFree, spawnAgentParams as spawnAgentParamsFree } from "./create-spawn-agent-tool.js"; export { createTaskUpdateTool as createTaskUpdateToolFree } from "./create-task-update-tool.js"; export { attemptExecutorVerificationFix as attemptExecutorVerificationFixFree } from "./attempt-executor-verification-fix.js"; diff --git a/packages/engine/src/executor/impl-bindings.ts b/packages/engine/src/executor/impl-bindings.ts index 9afd7da8a8..e339bb1f08 100644 --- a/packages/engine/src/executor/impl-bindings.ts +++ b/packages/engine/src/executor/impl-bindings.ts @@ -136,6 +136,7 @@ export { resetStepsIfWorkLost as resetStepsIfWorkLostImpl } from "./reset-steps- export { routeRetryableRemediationGraphFailureToPreMergeFix as routeRetryableRemediationGraphFailureToPreMergeFixImpl } from "./route-retryable-remediation.js"; export { buildForeachWorktreeDeps as buildForeachWorktreeDepsImpl } from "./build-foreach-worktree-deps.js"; export { requestPreMergeOptionalStepFix as requestPreMergeOptionalStepFixImpl } from "./request-pre-merge-optional-step-fix.js"; +export { appendReviewRemediationSteps as appendReviewRemediationStepsImpl } from "./append-review-remediation-steps.js"; export { createSpawnAgentTool as createSpawnAgentToolImpl } from "./create-spawn-agent-tool.js"; export { createTaskUpdateTool as createTaskUpdateToolImpl } from "./create-task-update-tool.js"; export { attemptExecutorVerificationFix as attemptExecutorVerificationFixImpl } from "./attempt-executor-verification-fix.js"; diff --git a/packages/engine/src/executor/request-pre-merge-optional-step-fix.ts b/packages/engine/src/executor/request-pre-merge-optional-step-fix.ts index 656f10e992..8c35d67d01 100644 --- a/packages/engine/src/executor/request-pre-merge-optional-step-fix.ts +++ b/packages/engine/src/executor/request-pre-merge-optional-step-fix.ts @@ -138,6 +138,8 @@ export type RequestPreMergeOptionalStepFixDeps = { feedback: string, ) => Promise; clearPausedAborted: (taskId: string) => void; + readTaskArtifact: (taskId: string, key: string) => Promise; + appendReviewRemediationSteps: (task: Task, info: RequestPreMergeOptionalStepFixInfo) => Promise; workflowLifecycleMovesInFlight: Set; sendTaskBackForFix: ( task: Task, @@ -364,6 +366,21 @@ export async function requestPreMergeOptionalStepFix( return true; } + const selection = await deps.store.getTaskWorkflowSelectionAsync?.(taskId) + ?? deps.store.getTaskWorkflowSelection?.(taskId); + /* + * FNXC:ReviewGatedRemediation 2026-08-23-05:23: + * Review-gated handoff runs only after the shared operator-hold, artifact, and provider-verdict + * guards above. A deterministic Verification failure has no reviewer verdict; Code Review still + * requires a genuine REVISE so transport failures cannot manufacture remediation work. + */ + if ( + selection?.workflowId === "builtin:review-gated-coding" + && (info.nodeId === "verification" || (info.nodeId === "code-review" && info.verdict === "REVISE")) + ) { + return deps.appendReviewRemediationSteps(liveTask, info); + } + if (info.verdict !== "REVISE") { // FNXC:RemediationVisibility 2026-07-26-19:20: a hard-failed gate with no parsed REVISE // verdict schedules nothing, so the remediation node fails and the card parks. Say so. diff --git a/packages/engine/src/executor/reset-merge-state.ts b/packages/engine/src/executor/reset-merge-state.ts index 8c7269e143..bb2bc3d3ff 100644 --- a/packages/engine/src/executor/reset-merge-state.ts +++ b/packages/engine/src/executor/reset-merge-state.ts @@ -19,7 +19,7 @@ export type ResetMergeStateDeps = { cleanupMergeStateForReverification: ( task: Task, logMessage: string, - options?: { preserveVerificationFailureCount?: boolean }, + options?: { preserveVerificationFailureCount?: boolean; stepReopenPolicy?: "reopen-trailing" | "none" }, ) => Promise; }; @@ -52,6 +52,8 @@ export async function resetMergeStateIfNeeded( return task; } + const selection = await deps.store.getTaskWorkflowSelectionAsync?.(task.id) + ?? deps.store.getTaskWorkflowSelection?.(task.id); return deps.cleanupMergeStateForReverification( task, `Task returned to in-progress from ${from} column — resetting verification steps and merge state for re-verification`, @@ -60,6 +62,7 @@ export async function resetMergeStateIfNeeded( // cycles. Status may be cleared by intermediate paths, so the counter is // the canonical signal once a bounce has started. preserveVerificationFailureCount: (task.verificationFailureCount ?? 0) > 0, + stepReopenPolicy: selection?.workflowId === "builtin:review-gated-coding" ? "none" : "reopen-trailing", }, ); } diff --git a/packages/engine/src/executor/run-graph-custom-node.ts b/packages/engine/src/executor/run-graph-custom-node.ts index b7f2faa8d8..4a13dcf39f 100644 --- a/packages/engine/src/executor/run-graph-custom-node.ts +++ b/packages/engine/src/executor/run-graph-custom-node.ts @@ -37,6 +37,7 @@ import { parseAwaitInputSentinel } from "./await-input-parse.js"; import { buildAgentPersona } from "./agent-binding-pure.js"; import { reviewWorkspacePerRepo } from "./workspace-review-per-repo.js"; import type { ReviewResult } from "../execution/reviewer.js"; +import { runDeterministicVerificationGate } from "../workflow-node-runners/verification-gate.js"; const WORKFLOW_THINKING_LEVEL_SET: ReadonlySet = new Set(THINKING_LEVELS); @@ -190,7 +191,8 @@ export async function runGraphCustomNode( leave runtime requiring a worktree that preparation declined to acquire. Plan Review remains excluded because it uses the narrow PROMPT.md writer. */ - const writeCapable = workflowNodeRequiresWorktree(node, { + const isDeterministicVerificationGate = cfg.workflowAction === "deterministic-verification"; + const writeCapable = isDeterministicVerificationGate || workflowNodeRequiresWorktree(node, { optionalGroupId, reviewerInlineFixes: (settings as Settings & { reviewerInlineFixes?: boolean }).reviewerInlineFixes, }); @@ -278,6 +280,9 @@ export async function runGraphCustomNode( const worktreePath = workspaceConfig && !writeCapable ? deps.rootDir : executionTarget.worktree || legacyWorkspacePath || workspaceTaskDir!; + if (isDeterministicVerificationGate) { + return runDeterministicVerificationGate({ store: deps.store }, node, live, settings, worktreePath); + } let prompt = typeof cfg.prompt === "string" ? cfg.prompt : ""; let modelProvider = typeof cfg.modelProvider === "string" && cfg.modelProvider.trim() ? cfg.modelProvider : undefined; let modelId = typeof cfg.modelId === "string" && cfg.modelId.trim() ? cfg.modelId : undefined; diff --git a/packages/engine/src/executor/run-implementation.ts b/packages/engine/src/executor/run-implementation.ts index 820b8a5640..cf094ca9e8 100644 --- a/packages/engine/src/executor/run-implementation.ts +++ b/packages/engine/src/executor/run-implementation.ts @@ -540,9 +540,12 @@ export async function runImplementation( if (task.column === preflightWipLane && task.mergeDetails) { executorLog.warn(`${task.id}: stale mergeDetails found while executing in-progress task — resetting merge state before continuing`); + const selection = await deps.store.getTaskWorkflowSelectionAsync?.(task.id) + ?? deps.store.getTaskWorkflowSelection?.(task.id); task = await deps.cleanupMergeStateForReverification( task, "Executor detected stale merge state while task was in-progress — reset verification steps and merge metadata before resuming", + { stepReopenPolicy: selection?.workflowId === "builtin:review-gated-coding" ? "none" : "reopen-trailing" }, ); } diff --git a/packages/engine/src/executor/send-task-back-for-fix.ts b/packages/engine/src/executor/send-task-back-for-fix.ts index 7b97763588..0c75b0f299 100644 --- a/packages/engine/src/executor/send-task-back-for-fix.ts +++ b/packages/engine/src/executor/send-task-back-for-fix.ts @@ -48,6 +48,7 @@ export async function sendTaskBackForFix( findings?: WorkflowReviewFinding[], /** Workspace remediation must not overwrite singular task checkout routing. */ persistWorktreePath?: boolean, + stepReopenPolicy: "reopen-trailing" | "none" = "reopen-trailing", ): Promise { const taskId = task.id; deps.clearCompletedTaskWatchdog(taskId); @@ -102,7 +103,9 @@ export async function sendTaskBackForFix( // 4. Re-open only the last step for a single in-place fix pass. Earlier // done steps stay done so the executor doesn't redo finished work. const updatedTask = await deps.store.getTask(taskId); - await deps.reopenLastStepForRevision(taskId, updatedTask); + if (stepReopenPolicy === "reopen-trailing") { + await deps.reopenLastStepForRevision(taskId, updatedTask); + } // 5. Clear error/status/session fields and reset workflow step retries. // FNXC:ReviewLeniency 2026-07-02-02:10: prior terminal failure results diff --git a/packages/engine/src/executor/task-executor-session-facades.ts b/packages/engine/src/executor/task-executor-session-facades.ts index e7f17a7a2b..ace09df090 100644 --- a/packages/engine/src/executor/task-executor-session-facades.ts +++ b/packages/engine/src/executor/task-executor-session-facades.ts @@ -86,6 +86,7 @@ export abstract class TaskExecutorSessionFacades extends TaskExecutorWorktreePur protected async executeReviewHandoff(...args: FacadeRestArgs): ReturnType { return impl.executeReviewHandoffImpl(bags.buildExecuteReviewHandoffDeps(this), ...args); } async recoverCompletedTask(task: import("@fusion/core").Task): Promise { return impl.recoverCompletedTaskImpl(bags.buildRecoverCompletedTaskDeps(this), task); } protected async parkPlanReviewReplanCapExhausted(...args: FacadeRestArgs): ReturnType { return impl.parkPlanReviewReplanCapExhaustedImpl(bags.buildStoreRunContextDeps(this), ...args); } + protected async appendReviewRemediationSteps(...args: FacadeRestArgs): ReturnType { return impl.appendReviewRemediationStepsImpl(bags.buildAppendReviewRemediationStepsDeps(this), ...args); } protected async requestPreMergeOptionalStepFix(...args: FacadeRestArgs): ReturnType { return impl.requestPreMergeOptionalStepFixImpl(bags.buildRequestPreMergeOptionalStepFixDeps(this), ...args); } protected async recoverMissingRequiredArtifacts(...args: FacadeRestArgs): ReturnType { return impl.recoverMissingRequiredArtifactsImpl(bags.buildRecoverMissingRequiredArtifactsDeps(this), ...args); } async recoverFailedPreMergeWorkflowStep(task: import("@fusion/core").Task): Promise { return impl.recoverFailedPreMergeWorkflowStepImpl(bags.buildRecoverFailedPreMergeWorkflowStepDeps(this), task); } diff --git a/packages/engine/src/merge/merger-ai.ts b/packages/engine/src/merge/merger-ai.ts index b4c4646eb0..2d6ba3fdef 100644 --- a/packages/engine/src/merge/merger-ai.ts +++ b/packages/engine/src/merge/merger-ai.ts @@ -72,6 +72,7 @@ import { selectUserCommentsForAgentContext } from "../agents/agent-user-comments import { resolveTaskWorkingBranch } from "../worktree/worktree-names.js"; import { resolveIntegrationBranch } from "./integration-branch.js"; import { shouldClearOrphanedMergeStamp } from "./merge-active-status.js"; + import { recordWorkspaceBaseBranchDecision, resolveWorkspaceRepoBaseBranch } from "../worktree/workspace-base-branch.js"; import { captureWorkspaceReviewEvidence } from "../worktree/workspace-review-evidence.js"; import { advanceIntegrationBranchRef } from "./merger-ref-update-advance.js"; @@ -149,6 +150,20 @@ const aiMergeLog = createLogger("merger-ai"); * sync telemetry is considered. This production helper makes that ordering independently * executable while retaining the fire-and-forget audit contract for ordinary sync failures. */ +/* + * FNXC:ReviewGatedRemediation 2026-08-23-05:23: + * The AI empty-merge path must carry the selected workflow's required gates into the shared + * zero-diff guard; otherwise a review-gated card can finalize before deterministic verification. + */ +async function resolveNoOpFinalizeGateIds(store: TaskStore, task: Task): Promise | undefined> { + const selection = store.getTaskWorkflowSelectionAsync + ? await store.getTaskWorkflowSelectionAsync(task.id) + : store.getTaskWorkflowSelection?.(task.id); + if (!selection) return undefined; + const ir = await resolveWorkflowIrForTask(store, task.id).catch(() => undefined); + return ir ? resolveRequiredPreMergeStepIds(ir, task.enabledWorkflowSteps) : undefined; +} + export function recordBranchGroupPrSyncFailureAudit( store: RunAuditSinkHost, taskId: string, @@ -1566,7 +1581,9 @@ export async function runAiMerge( }); if (landResult.outcome === "empty") { - const noCommitsFinalize = evaluateNoCommitsNoOpFinalize(task); + const noCommitsFinalize = evaluateNoCommitsNoOpFinalize(task, { + requiredVerificationStepIds: await resolveNoOpFinalizeGateIds(store, task), + }); if (noCommitsFinalize.blocked) { const reason = noCommitsFinalize.reason ?? "no-commits task has incomplete work with no net branch changes"; /* diff --git a/packages/engine/src/merger.ts b/packages/engine/src/merger.ts index b36b639bec..2012d3c285 100644 --- a/packages/engine/src/merger.ts +++ b/packages/engine/src/merger.ts @@ -6418,6 +6418,21 @@ workflow steps run exclusively as the workflow graph's own post-merge optional-g * - On finalize, best-effort cleanup of the stranded `task.worktree` and * `fusion/` branch keeps `.worktrees/` and the branch namespace tidy. */ +/* + * FNXC:ReviewGatedRemediation 2026-08-23-05:23: + * Empty-diff finalizers must resolve the selected workflow's required pre-merge gates, not infer + * verification from implementation-step names. Legacy callers without a workflow selection retain + * their historical guard behavior. + */ +async function resolveNoOpFinalizeGateIds(store: TaskStore, task: Task): Promise | undefined> { + const selection = store.getTaskWorkflowSelectionAsync + ? await store.getTaskWorkflowSelectionAsync(task.id) + : store.getTaskWorkflowSelection?.(task.id); + if (!selection) return undefined; + const ir = await resolveWorkflowIrForTask(store, task.id).catch(() => undefined); + return ir ? resolveRequiredPreMergeStepIds(ir, task.enabledWorkflowSteps) : undefined; +} + async function tryEarlyEmptyOwnDiffFinalize(input: { task: Task; taskId: string; @@ -6480,7 +6495,9 @@ async function tryEarlyEmptyOwnDiffFinalize(input: { return null; } - const noCommitsFinalize = evaluateNoCommitsNoOpFinalize(task); + const noCommitsFinalize = evaluateNoCommitsNoOpFinalize(task, { + requiredVerificationStepIds: await resolveNoOpFinalizeGateIds(store, task), + }); if (noCommitsFinalize.blocked) { const reason = noCommitsFinalize.reason ?? "no-commits task has incomplete work with no net branch changes"; /* @@ -7614,7 +7631,9 @@ export async function aiMergeTask( // — NOT a legitimate no-op. Demote to the unproven-recovery path which // moves the task back to todo with progress preserved instead of // clearing modifiedFiles to []. - const noCommitsFinalize = evaluateNoCommitsNoOpFinalize(task); + const noCommitsFinalize = evaluateNoCommitsNoOpFinalize(task, { + requiredVerificationStepIds: await resolveNoOpFinalizeGateIds(store, task), + }); if (noCommitsFinalize.blocked) { const reason = noCommitsFinalize.reason ?? "no-commits task has incomplete work with no net branch changes"; /* @@ -7918,7 +7937,9 @@ export async function aiMergeTask( result.mergeTargetSource = mergeTarget.source; mergerLog.log(`${taskId}: branch missing; recovered owned landed commit ${classification.commit.sha.slice(0, 8)}`); } else { - const noCommitsFinalize = evaluateNoCommitsNoOpFinalize(task); + const noCommitsFinalize = evaluateNoCommitsNoOpFinalize(task, { + requiredVerificationStepIds: await resolveNoOpFinalizeGateIds(store, task), + }); if (noCommitsFinalize.blocked) { const reason = noCommitsFinalize.reason ?? "no-commits task has incomplete work with no net branch changes"; /* diff --git a/packages/engine/src/self-healing.ts b/packages/engine/src/self-healing.ts index ed432e24dc..ac53e4f8fa 100644 --- a/packages/engine/src/self-healing.ts +++ b/packages/engine/src/self-healing.ts @@ -30,7 +30,7 @@ import { existsSync, mkdirSync, readdirSync, readFileSync, realpathSync, rmSync, import { readFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { basename, dirname, isAbsolute, join, relative, resolve } from "node:path"; -import { loadWorkspaceConfig, type TaskMoveLanes, resolveColumnFlags, IN_REVIEW_STALL_DEADLOCK_LOG_PREFIX, IN_REVIEW_STALL_LOG_PREFIX, IN_REVIEW_STALL_TERMINAL_LOG_PREFIX, allowsAutoMergeProcessing, hasSharedBranchMemberAutoMergeHold, resolveEffectiveAutoMerge, countRecentIdenticalStallEntries, detectDependencyCycle, detectSelfDefeatingDependency, evaluateNoCommitsNoOpFinalize, evaluateCompletedPromotionFailureProvenance, evaluateSkipBypassTaint, getInReviewStalledSignal, getInReviewStallReason, getPrimaryPrInfo, getStalePausedReviewSignal, getStalePausedTodoSignal, getTaskHardMergeBlocker, getTaskMergeBlocker, isEphemeralAgent, isMergeRequestContractShadowEnabled, isWorkspaceTask, isSharedBranchGroupMemberIntegration, isLiveSharedBranchGroupMemberIntegration, isNearDuplicateCanonicalInactive, parseExplicitDuplicateMarker, flagTriageDuplicate, isTriageDuplicateKeepAcknowledged, resolveMaxAutoMergeRetries, resolveOptionalStepRevisionBudget, resolveOptionalReviewRevisionBudget, getBuiltinWorkflow, isBuiltinWorkflowId, resolveWorkflowIrForTask, resolveWorkflowIrForTaskWithProvenance, resolveReboundTarget, resolveReboundTargetForTask, columnsWithFlag, resolveLifecycleColumns, resolveTaskLifecycleColumns, workflowHasColumn, planLegacyAdoption, resolveOrphanedPendingStepResults, classifyReviewLease, PLAN_REVIEW_LEASE_STALENESS_MS, DEFAULT_MAX_POST_REVIEW_FIXES, ACTIVE_WORKFLOW_WORK_ITEM_STATES, AWAITING_APPROVAL_PAUSE_REASON, type Agent, type AgentStore, type ChatStore, type MessageStore, type TaskStore, type Settings, type Task, type MergeDetails, type TaskPriority, type MergeResult, type WorkflowStepResult, type WorkflowIr, +import { loadWorkspaceConfig, type TaskMoveLanes, resolveColumnFlags, IN_REVIEW_STALL_DEADLOCK_LOG_PREFIX, IN_REVIEW_STALL_LOG_PREFIX, IN_REVIEW_STALL_TERMINAL_LOG_PREFIX, allowsAutoMergeProcessing, hasSharedBranchMemberAutoMergeHold, resolveEffectiveAutoMerge, countRecentIdenticalStallEntries, detectDependencyCycle, detectSelfDefeatingDependency, evaluateNoCommitsNoOpFinalize, evaluateCompletedPromotionFailureProvenance, evaluateSkipBypassTaint, getInReviewStalledSignal, getInReviewStallReason, getPrimaryPrInfo, getStalePausedReviewSignal, getStalePausedTodoSignal, getTaskHardMergeBlocker, getTaskMergeBlocker, isEphemeralAgent, isMergeRequestContractShadowEnabled, isWorkspaceTask, isSharedBranchGroupMemberIntegration, isLiveSharedBranchGroupMemberIntegration, isNearDuplicateCanonicalInactive, parseExplicitDuplicateMarker, flagTriageDuplicate, isTriageDuplicateKeepAcknowledged, resolveMaxAutoMergeRetries, resolveOptionalStepRevisionBudget, resolveOptionalReviewRevisionBudget, getBuiltinWorkflow, isBuiltinWorkflowId, resolveWorkflowIrForTask, resolveWorkflowIrForTaskWithProvenance, resolveRequiredPreMergeStepIds, resolveReboundTarget, resolveReboundTargetForTask, columnsWithFlag, resolveLifecycleColumns, resolveTaskLifecycleColumns, workflowHasColumn, planLegacyAdoption, resolveOrphanedPendingStepResults, classifyReviewLease, PLAN_REVIEW_LEASE_STALENESS_MS, DEFAULT_MAX_POST_REVIEW_FIXES, ACTIVE_WORKFLOW_WORK_ITEM_STATES, AWAITING_APPROVAL_PAUSE_REASON, type Agent, type AgentStore, type ChatStore, type MessageStore, type TaskStore, type Settings, type Task, type MergeDetails, type TaskPriority, type MergeResult, type WorkflowStepResult, type WorkflowIr, resolveNearDuplicateCanonicalFlags, LEGACY_COLUMN_IDS_BY_ROLE, TERMINAL_ROLES, @@ -613,6 +613,20 @@ const RECONCILE_SCOPE_OVERRIDE_MERGE_ACTIVE_STATUS_SET = new Set(MERGE_A // (notification-service tests in particular). Re-exported here for callers // that already depend on `self-healing.ts` exports. import { classifyTransientMergeError } from "./errors/transient-merge-error-classifier.js"; + +/* + * FNXC:ReviewGatedRemediation 2026-08-23-05:23: + * Self-healing's zero-diff finalizers use the selected workflow's required gate set so recovery + * cannot bypass a failed or absent deterministic Verification result. + */ +async function resolveNoOpFinalizeGateIds(store: TaskStore, task: Task): Promise | undefined> { + const selection = store.getTaskWorkflowSelectionAsync + ? await store.getTaskWorkflowSelectionAsync(task.id) + : store.getTaskWorkflowSelection?.(task.id); + if (!selection) return undefined; + const ir = await resolveWorkflowIrForTask(store, task.id).catch(() => undefined); + return ir ? resolveRequiredPreMergeStepIds(ir, task.enabledWorkflowSteps) : undefined; +} export { classifyTransientMergeError } from "./errors/transient-merge-error-classifier.js"; const MAX_STARVATION_DROPS = 3; type AutoArchiveFailureReason = "lineage-children" | "task-live" | "dependents" | "not-found" | "unknown"; @@ -3464,7 +3478,6 @@ export class SelfHealingManager extends SelfHealingGitEvidence { * FNXC:Lifecycle 2026-06-14-20:12: * FN-6461 keeps skipped-to-completion no-commits tasks out of the stranded-todo promoter so a finalize guard demotion cannot loop back into in-review before an operator fixes the incomplete work. */ - if (evaluateNoCommitsNoOpFinalize(task).blocked) return false; /* * FNXC:Lifecycle 2026-07-16-21:40: * FN-8141 — the stranded-todo promoter was the exact path that laundered FN-8141 into @@ -3490,6 +3503,9 @@ export class SelfHealingManager extends SelfHealingGitEvidence { const irCache = new Map>>(); const stranded: Task[] = []; for (const task of completedNonColumnCandidates) { + if (evaluateNoCommitsNoOpFinalize(task, { + requiredVerificationStepIds: await resolveNoOpFinalizeGateIds(this.store, task), + }).blocked) continue; let holdColumn = "todo"; try { const lifecycle = resolveLifecycleColumns( @@ -8693,7 +8709,9 @@ export class SelfHealingManager extends SelfHealingGitEvidence { // the audit trail of the lost work. Now we refuse to finalize and // move the task back to todo with progress preserved so the next // executor run can re-attempt. - const noCommitsFinalize = evaluateNoCommitsNoOpFinalize(task); + const noCommitsFinalize = evaluateNoCommitsNoOpFinalize(task, { + requiredVerificationStepIds: await resolveNoOpFinalizeGateIds(this.store, task), + }); if (noCommitsFinalize.blocked) { const reason = noCommitsFinalize.reason ?? "no-commits task has incomplete work with no net branch changes"; /* diff --git a/packages/engine/src/workflow-node-runners/parse-steps-runner.ts b/packages/engine/src/workflow-node-runners/parse-steps-runner.ts index bb2288edd8..84711e80f8 100644 --- a/packages/engine/src/workflow-node-runners/parse-steps-runner.ts +++ b/packages/engine/src/workflow-node-runners/parse-steps-runner.ts @@ -1,4 +1,4 @@ -import { getStepParser } from "@fusion/core"; +import { getStepParser, isRemediationStep } from "@fusion/core"; import type { TaskDetail, TaskStep, WorkflowIrNode } from "@fusion/core"; import type { WorkflowNodeHandler, WorkflowNodeResult } from "../workflows/workflow-graph-executor.js"; @@ -12,6 +12,8 @@ export interface ParseStepsHandlerDeps { writeSteps: (task: TaskDetail, steps: TaskStep[]) => Promise; hasExpandedForeach?: (task: TaskDetail) => Promise | boolean; audit?: (reason: string, detail: string) => void; + /** Re-read live task state before replacement writes can erase concurrently appended work. */ + getLiveTask?: (taskId: string) => Promise; } /* @@ -28,7 +30,22 @@ export class ParseStepsNodeRunner implements WorkflowNodeRunner { artifact?: unknown; parser?: unknown; requireStepsUnlessNoCommits?: unknown; + implementationOnlySteps?: unknown; + preserveRemediationSteps?: unknown; }; + + /* + * FNXC:ReviewGatedRemediation 2026-08-23-05:06: + * writeSteps replaces the whole list. Preserve live appended remediation before every parser, + * artifact, or empty-list path so re-entry cannot wipe pending correction work. + */ + if (cfg.preserveRemediationSteps === true) { + const liveTask = this.deps.getLiveTask ? await this.deps.getLiveTask(ctx.task.id) : ctx.task; + if (liveTask.steps.some(isRemediationStep)) { + this.audit("preserved-remediation-steps", `parse-steps node '${node.id}' preserved live remediation steps for task ${ctx.task.id}`); + return { outcome: "success", value: "preserved-remediation-steps" }; + } + } const parserId = typeof cfg.parser === "string" ? cfg.parser : ""; const artifactKey = typeof cfg.artifact === "string" && cfg.artifact.trim() !== "" @@ -115,6 +132,13 @@ export class ParseStepsNodeRunner implements WorkflowNodeRunner { if (Array.isArray(s.dependsOn)) step.dependsOn = s.dependsOn; return step; }); + if (cfg.implementationOnlySteps === true) { + const leakage = steps.filter((step) => /(^|[^a-z])(testing|verification|documentation|delivery)([^a-z]|$)/i.test(step.name)); + if (leakage.length > 0) { + // Detection is deliberately non-destructive: implementation names can legitimately contain these words. + this.audit("implementation-only-leakage", `parse-steps node '${node.id}' detected possible review-gate work: ${leakage.map((step) => step.name).join(", ")}`); + } + } try { await this.deps.writeSteps(ctx.task, steps); } catch (err) { diff --git a/packages/engine/src/workflow-node-runners/verification-gate.ts b/packages/engine/src/workflow-node-runners/verification-gate.ts new file mode 100644 index 0000000000..f35ae2d22e --- /dev/null +++ b/packages/engine/src/workflow-node-runners/verification-gate.ts @@ -0,0 +1,77 @@ +import type { Settings, TaskStore, WorkflowIrNode } from "@fusion/core"; +import { runVerificationCommand, truncateWithEllipsis } from "../execution/verification-utils.js"; +import { executorLog } from "../logger.js"; + +export type DeterministicVerificationGateDeps = { + store: TaskStore; + runCommand?: typeof runVerificationCommand; +}; + +export type DeterministicVerificationGateResult = { + outcome: "success" | "failure"; + value: "passed" | "failed" | "no-verification-command-configured" | "verification-infrastructure-failure"; + contextPatch: Record; +}; + +/** + * FNXC:ReviewGatedVerification 2026-08-23-05:02: + * Review-gated Verification is a measurement rather than an agent claim. Its result comes only + * from configured command exit outcomes; absent commands are an explicit failed gate so a task + * cannot acquire green merge evidence without running a real check. + */ +export async function runDeterministicVerificationGate( + deps: DeterministicVerificationGateDeps, + _node: WorkflowIrNode, + task: { id: string }, + settings: Settings, + worktreePath: string, +): Promise { + const commands = [ + { label: "testCommand", command: settings.testCommand?.trim(), type: "test" as const }, + { label: "buildCommand", command: settings.buildCommand?.trim(), type: "build" as const }, + ].filter((item): item is { label: string; command: string; type: "test" | "build" } => Boolean(item.command)); + + if (commands.length === 0) { + return { + outcome: "failure", + value: "no-verification-command-configured", + contextPatch: { output: "no-verification-command-configured" }, + }; + } + + const runCommand = deps.runCommand ?? runVerificationCommand; + for (const item of commands) { + const result = await runCommand( + deps.store, + worktreePath, + task.id, + item.command, + item.type, + undefined, + executorLog, + "executor", + undefined, + settings.verificationCommandTimeoutMs, + ); + if (!result.success) { + const infrastructureReason = result.timedOut + ? "timed-out" + : result.aborted + ? "aborted" + : result.executionError + ? "execution-error" + : undefined; + const output = truncateWithEllipsis([result.stdout, result.stderr].filter(Boolean).join("\n"), 20_000); + return { + outcome: "failure", + value: infrastructureReason ? "verification-infrastructure-failure" : "failed", + contextPatch: { + output: `${item.label}: ${infrastructureReason ?? "non-zero-exit"}${output ? `\n${output}` : ""}`, + verificationFailure: { commandLabel: item.label, ...(infrastructureReason ? { reason: infrastructureReason } : {}) }, + }, + }; + } + } + + return { outcome: "success", value: "passed", contextPatch: { output: "Verification passed." } }; +} diff --git a/packages/engine/src/workflows/workflow-graph-executor.ts b/packages/engine/src/workflows/workflow-graph-executor.ts index e1fc34db0f..001031d3c3 100644 --- a/packages/engine/src/workflows/workflow-graph-executor.ts +++ b/packages/engine/src/workflows/workflow-graph-executor.ts @@ -1316,7 +1316,7 @@ export class WorkflowGraphExecutor { if (!this.shouldTraverseEdge(edge, remediationRouteSource)) return false; const target = nodeMap.get(edge.to); const action = target?.config?.workflowAction; - return action === "plan-replan" || action === "pre-merge-remediation"; + return action === "plan-replan" || action === "pre-merge-remediation" || action === "review-remediation-steps"; }); if (explicitWorkflowRemediationRoute) { return await traverseChildren(node, remediationRouteSource); @@ -1331,7 +1331,7 @@ export class WorkflowGraphExecutor { } const workflowAction = node.config?.workflowAction; - if (workflowAction === "plan-replan" || workflowAction === "pre-merge-remediation") { + if (workflowAction === "plan-replan" || workflowAction === "pre-merge-remediation" || workflowAction === "review-remediation-steps") { const stepId = typeof node.config?.forWorkflowStepId === "string" ? node.config.forWorkflowStepId : undefined;