feat(FN-5112): complete Step 2 — gate fn_review_spec on dangling-ref detector
Fusion-Task-Id: FN-5112 Fusion-Task-Lineage: 692fba2a-5856-4256-8144-d87bda56e1aa
This commit is contained in:
committed by
gsxdsm
parent
b5e257d6e3
commit
574d661134
@@ -0,0 +1,140 @@
|
||||
import { describe, it, expect, vi } from "vitest";
|
||||
import { mkdtemp, mkdir, writeFile, rm } from "node:fs/promises";
|
||||
import { join } from "node:path";
|
||||
import { tmpdir } from "node:os";
|
||||
import type { TaskStore, TaskDetail, Settings } from "@fusion/core";
|
||||
import { TriageProcessor } from "../triage.js";
|
||||
|
||||
const { mockReviewStep } = vi.hoisted(() => ({
|
||||
mockReviewStep: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("../reviewer.js", () => ({
|
||||
reviewStep: mockReviewStep,
|
||||
}));
|
||||
|
||||
vi.mock("@fusion/core", async (importOriginal) => {
|
||||
const { createEngineCoreMock } = await import("../test/mockCore.js");
|
||||
return createEngineCoreMock(() => importOriginal<typeof import("@fusion/core")>(), {
|
||||
resolveAgentPrompt: vi.fn().mockReturnValue(null),
|
||||
});
|
||||
});
|
||||
|
||||
function createMockStore(overrides: Partial<TaskStore> = {}): TaskStore {
|
||||
return {
|
||||
getTask: vi.fn(),
|
||||
listTasks: vi.fn().mockResolvedValue([]),
|
||||
createTask: vi.fn(),
|
||||
moveTask: vi.fn(),
|
||||
updateTask: vi.fn().mockResolvedValue(undefined),
|
||||
deleteTask: vi.fn(),
|
||||
mergeTask: vi.fn(),
|
||||
getSettings: vi.fn().mockResolvedValue({
|
||||
maxConcurrent: 2,
|
||||
maxWorktrees: 4,
|
||||
pollIntervalMs: 10000,
|
||||
groupOverlappingFiles: false,
|
||||
autoMerge: true,
|
||||
} as Settings),
|
||||
updateSettings: vi.fn(),
|
||||
logEntry: vi.fn().mockResolvedValue(undefined),
|
||||
appendAgentLog: vi.fn().mockResolvedValue(undefined),
|
||||
getAgentLogs: vi.fn().mockResolvedValue([]),
|
||||
addSteeringComment: vi.fn(),
|
||||
parseDependenciesFromPrompt: vi.fn().mockResolvedValue([]),
|
||||
parseStepsFromPrompt: vi.fn().mockResolvedValue([]),
|
||||
parseFileScopeFromPrompt: vi.fn().mockResolvedValue([]),
|
||||
on: vi.fn(),
|
||||
emit: vi.fn(),
|
||||
...overrides,
|
||||
} as unknown as TaskStore;
|
||||
}
|
||||
|
||||
const mockTaskDetail: TaskDetail = {
|
||||
id: "FN-5112",
|
||||
description: "Test task",
|
||||
column: "triage",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: "2026-01-01T00:00:00.000Z",
|
||||
updatedAt: "2026-01-01T00:00:00.000Z",
|
||||
prompt: "# Task\n",
|
||||
attachments: [],
|
||||
comments: [],
|
||||
};
|
||||
|
||||
describe("triage fn_review_spec dangling references", () => {
|
||||
it("short-circuits to REVISE without invoking reviewer", async () => {
|
||||
const rootDir = await mkdtemp(join(tmpdir(), "fusion-triage-dangling-"));
|
||||
try {
|
||||
const taskId = "FN-5112";
|
||||
const promptPath = `.fusion/tasks/${taskId}/PROMPT.md`;
|
||||
await mkdir(join(rootDir, ".fusion", "tasks", taskId), { recursive: true });
|
||||
await writeFile(join(rootDir, promptPath), "## Steps\n### Step 0: Preflight\n- Read .fusion/tasks/FN-5112/notes.md\n");
|
||||
|
||||
const store = createMockStore({
|
||||
getTask: vi.fn().mockResolvedValue({ ...mockTaskDetail, id: taskId }),
|
||||
});
|
||||
|
||||
const processor = new TriageProcessor(store, rootDir);
|
||||
const verdictRef = { current: null as any };
|
||||
const tool = (processor as any).createReviewSpecTool(
|
||||
taskId,
|
||||
promptPath,
|
||||
{ current: null },
|
||||
{ current: null },
|
||||
verdictRef,
|
||||
{ current: "" },
|
||||
{},
|
||||
false,
|
||||
);
|
||||
|
||||
const result = await tool.execute({});
|
||||
expect(String(result.content[0]?.text)).toContain("REVISE");
|
||||
expect(String(result.content[0]?.text)).toContain("notes.md");
|
||||
expect(verdictRef.current).toBe("REVISE");
|
||||
expect(mockReviewStep).not.toHaveBeenCalled();
|
||||
} finally {
|
||||
await rm(rootDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("falls through to reviewer when referenced file is declared as new artifact", async () => {
|
||||
const rootDir = await mkdtemp(join(tmpdir(), "fusion-triage-dangling-ok-"));
|
||||
try {
|
||||
const taskId = "FN-5112";
|
||||
const promptPath = `.fusion/tasks/${taskId}/PROMPT.md`;
|
||||
await mkdir(join(rootDir, ".fusion", "tasks", taskId), { recursive: true });
|
||||
await writeFile(
|
||||
join(rootDir, promptPath),
|
||||
"## Steps\n### Step 1: Create\n- Read .fusion/tasks/FN-5112/notes.md\n\n**Artifacts:**\n- `.fusion/tasks/FN-5112/notes.md` (new)\n",
|
||||
);
|
||||
|
||||
mockReviewStep.mockResolvedValueOnce({ verdict: "APPROVE", summary: "ok", review: "" });
|
||||
const store = createMockStore({
|
||||
getTask: vi.fn().mockResolvedValue({ ...mockTaskDetail, id: taskId }),
|
||||
});
|
||||
const processor = new TriageProcessor(store, rootDir);
|
||||
const verdictRef = { current: null as any };
|
||||
const tool = (processor as any).createReviewSpecTool(
|
||||
taskId,
|
||||
promptPath,
|
||||
{ current: null },
|
||||
{ current: null },
|
||||
verdictRef,
|
||||
{ current: "" },
|
||||
{},
|
||||
false,
|
||||
);
|
||||
|
||||
const result = await tool.execute({});
|
||||
expect(result.content[0]?.text).toBe("APPROVE");
|
||||
expect(verdictRef.current).toBe("APPROVE");
|
||||
expect(mockReviewStep).toHaveBeenCalledTimes(1);
|
||||
} finally {
|
||||
await rm(rootDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -30,6 +30,7 @@ import {
|
||||
resolvePlanningSessionModel,
|
||||
} from "./agent-session-helpers.js";
|
||||
import { reviewStep, type ReviewVerdict } from "./reviewer.js";
|
||||
import { detectDanglingTaskDocReferences, formatDanglingDiagnostic } from "./spec-validation/task-document-references.js";
|
||||
import { buildSessionSkillContext } from "./session-skill-context.js";
|
||||
import { PRIORITY_SPECIFY, type AgentSemaphore } from "./concurrency.js";
|
||||
import { AgentLogger } from "./agent-logger.js";
|
||||
@@ -310,6 +311,8 @@ You MUST call \`fn_review_spec()\` after writing the PROMPT.md. Do not finish wi
|
||||
- Bad: generic wording, vague steps ("implement feature"), missing tests, or file scope that cannot realistically satisfy requested behavior.
|
||||
- Good file scope estimation includes likely touched tests, config, and integration files — not only the obvious implementation file.
|
||||
|
||||
Never reference a \`.fusion/tasks/<id>/<file>\` artifact in Context, Steps, or File Scope unless (a) the file already exists, (b) the step explicitly creates it (listed as \`(new)\` under Artifacts), or (c) it is \`PROMPT.md\` / \`task.json\` / \`attachments/*\` for a sibling task. Save planning scratch as task documents via \`fn_task_document_write\`, not as files on disk.
|
||||
|
||||
## Output
|
||||
Write the PROMPT.md directly using the write tool, then call \`fn_review_spec()\` for review.
|
||||
|
||||
@@ -517,6 +520,8 @@ After writing the PROMPT.md, call \`fn_review_spec()\` to confirm the spec.
|
||||
|
||||
Fast-mode specs are auto-approved — the review tool will return APPROVE immediately without spawning an independent reviewer. You do NOT need to wait for or iterate on review feedback.
|
||||
|
||||
Never reference a \`.fusion/tasks/<id>/<file>\` artifact in Context, Steps, or File Scope unless (a) the file already exists, (b) the step explicitly creates it (listed as \`(new)\` under Artifacts), or (c) it is \`PROMPT.md\` / \`task.json\` / \`attachments/*\` for a sibling task. Save planning scratch as task documents via \`fn_task_document_write\`, not as files on disk.
|
||||
|
||||
## Output
|
||||
Write the PROMPT.md directly using the write tool, then call \`fn_review_spec()\` to confirm.`;
|
||||
|
||||
@@ -1999,6 +2004,21 @@ export class TriageProcessor {
|
||||
};
|
||||
}
|
||||
|
||||
const danglingRefs = await detectDanglingTaskDocReferences(promptContent, {
|
||||
rootDir,
|
||||
taskId,
|
||||
});
|
||||
if (danglingRefs.length > 0) {
|
||||
const diagnostic = formatDanglingDiagnostic(danglingRefs);
|
||||
specReviewVerdictRef.current = "REVISE";
|
||||
planLog.warn(`${taskId}: ${diagnostic}`);
|
||||
await store.logEntry(taskId, "Spec review: REVISE (dangling task-document references)");
|
||||
return {
|
||||
content: [{ type: "text" as const, text: diagnostic }],
|
||||
details: {},
|
||||
};
|
||||
}
|
||||
|
||||
// Re-read task detail to get latest user comments
|
||||
const currentDetail = await store.getTask(taskId);
|
||||
const currentUserComments = (currentDetail.comments || []).filter(
|
||||
|
||||
Reference in New Issue
Block a user