fix(review): apply code-review fixes — dead code, step output/notes, seam-contract sync
From the U1-U6 code review (correctness/adversarial/reliability/maintainability):
- Delete dead code left by the runWorkflowSteps removal: parkTaskAfterWorkflowStepPause,
handleWorkflowRevisionRequest (+ createWorkflowRevisionFollowUpTask,
injectWorkflowRevisionInstructions), handleWorkflowStepFailure, the dead
partitionWorkflowRevisionFeedback export + its test, and 2 orphaned jsdocs;
reword 2 stale comments (executor.ts FN-6722, self-healing.ts jsdoc).
- Record step output/notes on graph workflow-step results: runGraphCustomNode now
emits contextPatch:{output,notes} so the Workflow tab shows real review feedback
and [pre-merge] revision logs carry detail (was always the fallback before).
- Sync the workflow-step seam contract: core (workflow-compiler SEAM_NAMES/order,
workflow-ir column map, builtin-workflow-prompts) now rejects the workflow-step
seam to match the engine's resolveSeamName, preventing a latent run-time crash on
a persisted/cloned def that core would otherwise parse.
Residual (tracked in the PR): re-introduce the FN-4343 per-step scope gate on the
graph path; the parked-failed recovery log wording; malformed-advisory->passed edge
case; recording for non-optional-group/split-branch step realizations.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -208,7 +208,7 @@ describe("compileWorkflowToSteps (U2)", () => {
|
||||
expect(err?.message).toMatch(/disconnected nodes/);
|
||||
});
|
||||
|
||||
it("rejects seams that are out of the planning -> execute -> workflow-step -> review -> merge order", () => {
|
||||
it("rejects seams that are out of the planning -> execute -> review -> merge order", () => {
|
||||
const ir: WorkflowIr = {
|
||||
version: "v1",
|
||||
name: "misordered-seams",
|
||||
@@ -226,7 +226,7 @@ describe("compileWorkflowToSteps (U2)", () => {
|
||||
};
|
||||
const err = validateLinearity(ir);
|
||||
expect(err).toBeInstanceOf(WorkflowCompileError);
|
||||
expect(err?.message).toMatch(/planning -> execute -> workflow-step -> review -> merge order/);
|
||||
expect(err?.message).toMatch(/planning -> execute -> review -> merge order/);
|
||||
});
|
||||
|
||||
it("rejects a graph with a duplicated seam role", () => {
|
||||
|
||||
@@ -11,7 +11,6 @@ export const BUILTIN_SEAM_PROMPTS: Record<string, string> = {
|
||||
planning: DEFAULT_TRIAGE_PROMPT,
|
||||
"planning-fast": DEFAULT_TRIAGE_FAST_PROMPT,
|
||||
"step-execute": DEFAULT_EXECUTOR_PROMPT,
|
||||
"workflow-step": DEFAULT_REVIEWER_PROMPT,
|
||||
review: DEFAULT_REVIEWER_PROMPT,
|
||||
merge: DEFAULT_MERGER_PROMPT,
|
||||
};
|
||||
|
||||
@@ -40,9 +40,9 @@ function isMergeRegionKind(node: WorkflowIrNode): boolean {
|
||||
}
|
||||
|
||||
/** Seam anchor kinds, encoded on IR nodes as `config.seam`. These map to the
|
||||
* fixed planning → execute → workflow-step → review → merge pipeline and are
|
||||
* fixed planning → execute → review → merge pipeline and are
|
||||
* not emitted as steps. */
|
||||
const SEAM_NAMES = new Set(["planning", "execute", "workflow-step", "review", "merge"]);
|
||||
const SEAM_NAMES = new Set(["planning", "execute", "review", "merge"]);
|
||||
|
||||
const ENGINE_PRIMITIVE_NODE_KINDS = new Set<WorkflowIrNode["kind"]>([
|
||||
"merge-gate",
|
||||
@@ -163,11 +163,11 @@ export function validateLinearity(ir: WorkflowIr): WorkflowCompileError | null {
|
||||
|
||||
// Reachability: the single main path must reach end and cover every node.
|
||||
// While walking, enforce the canonical seam pipeline: each of planning/
|
||||
// execute/workflow-step/review/merge may appear at most once and only in that
|
||||
// execute/review/merge may appear at most once and only in that
|
||||
// order. The compiler treats seams as a fixed lifecycle boundary (merge flips
|
||||
// pre- to post-merge), so out-of-order or duplicate seams would compile
|
||||
// inconsistently with the runtime contract.
|
||||
const expectedSeamOrder = ["planning", "execute", "workflow-step", "review", "merge"] as const;
|
||||
const expectedSeamOrder = ["planning", "execute", "review", "merge"] as const;
|
||||
const seenSeams = new Set<string>();
|
||||
let nextExpectedSeamIndex = 0;
|
||||
const visited = new Set<string>();
|
||||
@@ -193,7 +193,7 @@ export function validateLinearity(ir: WorkflowIr): WorkflowCompileError | null {
|
||||
}
|
||||
if (expectedSeamOrder[nextExpectedSeamIndex] !== seam) {
|
||||
return new WorkflowCompileError(
|
||||
"seams must follow the planning -> execute -> workflow-step -> review -> merge order",
|
||||
"seams must follow the planning -> execute -> review -> merge order",
|
||||
);
|
||||
}
|
||||
seenSeams.add(seam);
|
||||
|
||||
@@ -139,7 +139,6 @@ export const DEFAULT_WORKFLOW_COLUMN_IDS = [
|
||||
function defaultColumnForNode(node: WorkflowIrNode): string {
|
||||
const seam = node.config?.seam;
|
||||
if (seam === "execute") return "in-progress";
|
||||
if (seam === "workflow-step") return "in-progress";
|
||||
if (seam === "review") return "in-review";
|
||||
if (seam === "merge") return "in-review";
|
||||
return "todo";
|
||||
|
||||
@@ -155,6 +155,34 @@ describe("WorkflowGraphExecutor optional-group → task.workflowStepResults (pla
|
||||
expect(recorder.results).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("(e) carries the inner node's output AND notes through to the recorded result (REVISE with notes)", async () => {
|
||||
// FNXC:WorkflowStepResults 2026-06-26-00:00: The recorded WorkflowStepResult
|
||||
// must carry the step agent's `output` and the parsed verdict `notes` (surfaced
|
||||
// on the inner node result's contextPatch by runGraphCustomNode) so the Workflow
|
||||
// tab shows real detail instead of a fallback and `[pre-merge]` revision logs
|
||||
// carry the notes. Advisory REVISE keeps the success outcome (non-blocking).
|
||||
const recorder = makeRecorder();
|
||||
const executor = new WorkflowGraphExecutor({
|
||||
handlers: {
|
||||
prompt: innerHandler({
|
||||
outcome: "success",
|
||||
value: "REVISE",
|
||||
contextPatch: { output: "full step output text", notes: "please fix the null check" },
|
||||
}),
|
||||
},
|
||||
recordWorkflowStepResult: recorder.record,
|
||||
});
|
||||
|
||||
await executor.run(taskWith(["code-review"]), settingsOn(), codeReviewGroupIr());
|
||||
|
||||
expect(recorder.results).toHaveLength(1);
|
||||
const entry = recorder.results[0];
|
||||
expect(entry.status).toBe("advisory_failure");
|
||||
expect(entry.verdict).toBe("REVISE");
|
||||
expect(entry.output).toBe("full step output text");
|
||||
expect(entry.notes).toBe("please fix the null check");
|
||||
});
|
||||
|
||||
it("emits parity [pre-merge] logs for the enabled group via logTaskEntry", async () => {
|
||||
const logs: string[] = [];
|
||||
const recorder = makeRecorder();
|
||||
|
||||
@@ -1,81 +0,0 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
extractReferencedPathsFromWorkflowFeedback,
|
||||
partitionWorkflowRevisionFeedback,
|
||||
workflowPathMatchesDeclaredScope,
|
||||
} from "../executor.js";
|
||||
|
||||
describe("workflow revision scope partitioning", () => {
|
||||
const declaredScope = [
|
||||
"packages/engine/src/executor.ts",
|
||||
"packages/engine/src/__tests__/*",
|
||||
];
|
||||
|
||||
it("keeps fully in-scope feedback attached to the original task", () => {
|
||||
const feedback = [
|
||||
"Update `packages/engine/src/executor.ts` to guard the rerun path.",
|
||||
"Add a regression in `packages/engine/src/__tests__/executor-step-session.test.ts`.",
|
||||
].join("\n\n");
|
||||
|
||||
const result = partitionWorkflowRevisionFeedback(feedback, declaredScope);
|
||||
|
||||
expect(result.inScopeFeedback).toBe(feedback);
|
||||
expect(result.outOfScopeFeedback).toBe("");
|
||||
expect(result.outOfScopeSegments).toEqual([]);
|
||||
});
|
||||
|
||||
it("forks fully out-of-scope feedback into a follow-up block", () => {
|
||||
const feedback = [
|
||||
"Move the setting into `packages/core/src/types.ts`.",
|
||||
"Document it in `docs/settings-reference.md`.",
|
||||
].join("\n\n");
|
||||
|
||||
const result = partitionWorkflowRevisionFeedback(feedback, declaredScope);
|
||||
|
||||
expect(result.inScopeFeedback).toBe("");
|
||||
expect(result.outOfScopeFeedback).toBe(feedback);
|
||||
expect(result.outOfScopeSegments).toHaveLength(2);
|
||||
});
|
||||
|
||||
it("splits mixed feedback by paragraph and preserves pathless guidance with the original task", () => {
|
||||
const feedback = [
|
||||
"Tighten the executor behavior in `packages/engine/src/executor.ts`.",
|
||||
"Keep the rerun log message concise.",
|
||||
"Add the new opt-out setting in `packages/core/src/settings-schema.ts`.",
|
||||
].join("\n\n");
|
||||
|
||||
const result = partitionWorkflowRevisionFeedback(feedback, declaredScope);
|
||||
|
||||
expect(result.inScopeSegments).toEqual([
|
||||
"Tighten the executor behavior in `packages/engine/src/executor.ts`.",
|
||||
"Keep the rerun log message concise.",
|
||||
]);
|
||||
expect(result.outOfScopeSegments).toEqual([
|
||||
"Add the new opt-out setting in `packages/core/src/settings-schema.ts`.",
|
||||
]);
|
||||
});
|
||||
|
||||
it("keeps feedback with no detectable file paths on the original task", () => {
|
||||
const feedback = "Clarify the retry behavior and keep the reviewer-facing explanation actionable.";
|
||||
|
||||
const result = partitionWorkflowRevisionFeedback(feedback, declaredScope);
|
||||
|
||||
expect(result.detectedPaths).toEqual([]);
|
||||
expect(result.inScopeFeedback).toBe(feedback);
|
||||
expect(result.outOfScopeFeedback).toBe("");
|
||||
});
|
||||
|
||||
it("normalizes feedback paths before comparing them to declared scope", () => {
|
||||
expect(workflowPathMatchesDeclaredScope("./packages/engine/src/__tests__/executor-step-session.test.ts", declaredScope)).toBe(true);
|
||||
expect(workflowPathMatchesDeclaredScope("packages/core/src/types.ts", declaredScope)).toBe(false);
|
||||
});
|
||||
|
||||
it("extracts backticked and bare project-relative paths", () => {
|
||||
const feedback = "Touch `packages/engine/src/executor.ts`, then mirror the test in packages/engine/src/__tests__/executor-step-session.test.ts.";
|
||||
|
||||
expect(extractReferencedPathsFromWorkflowFeedback(feedback)).toEqual([
|
||||
"packages/engine/src/executor.ts",
|
||||
"packages/engine/src/__tests__/executor-step-session.test.ts",
|
||||
]);
|
||||
});
|
||||
});
|
||||
@@ -771,59 +771,6 @@ function evaluatePromptDerivedNoCommitEligibility(task: Task, promptContent: str
|
||||
return { eligible: true, reason: "prompt/source metadata derived operational no-commit contract" };
|
||||
}
|
||||
|
||||
export function partitionWorkflowRevisionFeedback(
|
||||
feedback: string,
|
||||
declaredFileScope: readonly string[],
|
||||
): WorkflowRevisionFeedbackPartition {
|
||||
const trimmedFeedback = feedback.trim();
|
||||
if (!trimmedFeedback || declaredFileScope.length === 0) {
|
||||
return {
|
||||
inScopeFeedback: trimmedFeedback,
|
||||
outOfScopeFeedback: "",
|
||||
inScopeSegments: trimmedFeedback ? [trimmedFeedback] : [],
|
||||
outOfScopeSegments: [],
|
||||
detectedPaths: extractReferencedPathsFromWorkflowFeedback(trimmedFeedback),
|
||||
};
|
||||
}
|
||||
|
||||
const segments = trimmedFeedback.split(/\n\s*\n/).map((segment) => segment.trim()).filter(Boolean);
|
||||
const allDetectedPaths = extractReferencedPathsFromWorkflowFeedback(trimmedFeedback);
|
||||
if (allDetectedPaths.length === 0) {
|
||||
return {
|
||||
inScopeFeedback: trimmedFeedback,
|
||||
outOfScopeFeedback: "",
|
||||
inScopeSegments: trimmedFeedback ? [trimmedFeedback] : [],
|
||||
outOfScopeSegments: [],
|
||||
detectedPaths: [],
|
||||
};
|
||||
}
|
||||
|
||||
const inScopeSegments: string[] = [];
|
||||
const outOfScopeSegments: string[] = [];
|
||||
for (const segment of segments) {
|
||||
const segmentPaths = extractReferencedPathsFromWorkflowFeedback(segment);
|
||||
if (segmentPaths.length === 0) {
|
||||
inScopeSegments.push(segment);
|
||||
continue;
|
||||
}
|
||||
|
||||
const hasOutOfScopePath = segmentPaths.some((path) => !workflowPathMatchesDeclaredScope(path, declaredFileScope));
|
||||
if (hasOutOfScopePath) {
|
||||
outOfScopeSegments.push(segment);
|
||||
} else {
|
||||
inScopeSegments.push(segment);
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
inScopeFeedback: inScopeSegments.join("\n\n"),
|
||||
outOfScopeFeedback: outOfScopeSegments.join("\n\n"),
|
||||
inScopeSegments,
|
||||
outOfScopeSegments,
|
||||
detectedPaths: allDetectedPaths,
|
||||
};
|
||||
}
|
||||
|
||||
class NonRetryableWorktreeError extends Error {}
|
||||
|
||||
function buildSessionWorktreePathRegex(rootDir: string, settings: Partial<Settings>): RegExp {
|
||||
@@ -1895,38 +1842,6 @@ export class TaskExecutor {
|
||||
return this.shouldDeferCompletionForGlobalPause(taskId, context);
|
||||
}
|
||||
|
||||
private async parkTaskAfterWorkflowStepPause(taskId: string): Promise<boolean> {
|
||||
let latestTask: Task | null = null;
|
||||
try {
|
||||
latestTask = await this.store.getTask(taskId);
|
||||
} catch {
|
||||
latestTask = null;
|
||||
}
|
||||
|
||||
if (!latestTask?.paused) {
|
||||
return false;
|
||||
}
|
||||
|
||||
executorLog.log(`${taskId}: workflow step interrupted by task pause — moving to todo`);
|
||||
await this.store.logEntry(
|
||||
taskId,
|
||||
"Execution paused during pre-merge workflow step — moved to todo",
|
||||
undefined,
|
||||
this.getRunContextFor(taskId),
|
||||
).catch(() => undefined);
|
||||
// FN-5256: synchronously reap any spawned shells BEFORE moving the task so
|
||||
// a fast re-dispatch (task:moved → in-progress) doesn't race a live shell.
|
||||
// The task:moved (away) listener also tracks an awaited disposal as a
|
||||
// backstop, but doing it here keeps `parkTaskAfterWorkflowStepPause`'s
|
||||
// contract straightforward for its callers.
|
||||
await this.awaitAbortInFlightTaskWork(taskId, "pause-before-park").catch((err) => {
|
||||
executorLog.warn(`${taskId}: awaitAbortInFlightTaskWork failed in pause-before-park: ${err}`);
|
||||
});
|
||||
if (latestTask.column === "in-progress") {
|
||||
await this.store.moveTask(taskId, "todo", { preserveResumeState: true });
|
||||
}
|
||||
return true;
|
||||
}
|
||||
/** Child agent sessions keyed by agent ID. Used for termination. */
|
||||
private childSessions = new Map<string, AgentSession>();
|
||||
/** Total count of currently spawned agents (across all parents). */
|
||||
@@ -6498,9 +6413,23 @@ export class TaskExecutor {
|
||||
const blocking = step.gateMode === "gate";
|
||||
// Script-mode outcomes carry no structured verdict; prompt-mode may.
|
||||
const verdict = (outcome as { verdict?: string }).verdict;
|
||||
// FNXC:WorkflowSteps 2026-06-26-00:00: Surface the step agent's output text
|
||||
// and parsed verdict notes on the node result's contextPatch so the
|
||||
// optional-group exit record carries them through to the recorded
|
||||
// WorkflowStepResult (workflow-graph-loop exitStepRecord →
|
||||
// workflow-graph-executor recordOptionalGroupStepResult). Without this the
|
||||
// Workflow tab only shows a generic fallback and `[pre-merge]` revision logs
|
||||
// pass `undefined` detail. `notes` is only attached when the parsed verdict
|
||||
// produced notes; `output` carries the raw step output when present.
|
||||
const stepOutput = (outcome as { output?: string }).output;
|
||||
const stepNotes = (outcome as { notes?: string }).notes;
|
||||
const contextPatch: Record<string, unknown> = {};
|
||||
if (typeof stepOutput === "string") contextPatch.output = stepOutput;
|
||||
if (typeof stepNotes === "string" && stepNotes) contextPatch.notes = stepNotes;
|
||||
return {
|
||||
outcome: outcome.success || !blocking ? "success" : "failure",
|
||||
value: verdict ?? (outcome.success ? "passed" : "failed"),
|
||||
...(Object.keys(contextPatch).length > 0 ? { contextPatch } : {}),
|
||||
};
|
||||
}
|
||||
|
||||
@@ -9475,8 +9404,8 @@ export class TaskExecutor {
|
||||
}
|
||||
// FNXC:WorkflowLifecycle 2026-06-21-00:00: FN-6722 — a mid-run abort on
|
||||
// a task that already has real step progress must not discard that
|
||||
// progress on the bounce to todo. The sibling pause-park path
|
||||
// (parkTaskAfterWorkflowStepPause, ~1826) moves with preserveResumeState;
|
||||
// progress on the bounce to todo. The sibling pause-park path moves
|
||||
// with preserveResumeState;
|
||||
// this teardown branch historically did not — it cleared `branch` AND
|
||||
// moved without preservation, which reset every step to pending
|
||||
// (store.moveTaskInternal ~7322 resetAllStepsToPending) and dropped the
|
||||
@@ -11814,114 +11743,6 @@ export class TaskExecutor {
|
||||
await this.store.logEntry(taskId, "Execution stopped — work discarded, moved to triage for re-planning");
|
||||
}
|
||||
|
||||
/**
|
||||
* Handle a workflow step revision request.
|
||||
*
|
||||
* Re-opens ONLY the last step so the executor has exactly one pending slot
|
||||
* to re-enter through. All earlier done steps stay done — the agent reads
|
||||
* the injected feedback from PROMPT.md and applies an in-place fix rather
|
||||
* than redoing any completed step.
|
||||
*/
|
||||
private async handleWorkflowRevisionRequest(
|
||||
task: Task,
|
||||
worktreePath: string,
|
||||
feedback: string,
|
||||
stepName: string,
|
||||
settings: Settings,
|
||||
): Promise<boolean> {
|
||||
executorLog.log(`${task.id}: workflow revision requested by step "${stepName}"`);
|
||||
this.clearCompletedTaskWatchdog(task.id);
|
||||
|
||||
const shouldForkOnScopeMismatch = settings.workflowRevisionForkOnScopeMismatch !== false;
|
||||
let inScopeFeedback = feedback.trim();
|
||||
let outOfScopeFeedback = "";
|
||||
let followUpTaskId: string | undefined;
|
||||
|
||||
if (shouldForkOnScopeMismatch) {
|
||||
const declaredFileScope = await this.store.parseFileScopeFromPrompt(task.id).catch(() => [] as string[]);
|
||||
const partition = partitionWorkflowRevisionFeedback(feedback, declaredFileScope);
|
||||
inScopeFeedback = partition.inScopeFeedback;
|
||||
outOfScopeFeedback = partition.outOfScopeFeedback;
|
||||
|
||||
if (outOfScopeFeedback) {
|
||||
const followUpTask = await this.createWorkflowRevisionFollowUpTask(task, stepName, outOfScopeFeedback);
|
||||
followUpTaskId = followUpTask.id;
|
||||
}
|
||||
}
|
||||
|
||||
if (!inScopeFeedback) {
|
||||
await this.store.logEntry(
|
||||
task.id,
|
||||
followUpTaskId
|
||||
? `Workflow step "${stepName}" requested revision — feedback forked to follow-up ${followUpTaskId}; original task left unchanged`
|
||||
: `Workflow step "${stepName}" requested revision — no in-scope feedback detected`,
|
||||
outOfScopeFeedback || feedback,
|
||||
this.getRunContextFor(task.id),
|
||||
);
|
||||
return false;
|
||||
}
|
||||
|
||||
const updatedTask = await this.store.getTask(task.id);
|
||||
const reopen = await this.reopenLastStepForRevision(task.id, updatedTask);
|
||||
const reopenSummary = reopen
|
||||
? `re-opening Step ${reopen.index + 1} ("${reopen.name}") for in-place fix`
|
||||
: "no step to re-open (none were completed)";
|
||||
|
||||
const logMessage = followUpTaskId
|
||||
? `Workflow step "${stepName}" requested revision — split feedback: appended in-scope guidance and forked out-of-scope work to ${followUpTaskId}; ${reopenSummary}`
|
||||
: `Workflow step "${stepName}" requested revision — feedback appended to original task; ${reopenSummary}`;
|
||||
await this.store.logEntry(task.id, logMessage, inScopeFeedback, this.getRunContextFor(task.id));
|
||||
|
||||
await this.injectWorkflowRevisionInstructions(task, inScopeFeedback);
|
||||
|
||||
await this.store.updateTask(task.id, {
|
||||
status: null,
|
||||
sessionFile: null,
|
||||
});
|
||||
|
||||
executorLog.log(`${task.id}: scheduling fresh execution after revision request`);
|
||||
this.scheduleWorkflowRerun(
|
||||
task.id,
|
||||
worktreePath,
|
||||
`${task.id}: revision rerun scheduled — moved to todo then in-progress`,
|
||||
);
|
||||
return true;
|
||||
}
|
||||
|
||||
private async createWorkflowRevisionFollowUpTask(
|
||||
task: Task,
|
||||
stepName: string,
|
||||
feedback: string,
|
||||
): Promise<Task> {
|
||||
const title = `${task.id}: workflow follow-up from ${stepName}`;
|
||||
const description = [
|
||||
`Follow-up work forked from workflow revision feedback on ${task.id}.`,
|
||||
"",
|
||||
`Original task: ${task.id}${task.title ? ` — ${task.title}` : ""}`,
|
||||
`Workflow step: ${stepName}`,
|
||||
"",
|
||||
"This feedback referenced files outside the original task's declared File Scope, so it was forked into a follow-up task instead of mutating the original PROMPT.md.",
|
||||
"",
|
||||
"## Out-of-Scope Workflow Revision Feedback",
|
||||
"",
|
||||
feedback,
|
||||
].join("\n");
|
||||
|
||||
return this.store.createTask({
|
||||
title,
|
||||
description,
|
||||
dependencies: [task.id],
|
||||
source: {
|
||||
sourceType: "workflow_step",
|
||||
sourceParentTaskId: task.id,
|
||||
sourceMetadata: {
|
||||
workflowStepName: stepName,
|
||||
routing: "scope-mismatch-fork",
|
||||
},
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Re-open the last non-pending step so a revision/failure handler gives the
|
||||
* executor exactly one pending slot to re-enter through. Returns the index
|
||||
@@ -11953,84 +11774,6 @@ export class TaskExecutor {
|
||||
return { index: targetIndex, name: steps[targetIndex].name };
|
||||
}
|
||||
|
||||
/**
|
||||
* Inject or update the "Workflow Revision Instructions" section in PROMPT.md.
|
||||
* This section contains feedback from workflow steps that requested revisions.
|
||||
* The section is replaced entirely to avoid accumulation of old feedback.
|
||||
*/
|
||||
private async injectWorkflowRevisionInstructions(
|
||||
task: Task,
|
||||
feedback: string,
|
||||
): Promise<void> {
|
||||
const promptPath = join(this.store.getFusionDir(), "tasks", task.id, "PROMPT.md");
|
||||
|
||||
// Read existing PROMPT.md
|
||||
let content: string;
|
||||
try {
|
||||
content = await readFile(promptPath, "utf-8");
|
||||
} catch {
|
||||
executorLog.warn(`${task.id}: PROMPT.md not found at ${promptPath}, skipping revision injection`);
|
||||
return;
|
||||
}
|
||||
|
||||
// All prior steps stay done — agent applies the feedback as an in-place
|
||||
// patch rather than re-planning or re-executing earlier steps.
|
||||
const scopeLine = "All prior steps remain **done**. Apply the feedback above as an in-place fix (make the necessary code changes, commit, and call `fn_task_done()` when complete). Do **not** re-run or re-plan any earlier step unless the feedback explicitly calls it out.";
|
||||
|
||||
// Check for existing Workflow Revision Instructions section
|
||||
const revisionSectionHeader = "## Workflow Revision Instructions";
|
||||
const revisionSectionContent = `${revisionSectionHeader}
|
||||
|
||||
The following feedback was received from quality gates and requires implementation changes:
|
||||
|
||||
${feedback}
|
||||
|
||||
**Important:** ${scopeLine}
|
||||
|
||||
`;
|
||||
|
||||
let newContent: string;
|
||||
if (content.includes(revisionSectionHeader)) {
|
||||
// Replace existing section
|
||||
const sectionRegex = new RegExp(
|
||||
`${revisionSectionHeader}[\\s\\S]*?(?=\\n## |\\n# |$)`,
|
||||
"i"
|
||||
);
|
||||
if (sectionRegex.test(content)) {
|
||||
newContent = content.replace(sectionRegex, revisionSectionContent);
|
||||
} else {
|
||||
// Fallback: append at end
|
||||
newContent = content + "\n" + revisionSectionContent;
|
||||
}
|
||||
} else {
|
||||
// Append new section before any closing markers or at end
|
||||
// Look for common markers like "## Acceptance Criteria" or just append
|
||||
const acceptanceCriteriaMatch = content.match(/\n##\s+Acceptance Criteria\n/);
|
||||
if (acceptanceCriteriaMatch) {
|
||||
const insertIdx = acceptanceCriteriaMatch.index!;
|
||||
newContent = content.slice(0, insertIdx) + "\n" + revisionSectionContent + content.slice(insertIdx);
|
||||
} else {
|
||||
newContent = content + "\n" + revisionSectionContent;
|
||||
}
|
||||
}
|
||||
|
||||
// Write updated content
|
||||
try {
|
||||
await writeFile(promptPath, newContent);
|
||||
executorLog.log(`${task.id}: injected workflow revision instructions into PROMPT.md`);
|
||||
} catch (err: unknown) {
|
||||
const errorMessage = err instanceof Error ? err.message : String(err);
|
||||
executorLog.error(`${task.id}: failed to inject revision instructions: ${errorMessage}`);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Handle workflow step hard failures by retrying execution up to MAX_WORKFLOW_STEP_RETRIES times.
|
||||
* This gives the executor a chance to fix workflow step failures automatically before
|
||||
* moving the task to in-review with failed status.
|
||||
*
|
||||
* @returns true if a retry was scheduled, false if retries are exhausted
|
||||
*/
|
||||
/**
|
||||
* Run deterministic verification (test + build commands) in the task's worktree.
|
||||
* Returns a structured result indicating whether all commands passed.
|
||||
@@ -12287,54 +12030,6 @@ ${failureContext.output.slice(0, VERIFICATION_LOG_MAX_CHARS)}
|
||||
}
|
||||
}
|
||||
|
||||
private async handleWorkflowStepFailure(
|
||||
task: Task,
|
||||
worktreePath: string,
|
||||
failureFeedback: string,
|
||||
stepName: string,
|
||||
): Promise<boolean> {
|
||||
this.clearCompletedTaskWatchdog(task.id);
|
||||
const currentRetries = task.workflowStepRetries ?? 0;
|
||||
|
||||
if (currentRetries >= MAX_WORKFLOW_STEP_RETRIES) {
|
||||
// Retries exhausted — caller should fall through to hard failure
|
||||
executorLog.warn(`${task.id}: workflow step "${stepName}" failed — retries exhausted (${MAX_WORKFLOW_STEP_RETRIES}/${MAX_WORKFLOW_STEP_RETRIES})`);
|
||||
return false;
|
||||
}
|
||||
|
||||
const retryCount = currentRetries + 1;
|
||||
executorLog.log(`${task.id}: workflow step "${stepName}" failed — retry ${retryCount}/${MAX_WORKFLOW_STEP_RETRIES} (executor will attempt to fix)`);
|
||||
|
||||
// 1. Update the workflowStepRetries counter on the task
|
||||
await this.store.updateTask(task.id, {
|
||||
workflowStepRetries: retryCount,
|
||||
});
|
||||
|
||||
// 2. Inject failure feedback into PROMPT.md
|
||||
await this.injectWorkflowStepFailureInstructions(task, failureFeedback, stepName, retryCount);
|
||||
|
||||
// 3. Re-open only the last step so the executor has a single pending
|
||||
// slot to re-enter. Earlier done steps stay done.
|
||||
const updatedTask = await this.store.getTask(task.id);
|
||||
await this.reopenLastStepForRevision(task.id, updatedTask);
|
||||
|
||||
// 4. Clear any session file so we get a fresh session
|
||||
await this.store.updateTask(task.id, {
|
||||
status: null,
|
||||
sessionFile: null,
|
||||
});
|
||||
|
||||
// 5. Schedule fresh execution after guard unwinds
|
||||
executorLog.log(`${task.id}: scheduling fresh execution after workflow step failure (retry ${retryCount}/${MAX_WORKFLOW_STEP_RETRIES})`);
|
||||
this.scheduleWorkflowRerun(
|
||||
task.id,
|
||||
worktreePath,
|
||||
`${task.id}: workflow step retry scheduled — moved to todo then in-progress`,
|
||||
);
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Send a task back to in-progress after verification failure.
|
||||
* Injects failure feedback into PROMPT.md, resets steps, clears session,
|
||||
@@ -12776,14 +12471,6 @@ ${failureFeedback}
|
||||
|
||||
// ── Worktree management ────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* Create a git worktree at `path` on a new branch.
|
||||
*
|
||||
* @param branch — Branch name (e.g., `fusion/fn-042`)
|
||||
* @param path — Absolute worktree directory path
|
||||
* @param startPoint — Optional git ref to branch from (e.g., `fusion/fn-041`).
|
||||
* When provided, the worktree starts from that ref instead of HEAD.
|
||||
*/
|
||||
/**
|
||||
* Execute a script-mode workflow step by resolving the scriptName to a command
|
||||
* from project settings and running it in the task worktree.
|
||||
|
||||
@@ -6016,11 +6016,10 @@ export class SelfHealingManager {
|
||||
* Recover `in-review` tasks parked by a failed pre-merge workflow step.
|
||||
*
|
||||
* When a pre-merge workflow step (e.g. Browser Verification) fails during an
|
||||
* active executor run, `executor.handleWorkflowStepFailure` retries up to
|
||||
* `MAX_WORKFLOW_STEP_RETRIES` times in-session. If all retries exhaust the
|
||||
* task ends up in `in-review` with the failed workflow step result still on
|
||||
* record, which `getTaskMergeBlocker` correctly treats as a merge block —
|
||||
* leaving the task stranded with no live session to un-stick it.
|
||||
* active executor run, the graph workflow records the failed step result and
|
||||
* the task ends up in `in-review` with that failed result still on record,
|
||||
* which `getTaskMergeBlocker` correctly treats as a merge block — leaving the
|
||||
* task stranded with no live session to un-stick it.
|
||||
*
|
||||
* This scan delegates back to the executor's `recoverFailedPreMergeWorkflowStep`
|
||||
* path (which reuses the same `sendTaskBackForFix` flow the executor uses
|
||||
|
||||
Reference in New Issue
Block a user