From cb128f58248eeb5a4929d9e78151dfd53505d7cd Mon Sep 17 00:00:00 2001 From: Phil Larson Date: Fri, 12 Jun 2026 14:37:01 -0700 Subject: [PATCH] test(FN-424): cover plan-only no-commit finalization Fusion-Task-Id: FN-424 Co-authored-by: Fusion --- .../executor-task-done-invariant.test.ts | 106 ++++++++++++++++++ packages/engine/src/executor.ts | 99 +++++++++++++++- 2 files changed, 203 insertions(+), 2 deletions(-) diff --git a/packages/engine/src/__tests__/executor-task-done-invariant.test.ts b/packages/engine/src/__tests__/executor-task-done-invariant.test.ts index dcf3710d2b..8cfa431ace 100644 --- a/packages/engine/src/__tests__/executor-task-done-invariant.test.ts +++ b/packages/engine/src/__tests__/executor-task-done-invariant.test.ts @@ -9,6 +9,51 @@ import * as worktreePool from "../worktree-pool.js"; import { TaskStore } from "@fusion/core"; import { createMockStore, mockedCreateFnAgent, mockedExec, mockedExecSync, resetExecutorMocks } from "./executor-test-helpers.js"; +const fn416Prompt = `# Task: FN-416 - Assign ready implementation task to active owner + +**Created:** 2026-06-12 +**Size:** S + +## Review Level: 1 (Plan Only) + +**Assessment:** This is an operational routing task with no expected product-source changes. + +## Mission +Assign or route exactly one ready implementation task to an eligible active owner, or record an intentional no-route state. No source files expected. + +## File Scope + +- FN-416 task document docs via fn_task_document_write +- .fusion/tasks/FN-416/ task log evidence only + +## Steps + +### Step 0: Preflight +- [x] Check board state + +### Step 1: Route exactly one existing ready task or record no-route +- [x] Record evidence in task documents/logs +`; + +const sourceChangingPlanOnlyPrompt = `# Task: FN-999 - Implement source fix + +**Size:** S + +## Review Level: 1 (Plan Only) + +## Mission +Implement a source-changing bug-fix in the executor. + +## File Scope + +- packages/engine/src/executor.ts + +## Steps + +### Step 1: Implement +- [ ] Change source +`; + function baseTask(overrides: Record = {}) { return { id: "FN-4114", @@ -142,6 +187,67 @@ describe("FN-4114 fn_task_done invariants", () => { expect(store.moveTask).toHaveBeenCalledWith("FN-4114", "todo", { preserveProgress: true }); }); + + it("FN-416 allows plan-only operational no-source completion with zero commits when the explicit flag is missing", async () => { + const { store, tool } = await setup({ + id: "FN-416", + title: "Assign ready implementation task to active owner", + description: "Operational routing task with no expected product-source changes; record routing evidence or no-route state.", + reviewLevel: 1, + prompt: fn416Prompt, + sourceMetadata: { fileScope: ["FN-416 task document docs via fn_task_document_write"] }, + log: [{ timestamp: new Date().toISOString(), action: "Routing evidence recorded", outcome: "No-route state documented in task docs" }], + steps: [ + { name: "Preflight", status: "done" as const }, + { name: "Route or record no-route", status: "done" as const }, + ], + }); + mockedExecSync.mockImplementation((cmd: string) => { + if (cmd.includes("rev-parse --show-toplevel")) return Buffer.from("/repo/.worktrees/swift-falcon\n"); + if (cmd.includes("rev-parse --abbrev-ref HEAD")) return Buffer.from("fusion/fn-4114\n"); + if (cmd.includes("rev-list --count")) return Buffer.from("0\n"); + if (cmd.includes("rev-parse HEAD")) return Buffer.from("def456\n"); + return Buffer.from(""); + }); + + const result = await tool.execute("id", {}); + expect(result.content[0].text).toContain("Task marked complete"); + expect(store.moveTask).not.toHaveBeenCalledWith("FN-416", "todo", { preserveProgress: true }); + expect(store.handoffToReview).not.toHaveBeenCalledWith("FN-416", expect.objectContaining({ + evidence: expect.objectContaining({ reason: "invariant-check-failed" }), + })); + expect(store.logEntry).toHaveBeenCalledWith( + "FN-4114", + expect.stringContaining("prompt/source metadata derived operational no-commit contract"), + undefined, + undefined, + ); + const revListCalled = mockedExecSync.mock.calls.some(([cmd]) => String(cmd).includes("rev-list --count")); + expect(revListCalled).toBe(false); + }); + + it("FN-416 keeps the missing-commit guard for source-changing plan-only tasks without an explicit contract", async () => { + const { store, tool } = await setup({ + title: "Implement executor fix", + description: "Plan Only but requires source-changing implementation work.", + reviewLevel: 1, + prompt: sourceChangingPlanOnlyPrompt, + sourceMetadata: { fileScope: ["packages/engine/src/executor.ts"] }, + steps: [{ name: "Implement", status: "done" as const }], + }); + mockedExecSync.mockImplementation((cmd: string) => { + if (cmd.includes("rev-parse --show-toplevel")) return Buffer.from("/repo/.worktrees/swift-falcon\n"); + if (cmd.includes("rev-parse --abbrev-ref HEAD")) return Buffer.from("fusion/fn-4114\n"); + if (cmd.includes("rev-list --count")) return Buffer.from("0\n"); + if (cmd.includes("rev-parse HEAD")) return Buffer.from("def456\n"); + return Buffer.from(""); + }); + + const result = await tool.execute("id", {}); + expect(result.content[0].text).toContain("fn_task_done refused: no_commits"); + expect(store.moveTask).toHaveBeenCalledWith("FN-4114", "todo", { preserveProgress: true }); + }); + it("FN-4114 allows fn_task_done on valid worktree/branch/commit state", async () => { const { store, tool } = await setup(); const result = await tool.execute("id", {}); diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index 7ce9123200..0a649c5c58 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -581,6 +581,92 @@ export function parseReviewLevelFromPrompt(prompt: string): number { return reviewMatch ? parseInt(reviewMatch[1], 10) : 0; } +function extractPromptSection(prompt: string, heading: string): string { + const escaped = heading.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + const pattern = new RegExp(`^##\\s+${escaped}\\s*$([\\s\\S]*?)(?=^##\\s+|$(?![\\s\\S]))`, "im"); + return pattern.exec(prompt)?.[1]?.trim() ?? ""; +} + +function extractPromptListEntries(section: string): string[] { + return section + .split(/\r?\n/) + .map((line) => line.trim()) + .map((line) => line.replace(/^[-*]\s+/, "").replace(/^`([^`]+)`.*$/, "$1").trim()) + .filter(Boolean); +} + +function isNoSourceScopeEntry(entry: string): boolean { + const normalized = entry.toLowerCase(); + return ( + normalized.includes("no source") || + normalized.includes("no product-source") || + normalized.includes("no code") || + normalized.includes("no file mutations") || + normalized.includes("task document") || + normalized.includes("task log") || + normalized.includes("agent log") || + normalized.includes("read-only evidence") || + normalized.startsWith(".fusion/tasks/") || + normalized.startsWith("/.fusion/tasks/") + ); +} + +function hasSourceChangingScopeEntry(entry: string): boolean { + const normalized = entry.toLowerCase(); + if (!normalized || isNoSourceScopeEntry(normalized)) return false; + if (normalized.includes("read-only")) return false; + if (/\b(source|sources|packages|tests|src|app|docs|scripts|\.changeset)\b/.test(normalized)) return true; + return /\.(ts|tsx|js|jsx|mjs|cjs|swift|kt|java|py|rs|go|rb|md|json|ya?ml|toml|css|scss|html)\b/.test(normalized); +} + +function getTaskTextForNoCommitEligibility(task: Task, promptContent: string): string { + const logText = (task.log ?? []) + .map((entry) => `${entry.action ?? ""}\n${entry.outcome ?? ""}`) + .join("\n"); + const sourceMetadata = task.sourceMetadata ? JSON.stringify(task.sourceMetadata) : ""; + return [task.title, task.description, promptContent, sourceMetadata, logText] + .filter((part): part is string => typeof part === "string" && part.length > 0) + .join("\n"); +} + +function evaluatePromptDerivedNoCommitEligibility(task: Task, promptContent: string): { eligible: boolean; reason?: string } { + const combined = getTaskTextForNoCommitEligibility(task, promptContent).toLowerCase(); + const reviewLevel = typeof task.reviewLevel === "number" ? task.reviewLevel : parseReviewLevelFromPrompt(promptContent); + const isPlanOnly = reviewLevel === 1 && (/plan\s*only/.test(combined) || combined.includes("plan-only")); + if (!isPlanOnly) return { eligible: false }; + + const explicitNoSourceIntent = [ + "no expected product-source changes", + "no product-source changes", + "no source changes expected", + "no source files expected", + "no code changes expected", + "no expected source changes", + "no file mutations", + "no source/config/file mutations", + ].some((phrase) => combined.includes(phrase)); + if (!explicitNoSourceIntent) return { eligible: false }; + + const excludedImplementationIntent = /\b(investigate and fix|fix if needed|implement|source-changing|code change|docs\/tests changes|documentation change|bug[- ]fix|feature)\b/.test(combined); + const operationalIntent = /\b(operational|routing|route|assign|assignment|owner|handoff|coordination|coordinate|no-route|triage)\b/.test(combined); + if (!operationalIntent || excludedImplementationIntent) return { eligible: false }; + + const promptScopeEntries = extractPromptListEntries(extractPromptSection(promptContent, "File Scope")); + const metadataScope = Array.isArray(task.sourceMetadata?.fileScope) + ? task.sourceMetadata.fileScope.filter((entry): entry is string => typeof entry === "string") + : []; + const declaredScope = [...promptScopeEntries, ...metadataScope]; + if (declaredScope.some(hasSourceChangingScopeEntry)) return { eligible: false }; + + const stepsComplete = Array.isArray(task.steps) && task.steps.length > 0 + ? task.steps.every((step) => step.status === "done" || step.status === "skipped") + : false; + const hasOperationalEvidence = /\b(evidence|recorded|documented|no-route|routed|assigned|handoff|decision)\b/.test(combined); + if (!stepsComplete && !hasOperationalEvidence) return { eligible: false }; + + return { eligible: true, reason: "prompt/source metadata derived operational no-commit contract" }; +} + export function partitionWorkflowRevisionFeedback( feedback: string, declaredFileScope: readonly string[], @@ -9419,8 +9505,17 @@ export class TaskExecutor { }; } - if (task.noCommitsExpected === true) { - executorLog.log(`${task.id}: fn_task_done no_commits guard skipped (noCommitsExpected=true)`); + const noCommitEligibility = task.noCommitsExpected === true + ? { eligible: true, reason: "noCommitsExpected=true" } + : evaluatePromptDerivedNoCommitEligibility(task, typeof task.prompt === "string" ? task.prompt : ""); + if (noCommitEligibility.eligible) { + executorLog.log(`${task.id}: fn_task_done no_commits guard skipped (${noCommitEligibility.reason})`); + await this.store.logEntry( + task.id, + `fn_task_done no_commits guard skipped (${noCommitEligibility.reason})`, + undefined, + this.getRunContextFor(task.id), + ); return { ok: true }; }