fix(FN-8288): preserve approved review and recovery state

Fusion-Task-Id: FN-8288
This commit is contained in:
gsxdsm
2026-07-18 17:24:49 -07:00
parent b8cd3d21fb
commit 358b628b8d
4 changed files with 123 additions and 31 deletions

View File

@@ -0,0 +1,7 @@
---
"@runfusion/fusion": patch
---
summary: Preserve approved task scope during review and committed work during worktree recovery.
category: fix
dev: Injects approved PROMPT.md into review gates and reclaims inactive same-task branches before cleanup.

View File

@@ -857,6 +857,35 @@ describe("CE workflow-step executor integration", () => {
// ── Item 5: FUSION_HEADLESS gating on stepEnv ───────────────────────────────
describe("executeWorkflowStep FUSION_HEADLESS (U3)", () => {
it("makes the approved PROMPT.md contract authoritative during Code Review", async () => {
const store = createMockStore();
const { executor } = makeExecutor(store);
const cap = captureSession();
vi.spyOn(executor as any, "readTaskArtifact").mockResolvedValue(`
# Approved contract
Ship FIVE kinds. Do NOT add roadmap-item in this task.
`.trim());
await (executor as any).executeWorkflowStep(
baseStepTask({ description: "Original request: ship SIX kinds including roadmap-item." }),
makeStep({
id: "graph:code-review-step",
name: "Code Review",
optionalGroupId: "code-review",
prompt: "Review the implementation against the approved task contract.",
gateMode: "gate",
}),
"/tmp/wt",
{},
);
expect(cap.last?.systemPrompt).toContain("--- BEGIN APPROVED PROMPT.md ---");
expect(cap.last?.systemPrompt).toContain("Ship FIVE kinds. Do NOT add roadmap-item in this task.");
expect(cap.last?.systemPrompt).toContain("PROMPT.md is the authoritative current contract");
expect(cap.last?.systemPrompt).toContain("Do not enforce superseded requirements from the original Task Description");
});
it("sets FUSION_HEADLESS=1 only when unattended=true; always sets FUSION_WORKFLOW_STEP", async () => {
const store = createMockStore();
const { executor } = makeExecutor(store);

View File

@@ -878,6 +878,40 @@ describe("TaskExecutor worktree recovery", () => {
);
});
it("reclaims an inactive same-task conflict when the branch preserves task commits", async () => {
const store = createMockStore();
const executor = new TaskExecutor(store, "/tmp/test");
const conflictPath = "/tmp/test/.worktrees/light-cedar";
vi.spyOn(executor as any, "shouldGenerateNewWorktreeName").mockResolvedValue(false);
const cleanup = vi.spyOn(executor as any, "cleanupConflictingWorktree").mockResolvedValue(true);
vi.spyOn(branchConflictModule, "inspectBranchConflict").mockResolvedValueOnce({
kind: "reclaimable",
livePath: conflictPath,
tipSha: "70b47804bc6f27659638e17ac7cf279ed343ff6f",
taskAttributedCommitCount: 10,
strandedCommits: [{ sha: "70b47804bc6f27659638e17ac7cf279ed343ff6f", subject: "fix(FN-8288): preserve implementation" }],
} as any);
const result = await (executor as any).handleWorktreeConflict(
conflictPath,
"fusion/fn-8288",
"/tmp/test/.worktrees/pearl-otter",
"FN-8288",
"main",
0,
false,
{},
);
expect(result).toEqual({ path: conflictPath, branch: "fusion/fn-8288" });
expect(cleanup).not.toHaveBeenCalled();
expect(store.logEntry).toHaveBeenCalledWith(
"FN-8288",
expect.stringContaining("10 commits preserved"),
"70b47804bc6f27659638e17ac7cf279ed343ff6f",
);
});
it("records recovery context when handling a branch conflict (FN-4847: now discards + requeues instead of pausing)", async () => {
// FN-4847: branch-conflict-unrecoverable previously paused the task with
// status=failed + pausedReason="branch-conflict-unrecoverable". The user has

View File

@@ -16436,10 +16436,11 @@ ${scopeGuard}
* FNXC:PlanReviewSpecInjection 2026-07-05-17:20:
* FN-7561: the Plan Review reviewer runs readonly with cwd=worktree, but the spec artifact lives at the project root under `.fusion/tasks/<id>/PROMPT.md` — OUTSIDE the task worktree. Instructing the agent to "Read PROMPT.md" therefore had it search the worktree, fail to find the file, and emit "no PROMPT.md file was found / task data lives in a DB" prose instead of a parseable verdict. That malformed/hard-failed output fed the unbounded triage↔plan-review replan loop (FN-7525 looped 13+ times overnight; FN-7575 too). Load the spec text from the store (document layer → on-disk PROMPT.md) ONCE and inject it directly into the reviewer prompt so the verdict never depends on the agent locating the file. Read from the store, not fs, so it is correct regardless of worktree vs project-root layout.
*/
const planReviewSpecArtifact = isPlanReviewStep
const workflowReviewSpecArtifact = isReviewTypeWorkflowStep
? await this.readTaskArtifact(task.id, "PROMPT.md")
: undefined;
const planReviewSpecText = typeof planReviewSpecArtifact === "string" ? planReviewSpecArtifact : "";
const workflowReviewSpecText = typeof workflowReviewSpecArtifact === "string" ? workflowReviewSpecArtifact : "";
const planReviewSpecText = isPlanReviewStep ? workflowReviewSpecText : "";
if (isPlanReviewStep && requireExternalIntegrationEvidence) {
/*
@@ -16500,6 +16501,18 @@ ${scopeGuard}
* unrelated local commits can make a plan-only gate reject implementation
* state and loop back to triage after the planner already approved the spec.
*/
const approvedContractBlock = !isPlanReviewStep && workflowReviewSpecText
? `
Approved Task Contract:
- PROMPT.md is the authoritative current contract for this review. It includes any approved planning revisions and scope decisions.
- The Task Description is historical input only. Do not enforce superseded requirements from the original Task Description when they conflict with PROMPT.md.
- Do not request behavior that PROMPT.md explicitly defers, excludes, or forbids. Review the implementation against the approved contract reproduced below.
--- BEGIN APPROVED PROMPT.md ---
${workflowReviewSpecText}
--- END APPROVED PROMPT.md ---`
: "";
const scopeBlock = isPlanReviewStep
? `Plan Review Scope:
- Review the task plan artifact (PROMPT.md), reproduced verbatim below, and task metadata only.
@@ -16516,7 +16529,7 @@ ${scopeFileBlock}${diffShortstat ? `\nDiff stat: ${diffShortstat}` : ""}
CRITICAL SCOPING RULES — read before doing anything else:
- Review ONLY the files listed above. Do NOT analyze unmodified files or unrelated parts of the codebase.
- If NONE of the files in the diff scope are relevant to your review category (e.g. a UX/design reviewer with no UI/CSS/component files in scope, a security reviewer with no auth/network code in scope, an a11y reviewer with no markup changes), respond IMMEDIATELY with a single short approval line such as "No relevant changes in scope — approved." and STOP. Do not start exploring the codebase.
- Your wall-clock budget is short. Spending it browsing unmodified files will cause this step to time out and block merge.`;
- Your wall-clock budget is short. Spending it browsing unmodified files will cause this step to time out and block merge.${approvedContractBlock}`;
const latestTaskForUserComments = await this.store.getTask(task.id).catch(() => task);
const workflowStepUserComments = selectUserCommentsForAgentContext(latestTaskForUserComments, { limit: null });
@@ -18196,17 +18209,44 @@ You have access to the file system to review changes.${inlineFixBlock}${verdictB
taskId,
);
if (shouldGenerateNewName) {
const inspection = await inspectBranchConflict({
repoDir: this.rootDir,
branchName: branch,
conflictingWorktreePath: conflictPath,
requestingTaskId: taskId,
ownerTaskId: taskId,
startPoint,
integrationRef: await resolveIntegrationBranch(this.rootDir, settings),
});
/*
* FNXC:ExecutorWorktree 2026-07-18-17:20:
* Inspect every branch/worktree collision before cleanup, including inactive
* same-task bindings. The old inactive path skipped inspection and called
* cleanupConflictingWorktree directly, which force-deleted a branch carrying
* completed task commits during workflow-node recovery. Liveness determines
* whether a sibling checkout is needed; it must never determine whether task
* history is disposable.
*/
const inspection = await inspectBranchConflict({
repoDir: this.rootDir,
branchName: branch,
conflictingWorktreePath: conflictPath,
requestingTaskId: taskId,
ownerTaskId: taskId,
startPoint,
integrationRef: await resolveIntegrationBranch(this.rootDir, settings),
});
if (inspection.kind === "reclaimable") {
await this.store.logEntry(
taskId,
`[recovery] reclaimed existing worktree for ${taskId} at ${inspection.livePath} (${inspection.taskAttributedCommitCount} commits preserved)`,
inspection.tipSha,
);
return { path: inspection.livePath, branch };
}
if (inspection.kind === "fully-subsumed") {
await this.store.logEntry(
taskId,
`[recovery] reclaimed existing worktree for ${taskId} at ${inspection.livePath} (0 commits preserved)`,
inspection.tipSha,
);
return { path: inspection.livePath, branch };
}
if (shouldGenerateNewName) {
if (inspection.kind === "stale" || inspection.kind === "stale-resolved" || inspection.kind === "tip-already-merged") {
const cleanupSuccess = await this.cleanupConflictingWorktree(conflictPath, branch, taskId);
if (cleanupSuccess) {
@@ -18220,24 +18260,6 @@ You have access to the file system to review changes.${inlineFixBlock}${verdictB
// worktree name.
}
if (inspection.kind === "reclaimable") {
await this.store.logEntry(
taskId,
`[recovery] reclaimed existing worktree for ${taskId} at ${inspection.livePath} (${inspection.taskAttributedCommitCount} commits preserved)`,
inspection.tipSha,
);
return { path: inspection.livePath, branch };
}
if (inspection.kind === "fully-subsumed") {
await this.store.logEntry(
taskId,
`[recovery] reclaimed existing worktree for ${taskId} at ${inspection.livePath} (0 commits preserved)`,
inspection.tipSha,
);
return { path: inspection.livePath, branch };
}
if (inspection.kind === "live-foreign") {
const cleanupSuccess = await this.cleanupConflictingWorktree(inspection.livePath, branch, taskId);
if (cleanupSuccess) {