feat(FN-4131): refactor mission execution loop: remove dead validationTaskI

Removed dead `validationTaskId` plumbing from the mission execution loop and added tests to pin the updated behavior, with documentation for the final delivery step. Fusion-Task-Id: FN-4131 Fusion-Task-Lineage: 1b1f010a-f520-43a0-9c10-40618b5fbc13
2026-05-12 16:45:43 -07:00
parent 7c91c7961d
commit fdd0af3b49
3 changed files with 68 additions and 52 deletions
--- a/packages/engine/src/tests/merger-autostash-cleanup.test.ts
+++ b/packages/engine/src/tests/merger-autostash-cleanup.test.ts
@@ -44,6 +44,10 @@ function stashList(dir: string): string {
  return git(dir, 'git stash list --format="%H %gd %s"');
 }

+function testTempParent(): string {
+  return process.env.FUSION_TEST_WORKER_ROOT ?? tmpdir();
+}
+
 function assertIsolatedWorkspace(dir: string): void {
  const repoRoot = process.env.FUSION_TEST_REAL_ROOT;
  if (!repoRoot) return;
@@ -75,7 +79,7 @@ describe("sweepStaleAutostashes", () => {
  let dir: string;

  beforeEach(() => {
-    dir = mkdtempSync(join(tmpdir(), "fusion-test-merger-autostash-stale-"));
+    dir = mkdtempSync(join(testTempParent(), "fusion-test-merger-autostash-stale-"));
    assertIsolatedWorkspace(dir);
    initRepo(dir);
  });
@@ -152,7 +156,7 @@ describe("sweepAutostashOrphans", () => {
  let dir: string;

  beforeEach(() => {
-    dir = mkdtempSync(join(tmpdir(), "fusion-test-merger-autostash-"));
+    dir = mkdtempSync(join(testTempParent(), "fusion-test-merger-autostash-"));
    assertIsolatedWorkspace(dir);
    initRepo(dir);
  });
--- a/packages/engine/src/tests/mission-execution-loop.test.ts
+++ b/packages/engine/src/tests/mission-execution-loop.test.ts
@@ -318,6 +318,11 @@ function makeAssertions(count: number): MissionContractAssertion[] {
  }));
 }

+function expectNoValidationBoardTaskMutation(taskStore: ReturnType<typeof createMockTaskStore>) {
+  expect(taskStore.updateTask).not.toHaveBeenCalled();
+  expect(taskStore.moveTask).not.toHaveBeenCalled();
+}
+
 // ── Tests ───────────────────────────────────────────────────────────────────

 describe("MissionExecutionLoop", () => {
@@ -470,6 +475,7 @@ describe("MissionExecutionLoop", () => {
        "validation:passed",
        expect.objectContaining({ featureId: "F-001" }),
      );
+      expectNoValidationBoardTaskMutation(taskStore);
    });

    it("does NOT create a board task for single-feature validation", async () => {
@@ -724,6 +730,7 @@ describe("MissionExecutionLoop", () => {
        "passed",
        expect.any(String),
      );
+      expectNoValidationBoardTaskMutation(taskStore);
    });

    it("should parse fail result from JSON in markdown code block", async () => {
@@ -777,6 +784,7 @@ describe("MissionExecutionLoop", () => {

      // createGeneratedFixFeature should be called
      expect(missionStore.createGeneratedFixFeature).toHaveBeenCalled();
+      expectNoValidationBoardTaskMutation(taskStore);
    });

    it("should handle malformed JSON gracefully", async () => {
@@ -1094,6 +1102,53 @@ describe("MissionExecutionLoop", () => {
          reason: expect.stringContaining("External API not available"),
        }),
      );
+      expectNoValidationBoardTaskMutation(taskStore);
+    });
+  });
+
+  // ── handleValidationError ───────────────────────────────────────────────
+
+  describe("handleValidationError", () => {
+    it("emits validation:error without mutating any board task", async () => {
+      const assertions = makeAssertions(1);
+      const feature = createMockFeature({
+        loopState: "implementing",
+        taskId: "FN-001",
+        id: "F-001",
+      });
+      missionStore._setFeature(feature);
+      missionStore.getFeatureByTaskId = vi.fn().mockReturnValue(feature);
+      missionStore.listAssertionsForFeature = vi.fn().mockReturnValue(assertions);
+      taskStore._setTask({ id: "FN-001", title: "Test", description: "Implementation", log: [] });
+
+      mockSessionHolder.session.state.messages = [
+        { role: "user", content: "Validate this" },
+        { role: "assistant", content: JSON.stringify({ status: "unknown", summary: "validator crashed" }) },
+      ];
+
+      loop = new MissionExecutionLoop({
+        taskStore: taskStore as any,
+        missionStore: missionStore as any,
+        rootDir: "/tmp",
+      });
+      const emitSpy = vi.spyOn(loop, "emit");
+      loop.start();
+
+      await loop.processTaskOutcome("FN-001");
+
+      expect(missionStore.completeValidatorRun).toHaveBeenCalledWith(
+        expect.any(String),
+        "error",
+        "Invalid status in validation response",
+      );
+      expect(emitSpy).toHaveBeenCalledWith(
+        "validation:error",
+        expect.objectContaining({
+          featureId: "F-001",
+          error: "Invalid status in validation response",
+        }),
+      );
+      expectNoValidationBoardTaskMutation(taskStore);
    });
  });

--- a/packages/engine/src/mission-execution-loop.ts
+++ b/packages/engine/src/mission-execution-loop.ts
@@ -244,7 +244,7 @@ export class MissionExecutionLoop extends EventEmitter {
      if (assertions.length === 0) {
        loopLog.log(`Feature ${feature.id} has no linked assertions; marking as passed`);
        // No assertions = automatically pass
-        await this.handleValidationPass(feature.id, undefined, "No assertions linked", undefined);
+        await this.handleValidationPass(feature.id, undefined, "No assertions linked");
        return;
      }

@@ -252,9 +252,9 @@ export class MissionExecutionLoop extends EventEmitter {
      this.activeValidations.add(feature.id);

      try {
-        loopLog.log(`Running internal validation for feature ${feature.id} — no board task created (policy: docs/task-authoring-standards.md §5)`);
+        loopLog.log(`Running internal validation for feature ${feature.id} — no board task created (policy: docs/missions.md)`);

-        // Start the validator run (no board task per docs/task-authoring-standards.md §5)
+        // Start the validator run (no board task per docs/missions.md)
        const run = this.missionStore.startValidatorRun(feature.id, "task_completion");
        loopLog.log(`Started validator run ${run.id} for feature ${feature.id}`);

@@ -263,13 +263,13 @@ export class MissionExecutionLoop extends EventEmitter {

        // Handle the result
        if (result.status === "pass") {
-          await this.handleValidationPass(feature.id, run.id, result.summary, undefined);
+          await this.handleValidationPass(feature.id, run.id, result.summary);
        } else if (result.status === "fail") {
-          await this.handleValidationFail(feature.id, run.id, result, undefined);
+          await this.handleValidationFail(feature.id, run.id, result);
        } else if (result.status === "blocked") {
-          await this.handleValidationBlocked(feature.id, run.id, result.blockedReason, undefined);
+          await this.handleValidationBlocked(feature.id, run.id, result.blockedReason);
        } else if (result.status === "error") {
-          await this.handleValidationError(feature.id, run.id, result.summary, undefined);
+          await this.handleValidationError(feature.id, run.id, result.summary);
        }
      } finally {
        this.activeValidations.delete(feature.id);
@@ -740,7 +740,6 @@ ${taskContext ? `\n\nImplementation context:\n${taskContext}` : ""}`;
    featureId: string,
    runId: string | undefined,
    summary: string,
-    validationTaskId: string | undefined,
  ): Promise<void> {
    try {
      if (runId) {
@@ -748,15 +747,6 @@ ${taskContext ? `\n\nImplementation context:\n${taskContext}` : ""}`;
      }
      loopLog.log(`Feature ${featureId} passed validation`);

-      // Move the validation board task to done if it exists
-      if (validationTaskId) {
-        await this.taskStore.updateTask(validationTaskId, {
-          summary: summary || "Validation passed",
-        });
-        await this.taskStore.moveTask(validationTaskId, "done");
-        loopLog.log(`Moved validation task ${validationTaskId} to done`);
-      }
-
      // Notify autopilot if configured
      if (this.missionAutopilot?.notifyValidationComplete) {
        await this.missionAutopilot.notifyValidationComplete(featureId, "passed");
@@ -775,7 +765,6 @@ ${taskContext ? `\n\nImplementation context:\n${taskContext}` : ""}`;
    featureId: string,
    runId: string | undefined,
    result: ValidationResult,
-    validationTaskId: string | undefined,
  ): Promise<void> {
    try {
      // Record the failures
@@ -799,16 +788,6 @@ ${taskContext ? `\n\nImplementation context:\n${taskContext}` : ""}`;

      loopLog.log(`Feature ${featureId} failed validation with ${failures.length} failures`);

-      // Move the validation board task to in-review if it exists
-      if (validationTaskId) {
-        await this.taskStore.updateTask(validationTaskId, {
-          error: result.summary,
-          summary: `Failed: ${failures.length} assertion(s) failed`,
-        });
-        await this.taskStore.moveTask(validationTaskId, "in-review");
-        loopLog.log(`Moved validation task ${validationTaskId} to in-review`);
-      }
-
      // Create fix feature
      try {
        const fixFeature = this.missionStore.createGeneratedFixFeature(
@@ -861,7 +840,6 @@ ${taskContext ? `\n\nImplementation context:\n${taskContext}` : ""}`;
    featureId: string,
    runId: string | undefined,
    blockedReason: string | undefined,
-    validationTaskId: string | undefined,
  ): Promise<void> {
    try {
      if (runId) {
@@ -869,16 +847,6 @@ ${taskContext ? `\n\nImplementation context:\n${taskContext}` : ""}`;
      }
      loopLog.log(`Feature ${featureId} blocked: ${blockedReason}`);

-      // Move the validation board task to in-review if it exists
-      if (validationTaskId) {
-        await this.taskStore.updateTask(validationTaskId, {
-          error: blockedReason,
-          summary: `Blocked: ${blockedReason}`,
-        });
-        await this.taskStore.moveTask(validationTaskId, "in-review");
-        loopLog.log(`Moved validation task ${validationTaskId} to in-review`);
-      }
-
      // Notify autopilot if configured
      if (this.missionAutopilot?.notifyValidationComplete) {
        await this.missionAutopilot.notifyValidationComplete(featureId, "blocked");
@@ -897,7 +865,6 @@ ${taskContext ? `\n\nImplementation context:\n${taskContext}` : ""}`;
    featureId: string,
    runId: string | undefined,
    error: string,
-    validationTaskId: string | undefined,
  ): Promise<void> {
    try {
      if (runId) {
@@ -905,16 +872,6 @@ ${taskContext ? `\n\nImplementation context:\n${taskContext}` : ""}`;
      }
      loopLog.error(`Feature ${featureId} validation error: ${error}`);

-      // Move the validation board task to in-review if it exists
-      if (validationTaskId) {
-        await this.taskStore.updateTask(validationTaskId, {
-          error,
-          summary: "Validation error",
-        });
-        await this.taskStore.moveTask(validationTaskId, "in-review");
-        loopLog.log(`Moved validation task ${validationTaskId} to in-review`);
-      }
-
      // Notify autopilot if configured
      if (this.missionAutopilot?.notifyValidationComplete) {
        await this.missionAutopilot.notifyValidationComplete(featureId, "error");