fix(FN-6035): address workflow projection review feedback

.changeset/workflow-projection-capacity.md

@@ -1,3 +1,5 @@
+---
 "@runfusion/fusion": patch
+---
 
 Fix workflow-native dispatch capacity accounting and publish workflow node task metadata to the existing task fields used by scheduler and dashboard surfaces.

packages/core/src/__tests__/store-update.test.ts

@@ -1198,5 +1198,43 @@ Task with acceptance criteria
     });
   });
 
+  describe("updateTaskAtomic", () => {
+    it("serializes read-merge-write patches against the freshest task snapshot", async () => {
+      const task = await store.createTask({ description: "atomic task projection" });
+      let releaseFirst: () => void = () => {};
+      const firstCanFinish = new Promise<void>((resolve) => {
+        releaseFirst = resolve;
+      });
+      let markFirstRead: () => void = () => {};
+      const firstRead = new Promise<void>((resolve) => {
+        markFirstRead = resolve;
+      });
+
+      const first = store.updateTaskAtomic(task.id, async (current) => {
+        markFirstRead();
+        expect(current.modifiedFiles).toBeUndefined();
+        await firstCanFinish;
+        return {
+          modifiedFiles: [...new Set([...(current.modifiedFiles ?? []), "src/a.ts"])].sort(),
+          mergeDetails: { ...(current.mergeDetails ?? {}), filesChanged: 1 },
+        };
+      });
+
+      await firstRead;
+
+      const second = store.updateTaskAtomic(task.id, (current) => ({
+        modifiedFiles: [...new Set([...(current.modifiedFiles ?? []), "src/b.ts"])].sort(),
+        mergeDetails: { ...(current.mergeDetails ?? {}), insertions: 2 },
+      }));
+
+      releaseFirst();
+      await Promise.all([first, second]);
+
+      const updated = await store.getTask(task.id);
+      expect(updated.modifiedFiles).toEqual(["src/a.ts", "src/b.ts"]);
+      expect(updated.mergeDetails).toEqual({ filesChanged: 1, insertions: 2 });
+    });
+  });
+
 
 });

packages/core/src/store.ts

@@ -7445,6 +7445,23 @@ ${TASK_UPSERT_SQL_ASSIGNMENTS}
     return this.withTaskLock(id, () => this.updateTaskUnlocked(id, updates, runContext));
   }
 
+  async updateTaskAtomic(
+    id: string,
+    updater: (
+      current: Task,
+    ) => Parameters<TaskStore["updateTask"]>[1] | null | undefined | Promise<Parameters<TaskStore["updateTask"]>[1] | null | undefined>,
+    runContext?: RunMutationContext,
+  ): Promise<Task> {
+    return this.withTaskLock(id, async () => {
+      const current = await this.readTaskJson(this.taskDir(id));
+      const updates = await updater(current);
+      if (!updates || Object.values(updates).every((value) => value === undefined)) {
+        return current;
+      }
+      return this.updateTaskUnlocked(id, updates, runContext);
+    });
+  }
+
   /**
    * Merge a validated/normalized custom-field patch into the existing values.
    * `null` in the patch deletes that field's value (the delete sentinel from

packages/engine/src/__tests__/workflow-graph-executor-handlers.test.ts

@@ -297,6 +297,43 @@ describe("WorkflowGraphExecutor traversal", () => {
     expect(result.context["node:a:projectionError"]).toBe("store unavailable");
   });
 
+  it("does not fail the node when the deprecated touched-files hook fails", async () => {
+    const ir: WorkflowIr = {
+      version: "v1",
+      name: "legacy-touched-files-failure",
+      nodes: [
+        { id: "start", kind: "start" },
+        { id: "a", kind: "prompt" },
+        { id: "end", kind: "end" },
+      ],
+      edges: [
+        { from: "start", to: "a" },
+        { from: "a", to: "end", condition: "success" },
+      ],
+    };
+    const publishTaskProjection = vi.fn();
+    const publishTouchedFiles = vi.fn(async () => {
+      throw new Error("legacy sink unavailable");
+    });
+    const executor = new WorkflowGraphExecutor({
+      handlers: {
+        prompt: async () => ({
+          outcome: "success",
+          contextPatch: { modifiedFiles: ["src/projected.ts"] },
+        }),
+      },
+      publishTaskProjection,
+      publishTouchedFiles,
+    });
+
+    const result = await executor.run(task, settingsOn(), ir);
+
+    expect(publishTaskProjection).toHaveBeenCalledTimes(1);
+    expect(publishTouchedFiles).toHaveBeenCalledTimes(1);
+    expect(result.outcome).toBe("success");
+    expect(result.context["node:a:projectionError"]).toBeUndefined();
+  });
+
   it("caps retries and converts exceptions to failure", async () => {
     const ir: WorkflowIr = {
       version: "v1",

packages/engine/src/executor.ts

@@ -3752,19 +3752,18 @@ export class TaskExecutor {
         runCustomNode: (node, nodeTask) =>
           this.runGraphCustomNode(node, nodeTask, settings, resolveBindingForNode(node.id)),
         publishTaskProjection: async (taskId, patch) => {
-          const liveTask = await this.store.getTask(taskId);
-          const update: Parameters<TaskStore["updateTask"]>[1] = {};
-          if (patch.modifiedFiles) {
-            const merged = [...new Set([...(liveTask?.modifiedFiles ?? []), ...patch.modifiedFiles])].sort();
-            if (merged.length > 0) update.modifiedFiles = merged;
-          }
-          if (patch.mergeDetails) {
-            update.mergeDetails = { ...(liveTask?.mergeDetails ?? {}), ...patch.mergeDetails };
-          }
-          if (patch.summary !== undefined) update.summary = patch.summary;
-          if (Object.keys(update).length > 0) {
-            await this.store.updateTask(taskId, update);
-          }
+          await this.store.updateTaskAtomic(taskId, (liveTask) => {
+            const update: Parameters<TaskStore["updateTask"]>[1] = {};
+            if (patch.modifiedFiles) {
+              const merged = [...new Set([...(liveTask.modifiedFiles ?? []), ...patch.modifiedFiles])].sort();
+              if (merged.length > 0) update.modifiedFiles = merged;
+            }
+            if (patch.mergeDetails) {
+              update.mergeDetails = { ...(liveTask.mergeDetails ?? {}), ...patch.mergeDetails };
+            }
+            if (patch.summary !== undefined) update.summary = patch.summary;
+            return update;
+          });
         },
         onEvent: (event) => executorLog.log(`[workflow-graph] ${event.type} ${event.taskId}: ${event.detail}`),
         // Wire SQLite-backed per-branch persistence in production (#1407): the

packages/engine/src/workflow-graph-executor.ts

@@ -717,9 +717,6 @@ export class WorkflowGraphExecutor {
     const source = { nodeId: node.id, nodeKind: node.kind };
     try {
       await this.deps.publishTaskProjection?.(taskId, patch, source);
-      if (patch.modifiedFiles && patch.modifiedFiles.length > 0) {
-        await this.deps.publishTouchedFiles?.(taskId, patch.modifiedFiles, source);
-      }
     } catch (error) {
       return {
         outcome: "failure",
@@ -730,6 +727,13 @@ export class WorkflowGraphExecutor {
         },
       };
     }
+    if (patch.modifiedFiles && patch.modifiedFiles.length > 0) {
+      try {
+        await this.deps.publishTouchedFiles?.(taskId, patch.modifiedFiles, source);
+      } catch {
+        // Deprecated compatibility hook; primary projection persistence owns node outcome.
+      }
+    }
     return result;
   }
 }