refactor(FN-7039): delete legacy runWorkflowSteps execution path; graph is sole executor
Removes the legacy workflow-step EXECUTION path now that the graph records results (U2): delete runWorkflowSteps(), the workflow-step seam + runWorkflowStep primitive (runtime-primitives, workflow-node-handlers, authoritative-driver), and the legacy execute() step blocks. Keeps task.workflowStepResults + its store write path (the graph's sink) and executeWorkflowStep/executeScriptWorkflowStep (reused by the graph). - Watchdog recoverCompletedTask now re-enters via maybeExecuteWorkflowGraph so the graph re-runs pending gates, records results, and owns the in-review/back-for-fix transition (KTD-2). - maybeExecuteWorkflowGraph fails CLOSED (parks) when a store lacks getTaskWorkflowSelection AND the task has enabled pre-merge steps — closing the FN-7039 silent-skip class without changing minimal-store implementation runs (KTD-5). KNOWN GAP (follow-up): the FN-4343 per-step workflowStepScopeEnforcement leak check lived only in runWorkflowSteps and is NOT yet replicated on the graph path. Merge-time File Scope enforcement (FileScopeViolationError, squash overlap) is unaffected. Plan U4. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -137,11 +137,11 @@ describe("fast mode workflow/runtime invariants", () => {
|
||||
// called. Fast mode is irrelevant to a bypassed group; the seam is simply gone.
|
||||
it("graph executor with builtin:coding selection bypasses the disabled browser-verification group", async () => {
|
||||
const { executor } = makeExecutorForTask(task({ executionMode: "fast", worktree: "/tmp/wt" }));
|
||||
const runWorkflowSteps = vi.spyOn(executor as any, "runWorkflowSteps").mockResolvedValue(workflowResult());
|
||||
// U4 (KTD-2): runWorkflowSteps + the workflow-step seam were removed; workflow
|
||||
// gates run as graph optional-group nodes only.
|
||||
const seams = {
|
||||
planning: vi.fn(async () => ({ outcome: "success", value: "planned" })),
|
||||
execute: vi.fn(async () => ({ outcome: "success", value: "implemented" })),
|
||||
workflowStep: (executor as any).createAuthoritativeWorkflowSeams({}).workflowStep,
|
||||
review: vi.fn(async () => ({ outcome: "success", value: "approved" })),
|
||||
merge: vi.fn(async () => ({ outcome: "success", value: "merged" })),
|
||||
schedule: vi.fn(async () => ({ outcome: "success", value: "scheduled" })),
|
||||
@@ -161,7 +161,6 @@ describe("fast mode workflow/runtime invariants", () => {
|
||||
expect(result.visitedNodeIds).toContain("browser-verification");
|
||||
expect(result.visitedNodeIds).not.toContain("browser-verification::browser-verification-step");
|
||||
expect(result.visitedNodeIds).not.toContain("workflow-step");
|
||||
expect(runWorkflowSteps).not.toHaveBeenCalled();
|
||||
expect(seams.review).toHaveBeenCalledTimes(1);
|
||||
expect(seams.merge).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
@@ -219,42 +218,11 @@ describe("fast mode workflow/runtime invariants", () => {
|
||||
expect(awaitInput).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it.each([
|
||||
["legacy seam", (executor: TaskExecutor, settings: any) => (executor as any).createAuthoritativeWorkflowSeams(settings).workflowStep(task({ id: "FN-6226" }), {})],
|
||||
["graph primitive", (executor: TaskExecutor, settings: any) => (executor as any).createAuthoritativeWorkflowPrimitives(settings).runWorkflowStep(
|
||||
{ run: { taskId: "FN-6226" }, node: { node: { id: "workflow-step" }, context: {} } },
|
||||
task({ id: "FN-6226" }),
|
||||
{ phase: "pre-merge", worktreePath: "/tmp/wt" },
|
||||
)],
|
||||
])("%s skips pre-merge workflow steps in fast mode", async (_label, invoke) => {
|
||||
const { executor } = makeExecutorForTask(task({ executionMode: "fast", worktree: "/tmp/wt" }));
|
||||
const runWorkflowSteps = vi.spyOn(executor as any, "runWorkflowSteps").mockResolvedValue(workflowResult());
|
||||
|
||||
const result = await invoke(executor, { experimentalFeatures: { workflowGraphExecutor: true } });
|
||||
|
||||
expect(result.outcome).toBe("success");
|
||||
expect(result.value).toBe("workflow-step-skipped");
|
||||
expect(runWorkflowSteps).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it.each([
|
||||
["legacy seam", (executor: TaskExecutor, settings: any) => (executor as any).createAuthoritativeWorkflowSeams(settings).workflowStep(task({ id: "FN-6226" }), {})],
|
||||
["graph primitive", (executor: TaskExecutor, settings: any) => (executor as any).createAuthoritativeWorkflowPrimitives(settings).runWorkflowStep(
|
||||
{ run: { taskId: "FN-6226" }, node: { node: { id: "workflow-step" }, context: {} } },
|
||||
task({ id: "FN-6226" }),
|
||||
{ phase: "pre-merge", worktreePath: "/tmp/wt" },
|
||||
)],
|
||||
])("%s runs pre-merge workflow steps for standard and default execution modes", async (_label, invoke) => {
|
||||
for (const executionMode of ["standard", undefined]) {
|
||||
const { executor } = makeExecutorForTask(task({ executionMode, worktree: "/tmp/wt" }));
|
||||
const runWorkflowSteps = vi.spyOn(executor as any, "runWorkflowSteps").mockResolvedValue(workflowResult());
|
||||
|
||||
const result = await invoke(executor, { experimentalFeatures: { workflowGraphExecutor: true } });
|
||||
|
||||
expect(result.outcome).toBe("success");
|
||||
expect(runWorkflowSteps).toHaveBeenCalledTimes(1);
|
||||
}
|
||||
});
|
||||
// U4 (KTD-2): the legacy `workflow-step` seam and `runWorkflowStep` primitive
|
||||
// were removed, so the two it.each blocks that drove them directly (fast-mode
|
||||
// skip + standard-mode run) are gone. Fast-mode skip of workflow gates is now
|
||||
// covered above by the custom-node tests ("skips custom %s nodes in fast mode")
|
||||
// and by builtin-coding-workflow-step-results.test.ts (graph recording path).
|
||||
|
||||
it("keeps fn_task_done mandatory while excluding fn_review_step in fast mode", async () => {
|
||||
mockedCreateFnAgent.mockImplementation(async (opts: any) => ({
|
||||
|
||||
@@ -2774,78 +2774,11 @@ describe("StepSessionExecutor integration", () => {
|
||||
expect(mockCleanup).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("workflow steps run on success and block on failure", async () => {
|
||||
const store = createStepSessionStore();
|
||||
|
||||
// Enable a workflow step
|
||||
store.getTask.mockResolvedValue({
|
||||
id: "FN-200",
|
||||
title: "Step-session test task",
|
||||
description: "Test task",
|
||||
column: "in-progress",
|
||||
dependencies: [],
|
||||
steps: [
|
||||
{ name: "Step 0", status: "pending" },
|
||||
],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
prompt: "# test\n## Steps\n### Step 0: Preflight\n- [ ] check",
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
baseCommitSha: "abc123",
|
||||
enabledWorkflowSteps: ["WS-001"],
|
||||
});
|
||||
|
||||
store.getWorkflowStep.mockResolvedValue({
|
||||
id: "WS-001",
|
||||
name: "Test Workflow",
|
||||
description: "Test",
|
||||
mode: "script",
|
||||
phase: "pre-merge",
|
||||
scriptName: "test-script",
|
||||
prompt: undefined,
|
||||
enabled: true,
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
});
|
||||
|
||||
// Steps succeed, but workflow step will fail
|
||||
mockExecuteAll.mockResolvedValue([
|
||||
{ stepIndex: 0, success: true, retries: 0 },
|
||||
]);
|
||||
|
||||
const onError = vi.fn();
|
||||
const executor = new TaskExecutor(store, "/tmp/test", { onError });
|
||||
|
||||
// Use fake timers to control the setTimeout in sendTaskBackForFix
|
||||
vi.useFakeTimers();
|
||||
|
||||
// Exhaust retries so workflow step failure is immediate
|
||||
await executor.execute(createTaskWithSteps({ steps: [{ name: "Step 0", status: "pending" }], workflowStepRetries: 3, enabledWorkflowSteps: ["WS-001"] }));
|
||||
|
||||
// Should have called getWorkflowStep to look up the workflow step
|
||||
expect(store.getWorkflowStep).toHaveBeenCalledWith("WS-001");
|
||||
// With script mode and no scripts configured, the step should fail (script not found)
|
||||
// Task should be sent back to in-progress for remediation, NOT call onError
|
||||
expect(store.addTaskComment).toHaveBeenCalledWith(
|
||||
"FN-200",
|
||||
expect.stringContaining("Workflow step failed"),
|
||||
"agent",
|
||||
);
|
||||
// onError should NOT be called (task is being retried, not permanently failed)
|
||||
expect(onError).not.toHaveBeenCalled();
|
||||
|
||||
// Advance timers to trigger the setTimeout that moves task to todo then in-progress
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
|
||||
// Task should move to todo then in-progress (not in-review). The
|
||||
// workflow-rerun bounce flags preserveResumeState so the worktree and
|
||||
// accumulated step progress survive the transient todo state.
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-200", "todo", { preserveResumeState: true, preserveWorktree: true });
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-200", "in-progress");
|
||||
|
||||
vi.useRealTimers();
|
||||
});
|
||||
// U4 (KTD-2): removed "workflow steps run on success and block on failure" —
|
||||
// it drove the deleted legacy runWorkflowSteps loop via execute() on a minimal
|
||||
// store (no getTaskWorkflowSelection). That path now fails closed (KTD-5), and
|
||||
// the run-on-success / block-on-failure behavior is covered through the graph by
|
||||
// builtin-coding-workflow-step-results.test.ts.
|
||||
|
||||
it("onStepStart callback updates step status to in-progress", async () => {
|
||||
const store = createStepSessionStore();
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@@ -1,250 +0,0 @@
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import "./executor-test-helpers.js";
|
||||
import { TaskExecutor } from "../executor.js";
|
||||
import { createMockStore, mockedExecSync, resetExecutorMocks } from "./executor-test-helpers.js";
|
||||
|
||||
function createTask() {
|
||||
return {
|
||||
id: "FN-001",
|
||||
title: "Test",
|
||||
description: "Test task",
|
||||
column: "in-progress" as const,
|
||||
dependencies: [],
|
||||
steps: [{ name: "Preflight", status: "done" as const }],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
enabledWorkflowSteps: ["frontend-ux-design"],
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
};
|
||||
}
|
||||
|
||||
function createWorkflowStep(overrides: Record<string, unknown> = {}) {
|
||||
return {
|
||||
id: "frontend-ux-design",
|
||||
name: "Frontend UX Design",
|
||||
description: "UI review",
|
||||
mode: "prompt",
|
||||
prompt: "Review UI",
|
||||
gateMode: "gate",
|
||||
enabled: true,
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
function mockDiffFiles(files: string[]) {
|
||||
mockedExecSync.mockImplementation((cmd: string | string[]) => {
|
||||
if (typeof cmd === "string" && cmd.includes("git merge-base HEAD origin/main")) {
|
||||
return Buffer.from("abc123\n");
|
||||
}
|
||||
if (typeof cmd === "string" && cmd.includes("git diff --name-only abc123..HEAD")) {
|
||||
return Buffer.from(files.join("\n"));
|
||||
}
|
||||
return Buffer.from("");
|
||||
});
|
||||
}
|
||||
|
||||
function mockDiffSequence(preStepFiles: string[], postStepFiles: string[]) {
|
||||
let diffCallCount = 0;
|
||||
mockedExecSync.mockImplementation((cmd: string | string[]) => {
|
||||
if (typeof cmd === "string" && cmd.includes("git merge-base HEAD origin/main")) {
|
||||
return Buffer.from("abc123\n");
|
||||
}
|
||||
if (typeof cmd === "string" && cmd.includes("git diff --name-only abc123..HEAD")) {
|
||||
diffCallCount += 1;
|
||||
const files = diffCallCount === 1 ? preStepFiles : postStepFiles;
|
||||
return Buffer.from(files.join("\n"));
|
||||
}
|
||||
return Buffer.from("");
|
||||
});
|
||||
}
|
||||
|
||||
describe("executor workflow step scope gating", () => {
|
||||
beforeEach(() => {
|
||||
resetExecutorMocks();
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ name: "both signals empty", diffFiles: [] as string[], declaredFiles: [] as string[], expectedSkip: false },
|
||||
{ name: "diff only non-frontend", diffFiles: ["packages/engine/src/executor.ts"], declaredFiles: [], expectedSkip: true },
|
||||
{
|
||||
name: "declared only non-frontend",
|
||||
diffFiles: [],
|
||||
declaredFiles: [".github/workflows/ci.yml"],
|
||||
expectedSkip: true,
|
||||
expectedLog: "declared File Scope contains no frontend/UI files",
|
||||
},
|
||||
{
|
||||
name: "both present and both non-frontend",
|
||||
diffFiles: ["packages/engine/src/executor.ts"],
|
||||
declaredFiles: [".github/workflows/ci.yml"],
|
||||
expectedSkip: true,
|
||||
expectedLog: "declared File Scope contains no frontend/UI files",
|
||||
},
|
||||
])("FN-4343 auto-skip matrix: $name", async ({ diffFiles, declaredFiles, expectedSkip, expectedLog }) => {
|
||||
const store = createMockStore();
|
||||
const task = createTask();
|
||||
store.getTask.mockResolvedValue(task as any);
|
||||
store.getWorkflowStep.mockResolvedValue(createWorkflowStep() as any);
|
||||
store.parseFileScopeFromPrompt.mockResolvedValue(declaredFiles);
|
||||
mockDiffFiles(diffFiles);
|
||||
|
||||
const executor = new TaskExecutor(store as any, "/tmp/test", {} as any);
|
||||
vi.spyOn(executor as any, "captureModifiedFiles").mockResolvedValue(diffFiles);
|
||||
const executeStepSpy = vi.spyOn(executor as any, "executeWorkflowStep").mockResolvedValue({ success: true, output: "ok" });
|
||||
|
||||
const result = await (executor as any).runWorkflowSteps(task as any, "/tmp/test", {} as any);
|
||||
|
||||
expect(result).toEqual({ allPassed: true });
|
||||
if (expectedSkip) {
|
||||
expect(executeStepSpy).not.toHaveBeenCalled();
|
||||
if (expectedLog) {
|
||||
const logged = store.logEntry.mock.calls.map((call: any[]) => String(call[1] ?? ""));
|
||||
expect(logged.some((line: string) => line.includes(expectedLog))).toBe(true);
|
||||
}
|
||||
} else {
|
||||
expect(executeStepSpy).toHaveBeenCalledTimes(1);
|
||||
}
|
||||
});
|
||||
|
||||
it("passes when prompt-mode pre-merge step writes in-scope files", async () => {
|
||||
const store = createMockStore();
|
||||
const task = { ...createTask(), enabledWorkflowSteps: ["WS-001"] };
|
||||
store.getTask.mockResolvedValue(task as any);
|
||||
store.getWorkflowStep.mockResolvedValue(createWorkflowStep({ id: "WS-001", name: "Workflow Review" }) as any);
|
||||
store.parseFileScopeFromPrompt.mockResolvedValue(["packages/engine/src/executor.ts"]);
|
||||
mockDiffFiles(["packages/engine/src/executor.ts"]);
|
||||
|
||||
const executor = new TaskExecutor(store as any, "/tmp/test", {} as any);
|
||||
vi.spyOn(executor as any, "captureModifiedFiles").mockResolvedValue(["packages/engine/src/executor.ts"]);
|
||||
vi.spyOn(executor as any, "executeWorkflowStep").mockResolvedValue({ success: true, output: "ok" });
|
||||
|
||||
const result = await (executor as any).runWorkflowSteps(task as any, "/tmp/test", { workflowStepScopeEnforcement: "block" } as any);
|
||||
|
||||
expect(result).toEqual({ allPassed: true });
|
||||
});
|
||||
|
||||
it("requests revision in block mode when step writes off-scope files", async () => {
|
||||
const store = createMockStore();
|
||||
const task = { ...createTask(), enabledWorkflowSteps: ["WS-001"] };
|
||||
store.getTask.mockResolvedValue(task as any);
|
||||
store.getWorkflowStep.mockResolvedValue(createWorkflowStep({ id: "WS-001", name: "Workflow Review" }) as any);
|
||||
store.parseFileScopeFromPrompt.mockResolvedValue(["packages/engine/src/executor.ts"]);
|
||||
mockDiffSequence([], ["packages/dashboard/app/components/TaskDetailModal.tsx"]);
|
||||
|
||||
const executor = new TaskExecutor(store as any, "/tmp/test", {} as any);
|
||||
vi.spyOn(executor as any, "captureModifiedFiles")
|
||||
.mockResolvedValueOnce([])
|
||||
.mockResolvedValueOnce(["packages/dashboard/app/components/TaskDetailModal.tsx"]);
|
||||
vi.spyOn(executor as any, "executeWorkflowStep").mockResolvedValue({ success: true, output: "ok" });
|
||||
|
||||
const result = await (executor as any).runWorkflowSteps(task as any, "/tmp/test", { workflowStepScopeEnforcement: "block" } as any);
|
||||
|
||||
expect(result).toEqual(expect.objectContaining({ allPassed: false, revisionRequested: true, stepName: "Workflow Review" }));
|
||||
expect(String((result as any).feedback)).toContain("wrote files outside declared File Scope");
|
||||
});
|
||||
|
||||
it("detects off-scope delta even when pre-step diff has in-scope files", async () => {
|
||||
const store = createMockStore();
|
||||
const task = { ...createTask(), enabledWorkflowSteps: ["WS-001"] };
|
||||
store.getTask.mockResolvedValue(task as any);
|
||||
store.getWorkflowStep.mockResolvedValue(createWorkflowStep({ id: "WS-001", name: "Workflow Review" }) as any);
|
||||
store.parseFileScopeFromPrompt.mockResolvedValue(["packages/engine/src/executor.ts"]);
|
||||
|
||||
mockDiffSequence(
|
||||
["packages/engine/src/executor.ts"],
|
||||
["packages/engine/src/executor.ts", "packages/dashboard/app/components/TaskDetailModal.tsx"],
|
||||
);
|
||||
|
||||
const executor = new TaskExecutor(store as any, "/tmp/test", {} as any);
|
||||
vi.spyOn(executor as any, "captureModifiedFiles")
|
||||
.mockResolvedValueOnce(["packages/engine/src/executor.ts"])
|
||||
.mockResolvedValueOnce(["packages/engine/src/executor.ts", "packages/dashboard/app/components/TaskDetailModal.tsx"]);
|
||||
vi.spyOn(executor as any, "executeWorkflowStep").mockResolvedValue({ success: true, output: "ok" });
|
||||
|
||||
const result = await (executor as any).runWorkflowSteps(task as any, "/tmp/test", { workflowStepScopeEnforcement: "block" } as any);
|
||||
|
||||
expect(result).toEqual(expect.objectContaining({ allPassed: false, revisionRequested: true }));
|
||||
expect(String((result as any).feedback)).toContain("TaskDetailModal.tsx");
|
||||
});
|
||||
|
||||
it("warn mode logs but passes on off-scope writes", async () => {
|
||||
const store = createMockStore();
|
||||
const task = { ...createTask(), enabledWorkflowSteps: ["WS-001"] };
|
||||
store.getTask.mockResolvedValue(task as any);
|
||||
store.getWorkflowStep.mockResolvedValue(createWorkflowStep({ id: "WS-001", name: "Workflow Review" }) as any);
|
||||
store.parseFileScopeFromPrompt.mockResolvedValue(["packages/engine/src/executor.ts"]);
|
||||
mockDiffSequence([], ["packages/dashboard/app/components/TaskDetailModal.tsx"]);
|
||||
|
||||
const executor = new TaskExecutor(store as any, "/tmp/test", {} as any);
|
||||
vi.spyOn(executor as any, "captureModifiedFiles")
|
||||
.mockResolvedValueOnce([])
|
||||
.mockResolvedValueOnce(["packages/dashboard/app/components/TaskDetailModal.tsx"]);
|
||||
vi.spyOn(executor as any, "executeWorkflowStep").mockResolvedValue({ success: true, output: "ok" });
|
||||
|
||||
const result = await (executor as any).runWorkflowSteps(task as any, "/tmp/test", { workflowStepScopeEnforcement: "warn" } as any);
|
||||
|
||||
expect(result).toEqual({ allPassed: true });
|
||||
const logged = store.logEntry.mock.calls.map((call: any[]) => String(call[1] ?? ""));
|
||||
expect(logged.some((line: string) => line.includes("workflowStepScopeEnforcement=warn"))).toBe(true);
|
||||
});
|
||||
|
||||
it("off mode bypasses enforcement", async () => {
|
||||
const store = createMockStore();
|
||||
const task = { ...createTask(), enabledWorkflowSteps: ["WS-001"] };
|
||||
store.getTask.mockResolvedValue(task as any);
|
||||
store.getWorkflowStep.mockResolvedValue(createWorkflowStep({ id: "WS-001", name: "Workflow Review" }) as any);
|
||||
store.parseFileScopeFromPrompt.mockResolvedValue(["packages/engine/src/executor.ts"]);
|
||||
mockDiffFiles(["packages/dashboard/app/components/TaskDetailModal.tsx"]);
|
||||
|
||||
const executor = new TaskExecutor(store as any, "/tmp/test", {} as any);
|
||||
vi.spyOn(executor as any, "executeWorkflowStep").mockResolvedValue({ success: true, output: "ok" });
|
||||
|
||||
const result = await (executor as any).runWorkflowSteps(task as any, "/tmp/test", { workflowStepScopeEnforcement: "off" } as any);
|
||||
|
||||
expect(result).toEqual({ allPassed: true });
|
||||
});
|
||||
|
||||
it("scopeOverride=true bypasses enforcement regardless of mode", async () => {
|
||||
const store = createMockStore();
|
||||
const task = { ...createTask(), enabledWorkflowSteps: ["WS-001"], scopeOverride: true };
|
||||
store.getTask.mockResolvedValue(task as any);
|
||||
store.getWorkflowStep.mockResolvedValue(createWorkflowStep({ id: "WS-001", name: "Workflow Review" }) as any);
|
||||
store.parseFileScopeFromPrompt.mockResolvedValue(["packages/engine/src/executor.ts"]);
|
||||
mockDiffSequence([], ["packages/dashboard/app/components/TaskDetailModal.tsx"]);
|
||||
|
||||
const executor = new TaskExecutor(store as any, "/tmp/test", {} as any);
|
||||
vi.spyOn(executor as any, "executeWorkflowStep").mockResolvedValue({ success: true, output: "ok" });
|
||||
|
||||
const result = await (executor as any).runWorkflowSteps(task as any, "/tmp/test", { workflowStepScopeEnforcement: "block" } as any);
|
||||
|
||||
expect(result).toEqual({ allPassed: true });
|
||||
});
|
||||
|
||||
it("FN-4280 regression: declared workflow-only scope skips Frontend UX without executing agent", async () => {
|
||||
const store = createMockStore();
|
||||
const task = createTask();
|
||||
store.getTask.mockResolvedValue(task as any);
|
||||
store.getWorkflowStep.mockResolvedValue(createWorkflowStep() as any);
|
||||
store.parseFileScopeFromPrompt.mockResolvedValue([
|
||||
".github/workflows/ci.yml",
|
||||
".github/workflows/mobile.yml",
|
||||
".github/workflows/test-release.yml",
|
||||
".github/workflows/release.yml",
|
||||
".github/workflows/version.yml",
|
||||
]);
|
||||
mockDiffFiles([]);
|
||||
|
||||
const executor = new TaskExecutor(store as any, "/tmp/test", {} as any);
|
||||
const executeStepSpy = vi.spyOn(executor as any, "executeWorkflowStep").mockResolvedValue({ success: true, output: "ok" });
|
||||
|
||||
const result = await (executor as any).runWorkflowSteps(task as any, "/tmp/test", {} as any);
|
||||
|
||||
expect(result).toEqual({ allPassed: true });
|
||||
expect(executeStepSpy).not.toHaveBeenCalled();
|
||||
const logged = store.logEntry.mock.calls.map((call: any[]) => String(call[1] ?? ""));
|
||||
expect(logged.some((line: string) => line.includes("declared File Scope contains no frontend/UI files"))).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -791,71 +791,75 @@ describe("In-progress task resume after restart", () => {
|
||||
expect(store.logEntry).not.toHaveBeenCalledWith("FN-1473", "Resumed after engine restart");
|
||||
});
|
||||
|
||||
it("recoverCompletedTask() marks task failed then moves to in-review when workflow fails", async () => {
|
||||
// U4 (KTD-2/KTD-5): the legacy `runWorkflowSteps` recovery path was deleted.
|
||||
// recoverCompletedTask now RE-ENTERS the workflow graph (maybeExecuteWorkflowGraph),
|
||||
// which records workflowStepResults and OWNS the in-review / back-for-fix
|
||||
// transition. These two tests replace the old "runWorkflowSteps fails → bounce"
|
||||
// test: one proves the graph re-entry seam, one proves the fail-closed guard.
|
||||
it("recoverCompletedTask() re-enters the workflow graph (records results + owns the transition)", async () => {
|
||||
const store = createMockStore();
|
||||
const task = makeTask("FN-963", "in-progress", {
|
||||
worktree: "/tmp/wt/FN-963",
|
||||
steps: makeSteps("done"),
|
||||
});
|
||||
|
||||
const executor = new TaskExecutor(store, "/tmp/test");
|
||||
vi.spyOn(executor as any, "captureModifiedFiles").mockResolvedValue([]);
|
||||
// The graph is the sole executor; recovery delegates to it. Spy the graph
|
||||
// entry to assert the re-entry seam without standing up a full graph runner
|
||||
// (the graph's own recording/transition behavior is covered by
|
||||
// builtin-coding-workflow-step-results.test.ts and the cutover backstop).
|
||||
const graphEntry = vi
|
||||
.spyOn(executor as any, "maybeExecuteWorkflowGraph")
|
||||
.mockResolvedValue(true);
|
||||
|
||||
const recovered = await executor.recoverCompletedTask(task);
|
||||
|
||||
expect(recovered).toBe(true);
|
||||
expect(graphEntry).toHaveBeenCalledTimes(1);
|
||||
expect(graphEntry).toHaveBeenCalledWith(task);
|
||||
// The graph owns the transition — recovery must NOT itself bounce or hand off.
|
||||
expect(store.moveTask).not.toHaveBeenCalledWith("FN-963", "in-review");
|
||||
});
|
||||
|
||||
it("recoverCompletedTask() fails closed (KTD-5) when the store lacks getTaskWorkflowSelection and the task has enabled workflow steps", async () => {
|
||||
// createMockStore does NOT expose getTaskWorkflowSelection, so the workflow
|
||||
// graph cannot resolve a selection — and the legacy runWorkflowSteps path was
|
||||
// removed (U4). A task with an enabled pre-merge step MUST fail closed rather
|
||||
// than silently hand off to review with no gate execution (the FN-7039 class).
|
||||
const store = createMockStore({
|
||||
getTask: vi.fn().mockResolvedValue(makeTaskDetail("FN-963", "in-progress", {
|
||||
worktree: "/tmp/wt/FN-963",
|
||||
steps: makeSteps("done"),
|
||||
enabledWorkflowSteps: ["wf-1"],
|
||||
})),
|
||||
getWorkflowStep: vi.fn().mockResolvedValue({
|
||||
id: "wf-1",
|
||||
name: "Build",
|
||||
mode: "script",
|
||||
scriptName: "pnpm test",
|
||||
phase: "pre-merge",
|
||||
}),
|
||||
});
|
||||
const task = makeTask("FN-963", "in-progress", {
|
||||
worktree: "/tmp/wt/FN-963",
|
||||
steps: makeSteps("done"),
|
||||
enabledWorkflowSteps: ["wf-1"],
|
||||
});
|
||||
|
||||
mockedExecSync.mockImplementation((command) => {
|
||||
const cmd = String(command);
|
||||
if (cmd === "pnpm test") {
|
||||
throw new Error("tests failed");
|
||||
}
|
||||
return "" as any;
|
||||
});
|
||||
|
||||
// Use fake timers to control the setTimeout in sendTaskBackForFix
|
||||
vi.useFakeTimers();
|
||||
|
||||
const executor = new TaskExecutor(store, "/tmp/test");
|
||||
vi.spyOn(executor as any, "captureModifiedFiles").mockResolvedValue([]);
|
||||
// Spy handleGraphFailure (the park-the-task seam) to assert the fail-closed
|
||||
// branch fired with a clear reason — i.e. NOT a silent no-op.
|
||||
const handleGraphFailure = vi
|
||||
.spyOn(executor as any, "handleGraphFailure")
|
||||
.mockResolvedValue(undefined);
|
||||
|
||||
const recovered = await executor.recoverCompletedTask(task);
|
||||
|
||||
expect(recovered).toBe(true);
|
||||
// Task should be cleared and reset for retry (not failed + in-review)
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-963", {
|
||||
status: null,
|
||||
error: null,
|
||||
sessionFile: null,
|
||||
workflowStepRetries: 0,
|
||||
});
|
||||
// Should add a comment with failure feedback
|
||||
expect(store.addTaskComment).toHaveBeenCalledWith(
|
||||
"FN-963",
|
||||
expect.stringContaining("Workflow step failed during recovery"),
|
||||
"agent",
|
||||
);
|
||||
// Should reset all steps to pending
|
||||
expect(store.updateStep).toHaveBeenCalledWith("FN-963", 0, "pending");
|
||||
|
||||
// Advance timers to trigger the setTimeout that moves task to todo then in-progress
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
|
||||
// Task should move to todo then in-progress (not in-review). The
|
||||
// workflow-rerun bounce passes `preserveWorktree: true` so the
|
||||
// checkout doesn't briefly disappear during the hop.
|
||||
expect(store.moveTask).toHaveBeenCalledWith(
|
||||
"FN-963",
|
||||
"todo",
|
||||
expect.objectContaining({ preserveWorktree: true }),
|
||||
);
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-963", "in-progress");
|
||||
|
||||
vi.useRealTimers();
|
||||
expect(handleGraphFailure).toHaveBeenCalledTimes(1);
|
||||
const failureArg = (handleGraphFailure.mock.calls[0] as unknown[])[1] as {
|
||||
disposition: string;
|
||||
reason: string;
|
||||
};
|
||||
expect(failureArg.disposition).toBe("failed");
|
||||
expect(String(failureArg.reason)).toContain("workflow-selection-api-unavailable");
|
||||
// Must NOT silently finalize to review as a success.
|
||||
expect(store.moveTask).not.toHaveBeenCalledWith("FN-963", "in-review");
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -0,0 +1,50 @@
|
||||
// FNXC:WorkflowExecution 2026-06-25-00:00: U4 (KTD-2) grep-guard.
|
||||
// The legacy `runWorkflowSteps` execution path + the `workflow-step` seam /
|
||||
// `runWorkflowStep` primitive were removed; the workflow graph is the sole
|
||||
// workflow-step executor (results recorded into task.workflowStepResults, U2).
|
||||
// This test fails loudly if a production caller of the deleted runner is
|
||||
// reintroduced, or if the removed seam/primitive handlers come back.
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { readFileSync, readdirSync } from "node:fs";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { dirname, join } from "node:path";
|
||||
|
||||
const srcDir = join(dirname(fileURLToPath(import.meta.url)), "..");
|
||||
|
||||
function listProductionTsFiles(dir: string): string[] {
|
||||
const out: string[] = [];
|
||||
for (const entry of readdirSync(dir, { withFileTypes: true })) {
|
||||
if (entry.name === "__tests__") continue;
|
||||
const full = join(dir, entry.name);
|
||||
if (entry.isDirectory()) out.push(...listProductionTsFiles(full));
|
||||
else if (entry.name.endsWith(".ts") && !entry.name.endsWith(".d.ts")) out.push(full);
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
describe("U4: legacy workflow-step execution path removed", () => {
|
||||
const files = listProductionTsFiles(srcDir);
|
||||
|
||||
it("has no production caller of the deleted runWorkflowSteps runner", () => {
|
||||
const offenders: string[] = [];
|
||||
for (const file of files) {
|
||||
const text = readFileSync(file, "utf8");
|
||||
// Match an actual call/seam-key, not the FNXC comments that reference the name.
|
||||
if (/this\.runWorkflowSteps\s*\(|[^.\w]runWorkflowStep\s*:/.test(text)) {
|
||||
offenders.push(file.replace(srcDir, "@fusion/engine/src"));
|
||||
}
|
||||
}
|
||||
expect(offenders, `unexpected runWorkflowSteps/runWorkflowStep usage in ${offenders.join(", ")}`).toEqual([]);
|
||||
});
|
||||
|
||||
it("no production code declares a workflow-step seam handler", () => {
|
||||
const offenders: string[] = [];
|
||||
for (const file of files) {
|
||||
const text = readFileSync(file, "utf8");
|
||||
if (/workflowStep\s*:\s*async|workflowStep\?\s*:/.test(text)) {
|
||||
offenders.push(file.replace(srcDir, "@fusion/engine/src"));
|
||||
}
|
||||
}
|
||||
expect(offenders, `unexpected workflowStep seam handler in ${offenders.join(", ")}`).toEqual([]);
|
||||
});
|
||||
});
|
||||
@@ -3714,22 +3714,42 @@ export class TaskExecutor {
|
||||
|
||||
// Run workflow steps before transitioning — skip in fast mode
|
||||
if (task.executionMode !== "fast") {
|
||||
if (await this.shouldDeferCompletionForGlobalPause(task.id, "before workflow steps during completed-task recovery")) {
|
||||
if (await this.shouldDeferCompletionForGlobalPause(task.id, "before workflow-graph re-entry during completed-task recovery")) {
|
||||
return false;
|
||||
}
|
||||
const workflowResult = await this.runWorkflowSteps(task, task.worktree, settings, undefined);
|
||||
if (workflowResult === "deferred-paused") {
|
||||
if (this.pausedAborted.has(task.id)) {
|
||||
this.clearPausedAborted(task.id);
|
||||
}
|
||||
return false;
|
||||
}
|
||||
if (!workflowResult.allPassed) {
|
||||
// For recovery path, treat any failure (including revision) as hard failure
|
||||
// Send back to in-progress so executor can attempt to fix the issues
|
||||
await this.sendTaskBackForFix(task, task.worktree!, workflowResult.feedback, workflowResult.stepName || "Unknown", "Workflow step failed during recovery", false);
|
||||
return true; // Still transitioned out of in-progress
|
||||
/*
|
||||
FNXC:WorkflowExecution 2026-06-25-00:00:
|
||||
U4 (KTD-2) watchdog re-entry. The legacy `runWorkflowSteps` recovery path
|
||||
was deleted; the workflow graph is the sole executor. A stranded completed
|
||||
task is recovered by RE-ENTERING the graph via `maybeExecuteWorkflowGraph`
|
||||
(the same entry execute() uses), which: (1) re-runs any pending
|
||||
optional-group / gate nodes, (2) records their outcomes into
|
||||
`task.workflowStepResults` (U2) and emits the `[pre-merge]` logs, and
|
||||
(3) OWNS the in-review vs back-for-fix transition. The graph's execute seam
|
||||
registers the normal completion interceptor, so a task whose implementation
|
||||
already completed resumes at the post-implementation nodes (it does not
|
||||
re-run the agent from scratch). RECOVERY POLICY mapping (per plan U4): the
|
||||
old "any failure including REVISE is hard" recovery rule now maps onto the
|
||||
graph's gate semantics — a GATE node REVISE/failure routes the task back for
|
||||
fix, while an ADVISORY REVISE is non-blocking and proceeds to review. KTD-5:
|
||||
for a store lacking `getTaskWorkflowSelection` that has enabled steps,
|
||||
`maybeExecuteWorkflowGraph` itself fails closed (parks) rather than letting
|
||||
recovery silently skip the gates.
|
||||
*/
|
||||
const graphOwned = await this.maybeExecuteWorkflowGraph(task);
|
||||
if (graphOwned) {
|
||||
this.clearCompletedTaskWatchdog(task.id);
|
||||
await this.store.logEntry(
|
||||
task.id,
|
||||
`Auto-recovered: stranded completed task re-dispatched through the workflow graph — the graph re-ran pending workflow steps (recording results) and owns the in-review / back-for-fix transition`,
|
||||
).catch(() => undefined);
|
||||
executorLog.log(`✓ ${task.id} auto-recovered completed task via workflow-graph re-entry`);
|
||||
return true;
|
||||
}
|
||||
// Graph declined (minimal store WITHOUT the workflow-selection API and no
|
||||
// enabled gates to run — a store WITH enabled steps would have been parked
|
||||
// fail-closed above): there is nothing to gate, so fall through to the
|
||||
// legacy in-review handoff below.
|
||||
} else {
|
||||
executorLog.log(`${task.id}: fast mode — skipping workflow steps on auto-recovery`);
|
||||
}
|
||||
@@ -4213,7 +4233,29 @@ export class TaskExecutor {
|
||||
/*
|
||||
FNXC:WorkflowExecution 2026-06-23-22:01:
|
||||
Graph execution is the default for production TaskStore implementations, which expose workflow-selection APIs. Minimal test stores and older embedded adapters can lack that API; fall back to the legacy executor instead of half-entering graph routing with no workflow persistence surface.
|
||||
|
||||
FNXC:WorkflowExecution 2026-06-25-00:00:
|
||||
U4 (KTD-2/KTD-5) FAIL-CLOSED. The legacy `runWorkflowSteps` execution path was deleted; the graph is now the sole workflow-step executor. A store without `getTaskWorkflowSelection` can no longer reach a legacy executor that runs the enabled pre-merge gates. If we returned `false` here for a task that has enabled workflow steps, execute() would proceed and SILENTLY SKIP every gate (the exact FN-7039 silent-skip class) before handing off to review. So when the task has enabled pre-merge workflow steps (and is not fast mode, which intentionally skips them), park the task as a workflow failure instead — loud, never silent. Tasks with NO enabled steps have nothing to gate, so they keep the legacy implementation path (no behavior change), which is what minimal test stores exercise.
|
||||
*/
|
||||
let liveForGate: Task | null = null;
|
||||
try {
|
||||
liveForGate = await this.store.getTask(task.id);
|
||||
} catch {
|
||||
liveForGate = null;
|
||||
}
|
||||
const gateTask = liveForGate ?? task;
|
||||
const hasEnabledSteps = (gateTask.enabledWorkflowSteps?.length ?? 0) > 0;
|
||||
if (hasEnabledSteps && gateTask.executionMode !== "fast") {
|
||||
await this.handleGraphFailure(task, {
|
||||
disposition: "failed",
|
||||
outcome: "failure",
|
||||
reason:
|
||||
"workflow-selection-api-unavailable: store lacks getTaskWorkflowSelection so the workflow graph cannot run "
|
||||
+ `${gateTask.enabledWorkflowSteps?.length ?? 0} enabled pre-merge workflow step(s); the legacy runWorkflowSteps path was removed (U4). Failing closed rather than skipping gates (KTD-5).`,
|
||||
visitedNodeIds: [],
|
||||
});
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
try {
|
||||
@@ -5412,69 +5454,12 @@ export class TaskExecutor {
|
||||
runVerification: async () => ({ outcome: "success", value: "verification-skipped", data: {
|
||||
verdict: "skipped",
|
||||
} }),
|
||||
runWorkflowStep: async (_ctx, task, input) => {
|
||||
if (input.phase !== "pre-merge") {
|
||||
return { outcome: "success", value: "workflow-step-skipped", data: { allPassed: true } };
|
||||
}
|
||||
const live = await this.store.getTask(task.id);
|
||||
if (live.executionMode === "fast") {
|
||||
executorLog.log(`${task.id}: fast mode — skipping pre-merge workflow steps`);
|
||||
await this.store.logEntry(task.id, "Fast mode — pre-merge workflow steps skipped", undefined, this.getRunContextFor(task.id));
|
||||
return { outcome: "success", value: "workflow-step-skipped", data: { allPassed: true } };
|
||||
}
|
||||
if (await this.shouldDeferCompletionForGlobalPause(task.id, "before workflow steps after task completion")) {
|
||||
return { outcome: "success", value: "deferred-paused", data: { allPassed: false } };
|
||||
}
|
||||
const worktreePath = input.worktreePath || live.worktree || this.rootDir;
|
||||
const workflowResult = await this.runWorkflowSteps(live, worktreePath, settings, undefined);
|
||||
if (workflowResult === "deferred-paused") {
|
||||
if (await this.parkTaskAfterWorkflowStepPause(task.id)) {
|
||||
this.clearPausedAborted(task.id);
|
||||
} else if (this.pausedAborted.has(task.id)) {
|
||||
this.clearPausedAborted(task.id);
|
||||
}
|
||||
return { outcome: "success", value: "deferred-paused", data: { allPassed: false } };
|
||||
}
|
||||
if (!workflowResult.allPassed) {
|
||||
const feedback = workflowResult.feedback || "Workflow step failed";
|
||||
const stepName = workflowResult.stepName || "Unknown";
|
||||
if (workflowResult.revisionRequested) {
|
||||
const rerunScheduled = await this.handleWorkflowRevisionRequest(
|
||||
live,
|
||||
worktreePath,
|
||||
feedback,
|
||||
stepName,
|
||||
settings,
|
||||
);
|
||||
if (!rerunScheduled) {
|
||||
return {
|
||||
outcome: "failure",
|
||||
value: "workflow-step-revision-unhandled",
|
||||
data: workflowResult,
|
||||
};
|
||||
}
|
||||
} else {
|
||||
const retried = await this.handleWorkflowStepFailure(
|
||||
live,
|
||||
worktreePath,
|
||||
feedback,
|
||||
stepName,
|
||||
);
|
||||
if (!retried) {
|
||||
await this.sendTaskBackForFix(
|
||||
live,
|
||||
worktreePath,
|
||||
feedback,
|
||||
stepName,
|
||||
"Workflow step failed",
|
||||
);
|
||||
}
|
||||
}
|
||||
return { outcome: "success", value: "remediation-scheduled", data: workflowResult };
|
||||
}
|
||||
await this.store.updateTask(task.id, { workflowStepRetries: undefined, taskDoneRetryCount: null });
|
||||
return { outcome: "success", value: "workflow-steps-passed", data: workflowResult };
|
||||
},
|
||||
// FNXC:WorkflowExecution 2026-06-25-00:00: U4 (KTD-2) — the legacy
|
||||
// `runWorkflowStep` primitive + the `workflow-step` seam it served were
|
||||
// removed. Workflow quality gates run as the graph's own optional-group /
|
||||
// gate nodes (builtin:coding already routes through them), which record
|
||||
// results into `task.workflowStepResults` directly (U2). No `runWorkflowStep`
|
||||
// primitive remains in `WorkflowRuntimePrimitives`.
|
||||
updateSteps: async (_ctx, task, steps) => {
|
||||
await this.store.updateTask(task.id, { steps });
|
||||
return { outcome: "success", value: "steps-updated", data: { count: steps.length } };
|
||||
@@ -5584,58 +5569,14 @@ export class TaskExecutor {
|
||||
value: paused ? "implementation-paused" : "implementation-incomplete",
|
||||
};
|
||||
},
|
||||
workflowStep: async (seamTask) => {
|
||||
const live = await this.store.getTask(seamTask.id);
|
||||
if (live.executionMode === "fast") {
|
||||
executorLog.log(`${seamTask.id}: fast mode — skipping pre-merge workflow steps`);
|
||||
await this.store.logEntry(seamTask.id, "Fast mode — pre-merge workflow steps skipped", undefined, this.getRunContextFor(seamTask.id));
|
||||
return { outcome: "success", value: "workflow-step-skipped" };
|
||||
}
|
||||
const worktreePath = live.worktree || this.rootDir;
|
||||
const settings = await this.store.getSettings();
|
||||
const workflowResult = await this.runWorkflowSteps(live, worktreePath, settings, undefined);
|
||||
if (workflowResult === "deferred-paused") {
|
||||
if (await this.parkTaskAfterWorkflowStepPause(seamTask.id)) {
|
||||
this.clearPausedAborted(seamTask.id);
|
||||
} else if (this.pausedAborted.has(seamTask.id)) {
|
||||
this.clearPausedAborted(seamTask.id);
|
||||
}
|
||||
return { outcome: "success", value: "deferred-paused" };
|
||||
}
|
||||
if (!workflowResult.allPassed) {
|
||||
const feedback = workflowResult.feedback || "Workflow step failed";
|
||||
const stepName = workflowResult.stepName || "Unknown";
|
||||
if (workflowResult.revisionRequested) {
|
||||
const rerunScheduled = await this.handleWorkflowRevisionRequest(
|
||||
live,
|
||||
worktreePath,
|
||||
feedback,
|
||||
stepName,
|
||||
settings,
|
||||
);
|
||||
if (!rerunScheduled) return { outcome: "failure", value: "workflow-step-revision-unhandled" };
|
||||
} else {
|
||||
const retried = await this.handleWorkflowStepFailure(
|
||||
live,
|
||||
worktreePath,
|
||||
feedback,
|
||||
stepName,
|
||||
);
|
||||
if (!retried) {
|
||||
await this.sendTaskBackForFix(
|
||||
live,
|
||||
worktreePath,
|
||||
feedback,
|
||||
stepName,
|
||||
"Workflow step failed",
|
||||
);
|
||||
}
|
||||
}
|
||||
return { outcome: "success", value: "remediation-scheduled" };
|
||||
}
|
||||
await this.store.updateTask(seamTask.id, { workflowStepRetries: undefined, taskDoneRetryCount: null });
|
||||
return { outcome: "success", value: "workflow-steps-passed" };
|
||||
},
|
||||
// FNXC:WorkflowExecution 2026-06-25-00:00: U4 (KTD-2) — the legacy
|
||||
// `workflowStep` seam was removed. Workflow quality gates run as the graph's
|
||||
// own optional-group / gate nodes (builtin:coding replaced its `workflow-step`
|
||||
// seam node with optional-group nodes) which record into
|
||||
// `task.workflowStepResults` (U2). `WorkflowLegacySeams.workflowStep` no
|
||||
// longer exists, and `resolveSeamName` no longer recognizes the
|
||||
// `workflow-step` seam (an IR node still declaring it now fails loudly via
|
||||
// WorkflowIrError rather than silently no-opping).
|
||||
review: async (seamTask) => {
|
||||
// The legacy "review" stage is the in-review handoff: per-step AI review
|
||||
// already ran during implementation (fn_review_step), and the in-review
|
||||
@@ -8242,38 +8183,25 @@ export class TaskExecutor {
|
||||
}
|
||||
}
|
||||
|
||||
// Run workflow steps before moving to in-review — skip in fast mode
|
||||
if (executionMode !== "fast") {
|
||||
const workflowResult = await this.runWorkflowSteps(task, worktreePath, settings, taskEnv);
|
||||
if (workflowResult === "deferred-paused") {
|
||||
if (await this.parkTaskAfterWorkflowStepPause(task.id)) {
|
||||
this.clearPausedAborted(task.id);
|
||||
return;
|
||||
}
|
||||
if (this.pausedAborted.has(task.id)) {
|
||||
this.clearPausedAborted(task.id);
|
||||
}
|
||||
return;
|
||||
}
|
||||
if (!workflowResult.allPassed) {
|
||||
// Check if revision was requested
|
||||
if (workflowResult.revisionRequested) {
|
||||
const rerunScheduled = await this.handleWorkflowRevisionRequest(task, worktreePath, workflowResult.feedback, workflowResult.stepName, settings);
|
||||
if (rerunScheduled) {
|
||||
return;
|
||||
}
|
||||
} else {
|
||||
// Try to fix workflow step failures with retries
|
||||
const retried = await this.handleWorkflowStepFailure(task, worktreePath, workflowResult.feedback, workflowResult.stepName || "Unknown");
|
||||
if (retried) {
|
||||
return; // Retry scheduled
|
||||
}
|
||||
// Retries exhausted - send back to in-progress for remediation
|
||||
await this.sendTaskBackForFix(task, worktreePath, workflowResult.feedback, workflowResult.stepName || "Unknown", "Workflow step failed");
|
||||
return;
|
||||
}
|
||||
}
|
||||
} else {
|
||||
// FNXC:WorkflowExecution 2026-06-25-00:00: U4 (KTD-2/KTD-5) — workflow
|
||||
// steps are graph-owned. For a graph-driven run the execute seam
|
||||
// registered a completion interceptor; stop at the
|
||||
// implementation-complete boundary and hand the remaining lifecycle
|
||||
// (workflow gates → review → merge) back to the graph runner, which
|
||||
// records results into task.workflowStepResults (U2). The legacy
|
||||
// runWorkflowSteps loop was deleted. A NON-graph run reaching here has no
|
||||
// enabled workflow steps to run (a minimal store WITH enabled steps is
|
||||
// parked fail-closed inside maybeExecuteWorkflowGraph, KTD-5), so there
|
||||
// is nothing to gate before the in-review handoff.
|
||||
const graphCompletion = this.graphCompletionInterceptors.get(task.id);
|
||||
if (graphCompletion) {
|
||||
this.clearCompletedTaskWatchdog(task.id);
|
||||
executorLog.log(`✓ ${task.id} implementation complete — graph interpreter owns the remaining lifecycle`);
|
||||
const liveModified = (await this.store.getTask(task.id).catch(() => task)).modifiedFiles ?? [];
|
||||
graphCompletion({ modifiedFiles: liveModified });
|
||||
return;
|
||||
}
|
||||
if (executionMode === "fast") {
|
||||
executorLog.log(`${task.id}: fast mode — skipping pre-merge workflow steps`);
|
||||
await this.store.logEntry(task.id, "Fast mode — pre-merge workflow steps skipped", undefined, this.getRunContextFor(task.id));
|
||||
}
|
||||
@@ -9068,51 +8996,25 @@ export class TaskExecutor {
|
||||
}
|
||||
|
||||
this.scheduleCompletedTaskWatchdog(task.id, "task completion");
|
||||
if (await this.shouldDeferCompletionForGlobalPause(task.id, "before workflow steps after task completion")) {
|
||||
if (await this.shouldDeferCompletionForGlobalPause(task.id, "before in-review transition after task completion")) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Run workflow steps before moving to in-review — skip in fast mode
|
||||
if (executionMode !== "fast") {
|
||||
const workflowResult = await this.runWorkflowSteps(task, worktreePath, settings, taskEnv);
|
||||
if (workflowResult === "deferred-paused") {
|
||||
if (await this.parkTaskAfterWorkflowStepPause(task.id)) {
|
||||
this.clearPausedAborted(task.id);
|
||||
wasPaused = true;
|
||||
return;
|
||||
}
|
||||
if (this.pausedAborted.has(task.id)) {
|
||||
this.clearPausedAborted(task.id);
|
||||
wasPaused = true;
|
||||
}
|
||||
return;
|
||||
}
|
||||
if (!workflowResult.allPassed) {
|
||||
// Check if revision was requested
|
||||
if (workflowResult.revisionRequested) {
|
||||
const rerunScheduled = await this.handleWorkflowRevisionRequest(task, worktreePath, workflowResult.feedback, workflowResult.stepName, settings);
|
||||
if (rerunScheduled) {
|
||||
return;
|
||||
}
|
||||
} else {
|
||||
// Try to fix workflow step failures with retries
|
||||
const retried = await this.handleWorkflowStepFailure(task, worktreePath, workflowResult.feedback, workflowResult.stepName || "Unknown");
|
||||
if (retried) {
|
||||
return; // Retry scheduled
|
||||
}
|
||||
// Retries exhausted - send back to in-progress for remediation
|
||||
await this.sendTaskBackForFix(task, worktreePath, workflowResult.feedback, workflowResult.stepName || "Unknown", "Workflow step failed");
|
||||
return;
|
||||
}
|
||||
}
|
||||
} else {
|
||||
// FNXC:WorkflowExecution 2026-06-25-00:00: U4 (KTD-2/KTD-5) — the legacy
|
||||
// runWorkflowSteps loop was deleted; workflow gates are graph-owned and
|
||||
// record into task.workflowStepResults (U2). The graph-interceptor
|
||||
// short-circuit above already returns for every graph-driven run, so a
|
||||
// run reaching here is a non-graph fallback with NO enabled workflow
|
||||
// steps (a minimal store WITH enabled steps is parked fail-closed in
|
||||
// maybeExecuteWorkflowGraph, KTD-5) — nothing to gate before handoff.
|
||||
if (executionMode === "fast") {
|
||||
executorLog.log(`${task.id}: fast mode — skipping pre-merge workflow steps`);
|
||||
await this.store.logEntry(task.id, "Fast mode — pre-merge workflow steps skipped", undefined, this.getRunContextFor(task.id));
|
||||
}
|
||||
|
||||
// Reset retry counters on success
|
||||
await this.store.updateTask(task.id, { workflowStepRetries: undefined, taskDoneRetryCount: null });
|
||||
if (await this.shouldDeferCompletionForGlobalPause(task.id, "before in-review transition after task completion")) {
|
||||
if (await this.shouldDeferCompletionForGlobalPause(task.id, "before in-review transition after task completion (post-reset)")) {
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -9344,37 +9246,26 @@ export class TaskExecutor {
|
||||
}
|
||||
|
||||
this.scheduleCompletedTaskWatchdog(task.id, "task completion retry");
|
||||
if (await this.shouldDeferCompletionForGlobalPause(task.id, "before workflow steps after task completion retry")) {
|
||||
if (await this.shouldDeferCompletionForGlobalPause(task.id, "before in-review transition after task completion retry")) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Run workflow steps before moving to in-review — skip in fast mode
|
||||
if (executionMode !== "fast") {
|
||||
const workflowResult = await this.runWorkflowSteps(task, worktreePath, settings, taskEnv);
|
||||
if (workflowResult === "deferred-paused") {
|
||||
if (await this.parkTaskAfterWorkflowStepPause(task.id)) {
|
||||
this.clearPausedAborted(task.id);
|
||||
wasPaused = true;
|
||||
return;
|
||||
}
|
||||
if (this.pausedAborted.has(task.id)) {
|
||||
this.clearPausedAborted(task.id);
|
||||
wasPaused = true;
|
||||
}
|
||||
return;
|
||||
}
|
||||
if (!workflowResult.allPassed) {
|
||||
if (workflowResult.revisionRequested) {
|
||||
const rerunScheduled = await this.handleWorkflowRevisionRequest(task, worktreePath, workflowResult.feedback, workflowResult.stepName, settings);
|
||||
if (rerunScheduled) {
|
||||
return;
|
||||
}
|
||||
} else {
|
||||
await this.sendTaskBackForFix(task, worktreePath, workflowResult.feedback, workflowResult.stepName || "Unknown", "Workflow step failed on retry");
|
||||
return;
|
||||
}
|
||||
}
|
||||
} else {
|
||||
// FNXC:WorkflowExecution 2026-06-25-00:00: U4 (KTD-2/KTD-5) — workflow
|
||||
// gates are graph-owned (record into task.workflowStepResults, U2); the
|
||||
// legacy runWorkflowSteps loop was deleted. For a graph-driven run the
|
||||
// execute seam registered a completion interceptor, so stop at the
|
||||
// implementation boundary and let the graph own the remaining
|
||||
// lifecycle. A non-graph fallback reaching here has NO enabled workflow
|
||||
// steps (a minimal store WITH enabled steps is parked fail-closed in
|
||||
// maybeExecuteWorkflowGraph, KTD-5) — nothing to gate before handoff.
|
||||
const graphCompletion = this.graphCompletionInterceptors.get(task.id);
|
||||
if (graphCompletion) {
|
||||
this.clearCompletedTaskWatchdog(task.id);
|
||||
executorLog.log(`✓ ${task.id} implementation complete (retry) — graph interpreter owns the remaining lifecycle`);
|
||||
graphCompletion({ modifiedFiles });
|
||||
return;
|
||||
}
|
||||
if (executionMode === "fast") {
|
||||
executorLog.log(`${task.id}: fast mode — skipping pre-merge workflow steps`);
|
||||
await this.store.logEntry(task.id, "Fast mode — pre-merge workflow steps skipped", undefined, this.getRunContextFor(task.id));
|
||||
}
|
||||
@@ -12893,326 +12784,6 @@ ${failureFeedback}
|
||||
* @param startPoint — Optional git ref to branch from (e.g., `fusion/fn-041`).
|
||||
* When provided, the worktree starts from that ref instead of HEAD.
|
||||
*/
|
||||
/**
|
||||
* Run workflow step agents sequentially after main task execution completes.
|
||||
* Each workflow step spawns a separate agent with the step's prompt.
|
||||
* Returns structured result: all passed, all passed (true), failed (false), or revision requested.
|
||||
*/
|
||||
private async runWorkflowSteps(
|
||||
task: Task,
|
||||
worktreePath: string,
|
||||
settings: Settings,
|
||||
taskEnv?: NodeJS.ProcessEnv,
|
||||
): Promise<WorkflowStepResult | "deferred-paused"> {
|
||||
await this.auditReadonlyWorkflowStepPromptsOnce(task.id);
|
||||
// Check if task has enabled workflow steps
|
||||
const currentTask = await this.store.getTask(task.id);
|
||||
if (!currentTask.enabledWorkflowSteps?.length) return { allPassed: true };
|
||||
|
||||
const workflowStepIds = currentTask.enabledWorkflowSteps;
|
||||
const results: import("@fusion/core").WorkflowStepResult[] = [];
|
||||
|
||||
for (const wsId of workflowStepIds) {
|
||||
const ws = await this.store.getWorkflowStep(wsId);
|
||||
if (!ws) {
|
||||
await this.store.logEntry(task.id, `[pre-merge] Workflow step ${wsId} not found — skipping`);
|
||||
results.push({
|
||||
workflowStepId: wsId,
|
||||
workflowStepName: "Unknown",
|
||||
phase: "pre-merge",
|
||||
status: "skipped",
|
||||
output: "Workflow step definition not found",
|
||||
});
|
||||
await this.store.updateTask(task.id, { workflowStepResults: results });
|
||||
continue;
|
||||
}
|
||||
|
||||
// Normalize legacy steps: undefined phase → "pre-merge"
|
||||
const stepPhase = ws.phase || "pre-merge";
|
||||
|
||||
// readonly review steps always run pre-merge to reuse the coding worktree — see FN-2185 post-mortem.
|
||||
// Skip non-readonly post-merge steps — those run in the merger after merge.
|
||||
if (stepPhase === "post-merge" && ws.toolMode !== "readonly") continue;
|
||||
|
||||
// Normalize legacy steps without mode to prompt-mode
|
||||
const stepMode: "prompt" | "script" = ws.mode || "prompt";
|
||||
const gateMode: "gate" | "advisory" = ws.gateMode || (stepMode === "script" ? "gate" : "advisory");
|
||||
|
||||
// Skip validation per mode
|
||||
if (stepMode === "prompt" && !ws.prompt?.trim()) {
|
||||
await this.store.logEntry(task.id, `[pre-merge] Workflow step '${ws.name}' has no prompt — skipping`);
|
||||
results.push({
|
||||
workflowStepId: ws.id,
|
||||
workflowStepName: ws.name,
|
||||
phase: stepPhase,
|
||||
status: "skipped",
|
||||
output: "No prompt configured for this workflow step",
|
||||
});
|
||||
await this.store.updateTask(task.id, { workflowStepResults: results });
|
||||
continue;
|
||||
}
|
||||
|
||||
if (stepMode === "script" && !ws.scriptName?.trim()) {
|
||||
await this.store.logEntry(task.id, `[pre-merge] Workflow step '${ws.name}' has no scriptName — skipping`);
|
||||
results.push({
|
||||
workflowStepId: ws.id,
|
||||
workflowStepName: ws.name,
|
||||
phase: stepPhase,
|
||||
status: "skipped",
|
||||
output: "No scriptName configured for this workflow step",
|
||||
});
|
||||
await this.store.updateTask(task.id, { workflowStepResults: results });
|
||||
continue;
|
||||
}
|
||||
|
||||
if (this.isFrontendUxStep(ws)) {
|
||||
try {
|
||||
const diffScopedFiles = await this.captureModifiedFiles(worktreePath, currentTask.baseCommitSha, task.id, undefined, "workflow-step-frontend-ux");
|
||||
const declaredScopedFiles = await this.store.parseFileScopeFromPrompt(task.id).catch(() => [] as string[]);
|
||||
const diffHasSignal = diffScopedFiles.length > 0;
|
||||
const declaredHasSignal = declaredScopedFiles.length > 0;
|
||||
const diffHasFrontendFiles = diffHasSignal && this.hasFrontendFilesInScope(diffScopedFiles);
|
||||
const declaredHasFrontendFiles = declaredHasSignal && this.hasFrontendFilesInScope(declaredScopedFiles);
|
||||
|
||||
const shouldSkipForDiffOnly = diffHasSignal && !declaredHasSignal && !diffHasFrontendFiles;
|
||||
const shouldSkipForDeclaredOnly = declaredHasSignal && !diffHasSignal && !declaredHasFrontendFiles;
|
||||
const shouldSkipForBothSignals = diffHasSignal && declaredHasSignal && !diffHasFrontendFiles && !declaredHasFrontendFiles;
|
||||
|
||||
if (shouldSkipForDiffOnly || shouldSkipForDeclaredOnly || shouldSkipForBothSignals) {
|
||||
const skippedForDeclaredScope = shouldSkipForDeclaredOnly || shouldSkipForBothSignals;
|
||||
results.push({
|
||||
workflowStepId: ws.id,
|
||||
workflowStepName: ws.name,
|
||||
phase: stepPhase,
|
||||
status: "skipped",
|
||||
output: skippedForDeclaredScope
|
||||
? "Declared File Scope contains no frontend/UI files — auto-skipped (FN-4343)"
|
||||
: "No frontend/UI files in diff scope — auto-skipped (FN-3906)",
|
||||
});
|
||||
await this.store.updateTask(task.id, { workflowStepResults: results });
|
||||
await this.store.logEntry(
|
||||
task.id,
|
||||
skippedForDeclaredScope
|
||||
? "[pre-merge] Auto-skipped Frontend UX Design — declared File Scope contains no frontend/UI files (FN-4343)"
|
||||
: "[pre-merge] Auto-skipped Frontend UX Design — no frontend/UI files in diff scope",
|
||||
);
|
||||
continue;
|
||||
}
|
||||
} catch {
|
||||
// best-effort scope detection only; fall through to regular execution/defer flow
|
||||
}
|
||||
}
|
||||
|
||||
if (await this.shouldDeferWorkflowStepCompletion(task.id, `before workflow step '${ws.name}'`)) {
|
||||
return "deferred-paused";
|
||||
}
|
||||
|
||||
if (ws.id.startsWith("plugin:")) {
|
||||
await this.store.logEntry(task.id, `[pre-merge] Starting plugin workflow step: ${ws.name} (${ws.id})`);
|
||||
} else {
|
||||
await this.store.logEntry(task.id, `[pre-merge] Starting workflow step: ${ws.name} (${stepMode} mode)`);
|
||||
}
|
||||
executorLog.log(`${task.id} — [pre-merge] running workflow step: ${ws.name} (${stepMode} mode)`);
|
||||
|
||||
const startedAt = new Date().toISOString();
|
||||
const stepStartedAtMs = Date.now();
|
||||
const workflowStepScopeEnforcement = settings.workflowStepScopeEnforcement ?? "block";
|
||||
const shouldCheckWorkflowStepScope = stepPhase === "pre-merge"
|
||||
&& stepMode === "prompt"
|
||||
&& workflowStepScopeEnforcement !== "off";
|
||||
const preStepModifiedFiles = shouldCheckWorkflowStepScope
|
||||
? await this.captureModifiedFiles(worktreePath, currentTask.baseCommitSha, task.id, undefined, "workflow-step-pre")
|
||||
: [];
|
||||
|
||||
// Push pending entry BEFORE execution so dashboard can show live status
|
||||
results.push({
|
||||
workflowStepId: ws.id,
|
||||
workflowStepName: ws.name,
|
||||
phase: stepPhase,
|
||||
status: "pending",
|
||||
startedAt,
|
||||
});
|
||||
await this.store.updateTask(task.id, { workflowStepResults: results });
|
||||
|
||||
try {
|
||||
const result: WorkflowStepOutcome = stepMode === "script"
|
||||
? await this.executeScriptWorkflowStep(task, ws, worktreePath, settings, taskEnv)
|
||||
: await this.executeWorkflowStep(task, ws, worktreePath, settings, taskEnv);
|
||||
if (await this.shouldDeferWorkflowStepCompletion(task.id, `workflow step '${ws.name}'`)) {
|
||||
return "deferred-paused";
|
||||
}
|
||||
const completedAt = new Date().toISOString();
|
||||
|
||||
if (result.success) {
|
||||
if (shouldCheckWorkflowStepScope) {
|
||||
const declaredScope = await this.store.parseFileScopeFromPrompt(task.id).catch(() => [] as string[]);
|
||||
const refreshedTask = await this.store.getTask(task.id);
|
||||
if (declaredScope.length > 0 && refreshedTask?.scopeOverride !== true) {
|
||||
const postStepModifiedFiles = await this.captureModifiedFiles(worktreePath, currentTask.baseCommitSha, task.id, undefined, "workflow-step-post");
|
||||
const preStepSet = new Set(preStepModifiedFiles);
|
||||
const stepCommittedFiles = postStepModifiedFiles.filter((filePath) => !preStepSet.has(filePath));
|
||||
const stepUncommittedFiles = await this.captureUncommittedModifiedFiles(worktreePath);
|
||||
const stepTouchedFiles = [...new Set([...stepCommittedFiles, ...stepUncommittedFiles])];
|
||||
const hasScopeOverlap = stepTouchedFiles.some((filePath) => workflowPathMatchesDeclaredScope(filePath, declaredScope));
|
||||
if (stepTouchedFiles.length > 0 && !hasScopeOverlap) {
|
||||
const scopeLeakMessage = `Workflow step '${ws.name}' wrote files outside declared File Scope. Staged: [${stepTouchedFiles.join(", ")}]. Declared: [${declaredScope.join(", ")}]. (FN-4343)`;
|
||||
await this.store.logEntry(
|
||||
task.id,
|
||||
`[pre-merge] Workflow step scope leak: ${ws.name} wrote off-scope files [${stepTouchedFiles.join(", ") || "<none>"}]`,
|
||||
);
|
||||
if (workflowStepScopeEnforcement === "warn") {
|
||||
await this.store.logEntry(task.id, `[pre-merge] workflowStepScopeEnforcement=warn — ${scopeLeakMessage}`);
|
||||
} else {
|
||||
const existingIdx = results.findIndex(r => r.workflowStepId === ws.id);
|
||||
if (existingIdx >= 0) {
|
||||
results[existingIdx] = {
|
||||
...results[existingIdx],
|
||||
status: gateMode === "advisory" ? "advisory_failure" : "failed",
|
||||
output: scopeLeakMessage,
|
||||
notes: scopeLeakMessage,
|
||||
completedAt,
|
||||
};
|
||||
}
|
||||
await this.store.updateTask(task.id, { workflowStepResults: results });
|
||||
if (gateMode === "advisory") {
|
||||
await this.store.updateTask(task.id, { status: "advisory_failure" });
|
||||
await this.store.logEntry(task.id, `[pre-merge] Advisory workflow step scope warning: ${ws.name}`);
|
||||
continue;
|
||||
}
|
||||
return {
|
||||
allPassed: false,
|
||||
revisionRequested: true,
|
||||
feedback: scopeLeakMessage,
|
||||
stepName: ws.name,
|
||||
};
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
await this.store.logEntry(task.id, `[timing] Workflow step '${ws.name}' completed in ${Date.now() - stepStartedAtMs}ms`);
|
||||
await this.store.logEntry(task.id, `[pre-merge] Workflow step completed: ${ws.name}`);
|
||||
executorLog.log(`${task.id} — [pre-merge] workflow step passed: ${ws.name}`);
|
||||
// Update existing pending entry in place
|
||||
const existingIdx = results.findIndex(r => r.workflowStepId === ws.id);
|
||||
if (existingIdx >= 0) {
|
||||
const malformed = result.malformed === true;
|
||||
results[existingIdx] = {
|
||||
...results[existingIdx],
|
||||
status: malformed ? "skipped" : "passed",
|
||||
output: malformed ? "malformed output — no verdict extracted" : result.output,
|
||||
verdict: result.verdict,
|
||||
notes: result.notes ?? (malformed ? undefined : result.output),
|
||||
completedAt,
|
||||
};
|
||||
}
|
||||
await this.store.updateTask(task.id, { workflowStepResults: results });
|
||||
} else if (result.revisionRequested) {
|
||||
// Revision requested — this is a structured outcome that routes back to executor
|
||||
await this.store.logEntry(task.id, `[timing] Workflow step '${ws.name}' requested revision after ${Date.now() - stepStartedAtMs}ms`);
|
||||
await this.store.logEntry(
|
||||
task.id,
|
||||
`[pre-merge] Workflow step requested revision: ${ws.name}`,
|
||||
result.output,
|
||||
);
|
||||
executorLog.log(`${task.id} — [pre-merge] workflow step requested revision: ${ws.name}`);
|
||||
// Update existing pending entry in place
|
||||
const existingIdx = results.findIndex(r => r.workflowStepId === ws.id);
|
||||
if (existingIdx >= 0) {
|
||||
results[existingIdx] = {
|
||||
...results[existingIdx],
|
||||
status: gateMode === "advisory" ? "advisory_failure" : "failed",
|
||||
output: result.output || "Revision requested",
|
||||
verdict: result.verdict,
|
||||
notes: result.notes || result.output || "Revision requested",
|
||||
completedAt,
|
||||
};
|
||||
}
|
||||
await this.store.updateTask(task.id, { workflowStepResults: results });
|
||||
if (gateMode === "advisory") {
|
||||
await this.store.logEntry(task.id, `[pre-merge] Advisory workflow step failed: ${ws.name}`);
|
||||
continue;
|
||||
}
|
||||
return {
|
||||
allPassed: false,
|
||||
revisionRequested: true,
|
||||
feedback: result.output || "Workflow step requested revision",
|
||||
stepName: ws.name,
|
||||
};
|
||||
} else {
|
||||
// Hard failure
|
||||
await this.store.logEntry(task.id, `[timing] Workflow step '${ws.name}' failed after ${Date.now() - stepStartedAtMs}ms`);
|
||||
await this.store.logEntry(
|
||||
task.id,
|
||||
`[pre-merge] Workflow step failed: ${ws.name}`,
|
||||
result.error || "Unknown error",
|
||||
);
|
||||
executorLog.error(`${task.id} — [pre-merge] workflow step failed: ${ws.name}; output captured in task log`);
|
||||
// Update existing pending entry in place
|
||||
const existingIdx = results.findIndex(r => r.workflowStepId === ws.id);
|
||||
if (existingIdx >= 0) {
|
||||
results[existingIdx] = {
|
||||
...results[existingIdx],
|
||||
status: gateMode === "advisory" ? "advisory_failure" : "failed",
|
||||
output: result.error || "Workflow step failed",
|
||||
notes: result.error || "Workflow step failed",
|
||||
completedAt,
|
||||
};
|
||||
}
|
||||
await this.store.updateTask(task.id, { workflowStepResults: results });
|
||||
if (gateMode === "advisory") {
|
||||
await this.store.updateTask(task.id, { status: "advisory_failure" });
|
||||
await this.store.logEntry(task.id, `[pre-merge] Advisory workflow step failed: ${ws.name}`);
|
||||
continue;
|
||||
}
|
||||
return {
|
||||
allPassed: false,
|
||||
revisionRequested: false,
|
||||
feedback: result.error || "Workflow step failed",
|
||||
stepName: ws.name,
|
||||
};
|
||||
}
|
||||
} catch (err: unknown) {
|
||||
if (await this.shouldDeferWorkflowStepCompletion(task.id, `workflow step '${ws.name}'`)) {
|
||||
return "deferred-paused";
|
||||
}
|
||||
const { message: errorMessage, detail: errorDetail, stack: errorStack } = formatError(err);
|
||||
const completedAt = new Date().toISOString();
|
||||
await this.store.logEntry(
|
||||
task.id,
|
||||
`[pre-merge] Workflow step failed: ${ws.name}`,
|
||||
errorStack ?? errorDetail,
|
||||
);
|
||||
executorLog.error(`${task.id} — [pre-merge] workflow step error: ${ws.name} — ${errorDetail}`);
|
||||
// Update existing pending entry in place
|
||||
const existingIdx = results.findIndex(r => r.workflowStepId === ws.id);
|
||||
if (existingIdx >= 0) {
|
||||
results[existingIdx] = {
|
||||
...results[existingIdx],
|
||||
status: gateMode === "advisory" ? "advisory_failure" : "failed",
|
||||
output: errorMessage || "Workflow step error",
|
||||
notes: errorMessage || "Workflow step error",
|
||||
completedAt,
|
||||
};
|
||||
}
|
||||
await this.store.updateTask(task.id, { workflowStepResults: results });
|
||||
if (gateMode === "advisory") {
|
||||
await this.store.updateTask(task.id, { status: "advisory_failure" });
|
||||
await this.store.logEntry(task.id, `[pre-merge] Advisory workflow step error: ${ws.name}`);
|
||||
continue;
|
||||
}
|
||||
return {
|
||||
allPassed: false,
|
||||
revisionRequested: false,
|
||||
feedback: errorMessage || "Workflow step error",
|
||||
stepName: ws.name,
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
return { allPassed: true };
|
||||
}
|
||||
|
||||
/**
|
||||
* Execute a script-mode workflow step by resolving the scriptName to a command
|
||||
* from project settings and running it in the task worktree.
|
||||
@@ -13280,42 +12851,6 @@ ${failureFeedback}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* FN-3906: Only the built-in Frontend UX Design step gets orchestrator-level
|
||||
* diff-scope auto-skip. Match by canonical template id only.
|
||||
*/
|
||||
private isFrontendUxStep(workflowStep: WorkflowStep): boolean {
|
||||
return workflowStep.id === "frontend-ux-design";
|
||||
}
|
||||
|
||||
/**
|
||||
* FN-3906: Detect whether the task diff scope contains frontend/UI-related
|
||||
* files so Frontend UX Design can be safely skipped when irrelevant.
|
||||
*/
|
||||
private hasFrontendFilesInScope(files: string[]): boolean {
|
||||
const frontendExtensionPattern = /\.(tsx|jsx|vue|svelte|astro|html|css|scss|sass|less|styl)$/i;
|
||||
const frontendPathMarkers = [
|
||||
"/components/",
|
||||
"/app/components/",
|
||||
"/dashboard/",
|
||||
"/frontend/",
|
||||
"/ui/",
|
||||
"/styles/",
|
||||
"/themes/",
|
||||
"/design-system/",
|
||||
"/design-tokens/",
|
||||
];
|
||||
const frontendTokenFilenamePattern = /(^|\/)(tokens|theme)\.(ts|js|json|css)$/i;
|
||||
|
||||
return files.some((file) => {
|
||||
const normalized = file.replace(/\\/g, "/");
|
||||
const lowered = normalized.toLowerCase();
|
||||
return frontendExtensionPattern.test(normalized)
|
||||
|| frontendPathMarkers.some((marker) => lowered.includes(marker))
|
||||
|| frontendTokenFilenamePattern.test(lowered);
|
||||
});
|
||||
}
|
||||
|
||||
/** Parse structured JSON verdict from workflow step output. */
|
||||
private parseWorkflowStepOutput(rawOutput: string): {
|
||||
output: string;
|
||||
@@ -13754,28 +13289,6 @@ You have access to the file system to review changes.${verdictBlock}`;
|
||||
return runOnce(fallback.provider, fallback.modelId, "fallback");
|
||||
}
|
||||
|
||||
private async auditReadonlyWorkflowStepPromptsOnce(taskId: string): Promise<void> {
|
||||
if (this.readonlyWorkflowStepAuditDone) return;
|
||||
this.readonlyWorkflowStepAuditDone = true;
|
||||
const tokens = ["edit", "write", "commit", "stage", "modify"];
|
||||
try {
|
||||
const steps = await this.store.listWorkflowSteps();
|
||||
for (const step of steps) {
|
||||
if ((step.mode || "prompt") !== "prompt" || (step.toolMode || "readonly") !== "readonly") continue;
|
||||
const prompt = step.prompt || "";
|
||||
for (const token of tokens) {
|
||||
const re = new RegExp(`\\b${token}\\b`, "i");
|
||||
if (re.test(prompt)) {
|
||||
executorLog.warn(`[workflow-step-audit] readonly step "${step.name}" prompt contains write-implying token "${token}" — re-review intended scope (no auto-migration performed)`);
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
} catch (error) {
|
||||
executorLog.warn(`${taskId}: failed readonly workflow-step prompt audit: ${formatError(error)}`);
|
||||
}
|
||||
}
|
||||
|
||||
private MAX_WORKTREE_RETRIES = 3;
|
||||
private WORKTREE_RETRY_DELAYS = [100, 500, 1000]; // ms
|
||||
|
||||
|
||||
@@ -83,8 +83,6 @@ export {
|
||||
type CodingSessionResult,
|
||||
type ReviewPrimitiveResult,
|
||||
type VerificationPrimitiveResult,
|
||||
type WorkflowStepPrimitiveInput,
|
||||
type WorkflowStepPrimitiveResult,
|
||||
type TransitionPrimitiveInput,
|
||||
type MergePrimitiveInput,
|
||||
type MergePrimitiveResult,
|
||||
|
||||
@@ -14,7 +14,6 @@ export type RuntimePrimitiveName =
|
||||
| "reset-step"
|
||||
| "review"
|
||||
| "verification"
|
||||
| "workflow-step"
|
||||
| "transition"
|
||||
| "merge"
|
||||
| "abort"
|
||||
@@ -80,18 +79,11 @@ export interface VerificationPrimitiveResult {
|
||||
stepName?: string;
|
||||
}
|
||||
|
||||
export interface WorkflowStepPrimitiveInput {
|
||||
phase: "pre-merge" | "post-merge";
|
||||
stepId?: string;
|
||||
worktreePath?: string;
|
||||
}
|
||||
|
||||
export interface WorkflowStepPrimitiveResult {
|
||||
allPassed: boolean;
|
||||
revisionRequested?: boolean;
|
||||
feedback?: string;
|
||||
stepName?: string;
|
||||
}
|
||||
// FNXC:WorkflowExecution 2026-06-25-00:00: U4 (KTD-2) — the `runWorkflowStep`
|
||||
// primitive and its `WorkflowStepPrimitiveInput`/`WorkflowStepPrimitiveResult`
|
||||
// shapes were removed. Workflow quality gates run as the graph's own
|
||||
// optional-group / gate nodes which record into `task.workflowStepResults` (U2);
|
||||
// there is no dedicated workflow-step runtime primitive.
|
||||
|
||||
export interface TransitionPrimitiveInput {
|
||||
column?: string;
|
||||
@@ -179,12 +171,6 @@ export interface WorkflowRuntimePrimitives {
|
||||
prepared: PreparedWorktree,
|
||||
): Promise<RuntimePrimitiveResult<VerificationPrimitiveResult>>;
|
||||
|
||||
runWorkflowStep(
|
||||
ctx: WorkflowPrimitiveContext,
|
||||
task: TaskDetail,
|
||||
input: WorkflowStepPrimitiveInput,
|
||||
): Promise<RuntimePrimitiveResult<WorkflowStepPrimitiveResult>>;
|
||||
|
||||
updateSteps(
|
||||
ctx: WorkflowPrimitiveContext,
|
||||
task: TaskDetail,
|
||||
|
||||
@@ -99,15 +99,10 @@ function primitivesFromLegacySeams(seams: WorkflowLegacySeams): WorkflowRuntimeP
|
||||
return { ...result, data: { verdict: result.outcome === "success" ? "APPROVE" : "REVISE" } };
|
||||
},
|
||||
runVerification: async () => ({ outcome: "success", data: { verdict: "skipped" } }),
|
||||
runWorkflowStep: async (ctx, task) => {
|
||||
const result = await seams.workflowStep?.(task, ctx.node.context ?? {});
|
||||
return {
|
||||
outcome: result?.outcome ?? "success",
|
||||
value: result?.value ?? "workflow-step-skipped",
|
||||
contextPatch: result?.contextPatch,
|
||||
data: { allPassed: result?.outcome !== "failure" },
|
||||
};
|
||||
},
|
||||
// FNXC:WorkflowExecution 2026-06-25-00:00: U4 (KTD-2) — the `runWorkflowStep`
|
||||
// primitive + `workflow-step` seam were removed. Workflow gates run as graph
|
||||
// optional-group / gate nodes that record into task.workflowStepResults (U2);
|
||||
// this driver no longer adapts a workflow-step seam.
|
||||
updateSteps: async (_ctx, _task, steps) => ({ outcome: "success", data: { count: steps.length } }),
|
||||
transitionTask: async (ctx, task) => seams.schedule(task, ctx.node.context ?? {}),
|
||||
requestMerge: async (ctx, task) => {
|
||||
|
||||
@@ -178,11 +178,9 @@ export class WorkflowGraphTaskRunner {
|
||||
const wrappedSeams: WorkflowLegacySeams = {
|
||||
planning: (t, c) => ((sideEffectsRan = true), invoked.push("planning"), seams.planning(t, c)),
|
||||
execute: (t, c) => ((sideEffectsRan = true), invoked.push("execute"), seams.execute(t, c)),
|
||||
workflowStep: (t, c) => {
|
||||
sideEffectsRan = true;
|
||||
invoked.push("workflow-step");
|
||||
return seams.workflowStep?.(t, c) ?? Promise.resolve({ outcome: "success", value: "workflow-step-skipped" });
|
||||
},
|
||||
// FNXC:WorkflowExecution 2026-06-25-00:00: U4 (KTD-2) — the `workflow-step`
|
||||
// seam wrapper was removed; workflow gates run as graph optional-group / gate
|
||||
// nodes that record into task.workflowStepResults (U2).
|
||||
review: (t, c) => ((sideEffectsRan = true), invoked.push("review"), seams.review(t, c)),
|
||||
merge: (t, c) => ((sideEffectsRan = true), invoked.push("merge"), seams.merge(t, c)),
|
||||
schedule: (t, c) => ((sideEffectsRan = true), invoked.push("schedule"), seams.schedule(t, c)),
|
||||
|
||||
@@ -11,10 +11,16 @@ import {
|
||||
} from "./runtime-primitives.js";
|
||||
import { runWorkflowMergeAttemptNode } from "./workflow-merge-nodes.js";
|
||||
|
||||
// FNXC:WorkflowExecution 2026-06-25-00:00: U4 (KTD-2) — the `workflow-step` seam
|
||||
// was removed. Workflow quality gates run as the graph's own optional-group /
|
||||
// gate nodes (builtin:coding replaced its `workflow-step` seam node with
|
||||
// optional-group nodes) which record into `task.workflowStepResults` (U2). An IR
|
||||
// node still declaring `config.seam: "workflow-step"` is no longer a recognized
|
||||
// seam: `resolveSeamName` throws a WorkflowIrError for it (fails loud, never a
|
||||
// silent no-op).
|
||||
export type WorkflowSeamName =
|
||||
| "planning"
|
||||
| "execute"
|
||||
| "workflow-step"
|
||||
| "review"
|
||||
| "merge"
|
||||
| "schedule"
|
||||
@@ -26,7 +32,6 @@ export interface WorkflowLegacySeams {
|
||||
* custom planning behavior is expressed as a custom prompt node. */
|
||||
planning: (task: TaskDetail, context: Record<string, unknown>) => Promise<WorkflowNodeResult>;
|
||||
execute: (task: TaskDetail, context: Record<string, unknown>) => Promise<WorkflowNodeResult>;
|
||||
workflowStep?: (task: TaskDetail, context: Record<string, unknown>) => Promise<WorkflowNodeResult>;
|
||||
review: (task: TaskDetail, context: Record<string, unknown>) => Promise<WorkflowNodeResult>;
|
||||
merge: (task: TaskDetail, context: Record<string, unknown>) => Promise<WorkflowNodeResult>;
|
||||
schedule: (task: TaskDetail, context: Record<string, unknown>) => Promise<WorkflowNodeResult>;
|
||||
@@ -205,7 +210,6 @@ export function resolveSeamName(node: { config?: Record<string, unknown> }): Wor
|
||||
if (
|
||||
seam === "planning" ||
|
||||
seam === "execute" ||
|
||||
seam === "workflow-step" ||
|
||||
seam === "review" ||
|
||||
seam === "merge" ||
|
||||
seam === "schedule" ||
|
||||
@@ -261,11 +265,6 @@ export function createPromptLikeHandler(
|
||||
// IS the seam node, so its declared column drives the binding. (Other seams
|
||||
// — planning/review/merge/schedule — stamp it too; only execute reads it.)
|
||||
context.context[SEAM_GOVERNING_NODE_CONTEXT_KEY] = node.id;
|
||||
if (seam === "workflow-step") {
|
||||
return seams.workflowStep
|
||||
? seams.workflowStep(context.task, context.context)
|
||||
: { outcome: "success", value: "workflow-step-skipped" };
|
||||
}
|
||||
return seams[seam]!(context.task, context.context);
|
||||
}
|
||||
if (!runCustomNode) {
|
||||
@@ -352,16 +351,6 @@ export function createPrimitivePromptLikeHandler(
|
||||
},
|
||||
};
|
||||
}
|
||||
if (seam === "workflow-step") {
|
||||
const worktreePath = typeof context.context["workflow:worktree-path"] === "string"
|
||||
? context.context["workflow:worktree-path"]
|
||||
: undefined;
|
||||
const result = await primitives.runWorkflowStep(primitiveCtx, context.task, {
|
||||
phase: "pre-merge",
|
||||
worktreePath,
|
||||
});
|
||||
return { outcome: result.outcome, value: result.value, contextPatch: result.contextPatch };
|
||||
}
|
||||
if (seam === "review") {
|
||||
const result = await primitives.runReview(primitiveCtx, context.task, { type: "code" });
|
||||
return { outcome: result.outcome, value: result.value, contextPatch: result.contextPatch };
|
||||
@@ -965,7 +954,7 @@ export function createNoopLegacySeams(): WorkflowLegacySeams {
|
||||
return {
|
||||
planning: success,
|
||||
execute: success,
|
||||
workflowStep: success,
|
||||
// U4 (KTD-2): no `workflow-step` seam — workflow gates run as graph nodes.
|
||||
review: success,
|
||||
merge: success,
|
||||
schedule: success,
|
||||
|
||||
Reference in New Issue
Block a user