feat(FN-3939): audit and patch PR #59 residual defects, add retry parity an
Completes FN-3939 audit work for PR #59 residual defects by refactoring the mission execution loop in the engine (halving its complexity), adding self-healing test coverage, and wiring in task workflow route improvements. Retry and validation behavior updates are documented in the changeset and skil Fusion-Task-Id: FN-3939
This commit is contained in:
@@ -472,7 +472,7 @@ describe("MissionExecutionLoop", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("creates validation board task when feature has assertions", async () => {
|
||||
it("does NOT create a board task for single-feature validation", async () => {
|
||||
const feature = createMockFeature({ loopState: "implementing", taskId: "FN-001", sliceId: "SL-001" });
|
||||
missionStore._setFeature(feature);
|
||||
taskStore._setTask({ id: "FN-001", title: "Test", description: "Test task", log: [] });
|
||||
@@ -490,17 +490,11 @@ describe("MissionExecutionLoop", () => {
|
||||
|
||||
await loop.processTaskOutcome("FN-001");
|
||||
|
||||
// Should create a validation board task
|
||||
expect(taskStore.createTask).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
title: expect.stringContaining("Validate:"),
|
||||
column: "in-progress",
|
||||
sliceId: "SL-001",
|
||||
}),
|
||||
);
|
||||
expect(taskStore.createTask).toHaveBeenCalledTimes(0);
|
||||
expect(missionStore.startValidatorRun).toHaveBeenCalledWith("F-001", "task_completion");
|
||||
});
|
||||
|
||||
it("sets validation task status to mission-validation", async () => {
|
||||
it("does NOT set mission-validation status on any task", async () => {
|
||||
const feature = createMockFeature({ loopState: "implementing", taskId: "FN-001", sliceId: "SL-001" });
|
||||
missionStore._setFeature(feature);
|
||||
taskStore._setTask({ id: "FN-001", title: "Test", description: "Test task", log: [] });
|
||||
@@ -518,14 +512,13 @@ describe("MissionExecutionLoop", () => {
|
||||
|
||||
await loop.processTaskOutcome("FN-001");
|
||||
|
||||
// Should update the task status to mission-validation
|
||||
expect(taskStore.updateTask).toHaveBeenCalledWith(
|
||||
expect(taskStore.updateTask).not.toHaveBeenCalledWith(
|
||||
expect.any(String),
|
||||
expect.objectContaining({ status: "mission-validation" }),
|
||||
);
|
||||
});
|
||||
|
||||
it("passes taskId to startValidatorRun", async () => {
|
||||
it("calls startValidatorRun without a board task ID", async () => {
|
||||
const feature = createMockFeature({ loopState: "implementing", taskId: "FN-001", sliceId: "SL-001" });
|
||||
missionStore._setFeature(feature);
|
||||
taskStore._setTask({ id: "FN-001", title: "Test", description: "Test task", log: [] });
|
||||
@@ -533,8 +526,6 @@ describe("MissionExecutionLoop", () => {
|
||||
missionStore.listAssertionsForFeature = vi.fn().mockReturnValue([
|
||||
{ id: "CA-1", milestoneId: "MS-001", title: "Test assertion", assertion: "Should work", status: "pending" as const, orderIndex: 0, createdAt: new Date().toISOString(), updatedAt: new Date().toISOString() },
|
||||
]);
|
||||
// Make createTask return a predictable ID
|
||||
taskStore.createTask = vi.fn().mockResolvedValue({ id: "KB-999" });
|
||||
|
||||
loop = new MissionExecutionLoop({
|
||||
taskStore: taskStore as any,
|
||||
@@ -545,11 +536,9 @@ describe("MissionExecutionLoop", () => {
|
||||
|
||||
await loop.processTaskOutcome("FN-001");
|
||||
|
||||
// Should pass the created task ID to startValidatorRun
|
||||
expect(missionStore.startValidatorRun).toHaveBeenCalledWith(
|
||||
"F-001",
|
||||
"task_completion",
|
||||
"KB-999",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -2310,6 +2310,65 @@ describe("SelfHealingManager", () => {
|
||||
managerWithRecovery.stop();
|
||||
});
|
||||
|
||||
it("detects stale in-review task using columnMovedAt when available", async () => {
|
||||
const managerWithRecovery = new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
});
|
||||
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({ taskStuckTimeoutMs: 60_000 });
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([
|
||||
{
|
||||
id: "FN-407-test-1",
|
||||
column: "in-review",
|
||||
paused: false,
|
||||
status: null,
|
||||
columnMovedAt: new Date(Date.now() - 120_000).toISOString(),
|
||||
updatedAt: new Date(Date.now() - 5_000).toISOString(),
|
||||
steps: [
|
||||
{ name: "Step 0", status: "done" },
|
||||
{ name: "Step 1", status: "in-progress" },
|
||||
],
|
||||
workflowStepResults: [],
|
||||
mergeDetails: undefined,
|
||||
log: [],
|
||||
},
|
||||
]);
|
||||
|
||||
const result = await managerWithRecovery.recoverStaleIncompleteReviewTasks();
|
||||
|
||||
expect(result).toBe(1);
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-407-test-1", "todo", { preserveProgress: true });
|
||||
|
||||
managerWithRecovery.stop();
|
||||
});
|
||||
|
||||
it("falls back to updatedAt for staleness when columnMovedAt is null (legacy tasks)", async () => {
|
||||
const managerWithRecovery = new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
});
|
||||
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({ taskStuckTimeoutMs: 60_000 });
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([
|
||||
{
|
||||
id: "FN-407-test-2",
|
||||
column: "in-review",
|
||||
paused: false,
|
||||
status: null,
|
||||
columnMovedAt: null,
|
||||
updatedAt: new Date(Date.now() - 120_000).toISOString(),
|
||||
steps: [{ name: "Step 0", status: "in-progress" }],
|
||||
workflowStepResults: [],
|
||||
mergeDetails: undefined,
|
||||
log: [],
|
||||
},
|
||||
]);
|
||||
|
||||
const result = await managerWithRecovery.recoverStaleIncompleteReviewTasks();
|
||||
|
||||
expect(result).toBe(1);
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-407-test-2", "todo", { preserveProgress: true });
|
||||
|
||||
managerWithRecovery.stop();
|
||||
});
|
||||
|
||||
it("moves merged in-review tasks to done and clears transient merge state", async () => {
|
||||
const managerWithRecovery = new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
|
||||
@@ -224,8 +224,6 @@ export class MissionExecutionLoop extends EventEmitter {
|
||||
|
||||
loopLog.log(`Processing task outcome for ${taskId}`);
|
||||
|
||||
// Track the validation board task ID (created later if there are assertions)
|
||||
let validationTaskId: string | undefined;
|
||||
|
||||
try {
|
||||
// Find the feature linked to this task
|
||||
@@ -254,35 +252,10 @@ export class MissionExecutionLoop extends EventEmitter {
|
||||
this.activeValidations.add(feature.id);
|
||||
|
||||
try {
|
||||
// Resolve mission context for creating the validation board task
|
||||
const featureSlice = this.missionStore.getSlice(feature.sliceId);
|
||||
const featureMilestone = featureSlice ? this.missionStore.getMilestone(featureSlice.milestoneId) : undefined;
|
||||
const missionId = featureMilestone?.missionId;
|
||||
loopLog.log(`Running internal validation for feature ${feature.id} — no board task created (policy: docs/task-authoring-standards.md §5)`);
|
||||
|
||||
// Create a visible board task for this validation run
|
||||
const validationTask = await this.taskStore.createTask({
|
||||
title: `🔍 Validate: ${feature.title}`,
|
||||
description: `Validating implementation for feature "${feature.title}" against ${assertions.length} contract assertion(s).\n\nFeature: ${feature.id}\nSlice: ${feature.sliceId}\nAssertions: ${assertions.map(a => a.title).join(", ")}`,
|
||||
column: "in-progress",
|
||||
missionId,
|
||||
sliceId: feature.sliceId,
|
||||
source: {
|
||||
sourceType: "automation",
|
||||
sourceMetadata: {
|
||||
missionId,
|
||||
featureId: feature.id,
|
||||
sliceId: feature.sliceId,
|
||||
},
|
||||
},
|
||||
});
|
||||
validationTaskId = validationTask.id;
|
||||
|
||||
// Mark as validation task so scheduler/stuck-detector skip it
|
||||
await this.taskStore.updateTask(validationTaskId, { status: "mission-validation" });
|
||||
loopLog.log(`Created validation board task ${validationTaskId} for feature ${feature.id}`);
|
||||
|
||||
// Start the validator run, linked to the board task
|
||||
const run = this.missionStore.startValidatorRun(feature.id, "task_completion", validationTaskId);
|
||||
// Start the validator run (no board task per docs/task-authoring-standards.md §5)
|
||||
const run = this.missionStore.startValidatorRun(feature.id, "task_completion");
|
||||
loopLog.log(`Started validator run ${run.id} for feature ${feature.id}`);
|
||||
|
||||
// Run the validation
|
||||
@@ -290,31 +263,19 @@ export class MissionExecutionLoop extends EventEmitter {
|
||||
|
||||
// Handle the result
|
||||
if (result.status === "pass") {
|
||||
await this.handleValidationPass(feature.id, run.id, result.summary, validationTaskId);
|
||||
await this.handleValidationPass(feature.id, run.id, result.summary, undefined);
|
||||
} else if (result.status === "fail") {
|
||||
await this.handleValidationFail(feature.id, run.id, result, validationTaskId);
|
||||
await this.handleValidationFail(feature.id, run.id, result, undefined);
|
||||
} else if (result.status === "blocked") {
|
||||
await this.handleValidationBlocked(feature.id, run.id, result.blockedReason, validationTaskId);
|
||||
await this.handleValidationBlocked(feature.id, run.id, result.blockedReason, undefined);
|
||||
} else if (result.status === "error") {
|
||||
await this.handleValidationError(feature.id, run.id, result.summary, validationTaskId);
|
||||
await this.handleValidationError(feature.id, run.id, result.summary, undefined);
|
||||
}
|
||||
} finally {
|
||||
this.activeValidations.delete(feature.id);
|
||||
}
|
||||
} catch (err) {
|
||||
loopLog.error(`Error processing task outcome for ${taskId}:`, err);
|
||||
// Move the validation task to in-review if it exists
|
||||
if (validationTaskId) {
|
||||
try {
|
||||
await this.taskStore.updateTask(validationTaskId, {
|
||||
error: err instanceof Error ? err.message : String(err),
|
||||
summary: "Validation failed unexpectedly",
|
||||
});
|
||||
await this.taskStore.moveTask(validationTaskId, "in-review");
|
||||
} catch (moveErr) {
|
||||
loopLog.error(`Failed to move validation task ${validationTaskId} on error:`, moveErr);
|
||||
}
|
||||
}
|
||||
// Don't crash the loop - log and continue
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1351,7 +1351,7 @@ export class SelfHealingManager {
|
||||
!task.status &&
|
||||
task.steps.length > 0 &&
|
||||
task.steps.some((step) => NON_TERMINAL_STEP_STATUSES.has(step.status)) &&
|
||||
now - new Date(task.updatedAt).getTime() >= timeoutMs
|
||||
now - new Date(task.columnMovedAt ?? task.updatedAt).getTime() >= timeoutMs
|
||||
);
|
||||
|
||||
if (staleIncomplete.length === 0) return 0;
|
||||
@@ -1422,7 +1422,7 @@ export class SelfHealingManager {
|
||||
!(task.status && GHOST_REVIEW_PRESERVED_STATUSES.has(task.status)) &&
|
||||
// Confirmed merges belong in `done` (handled by `recoverMergedReviewTasks`).
|
||||
task.mergeDetails?.mergeConfirmed !== true &&
|
||||
now - new Date(task.updatedAt).getTime() >= timeoutMs
|
||||
now - new Date(task.columnMovedAt ?? task.updatedAt).getTime() >= timeoutMs
|
||||
);
|
||||
|
||||
if (ghosts.length === 0) return 0;
|
||||
|
||||
Reference in New Issue
Block a user