Add circular reference detection to smartClone method

packages/task-graph/src/task/Task.ts

@@ -386,6 +386,7 @@ export class Task<
   /**
    * Smart clone that deep-clones plain objects and arrays while preserving
    * class instances (objects with non-Object prototype) by reference.
+   * Detects and throws an error on circular references.
    *
    * This is necessary because:
    * - structuredClone cannot clone class instances (methods are lost)
@@ -396,13 +397,27 @@ export class Task<
    * more efficient use cases. Do be careful with this though! Use sparingly.
    *
    * @param obj The object to clone
+   * @param visited Set of objects in the current cloning path (for circular reference detection)
    * @returns A cloned object with class instances preserved by reference
    */
-  private smartClone(obj: any): any {
+  private smartClone(obj: any, visited: WeakSet<object> = new WeakSet()): any {
     if (obj === null || obj === undefined) {
       return obj;
     }
 
+    // Primitives (string, number, boolean, symbol, bigint) are returned as-is
+    if (typeof obj !== "object") {
+      return obj;
+    }
+
+    // Check for circular references
+    if (visited.has(obj)) {
+      throw new Error(
+        "Circular reference detected in input data. " +
+          "Cannot clone objects with circular references."
+      );
+    }
+
     // Clone TypedArrays (Float32Array, Int8Array, etc.) to avoid shared-mutation
     // between defaults and runInputData, while preserving DataView by reference.
     if (ArrayBuffer.isView(obj)) {
@@ -417,31 +432,35 @@ export class Task<
 
     // Preserve class instances (objects with non-Object/non-Array prototype)
     // This includes repository instances, custom classes, etc.
-    if (typeof obj === "object" && !Array.isArray(obj)) {
+    if (!Array.isArray(obj)) {
       const proto = Object.getPrototypeOf(obj);
       if (proto !== Object.prototype && proto !== null) {
         return obj; // Pass by reference
       }
     }
 
-    // Deep clone arrays, preserving class instances within
-    if (Array.isArray(obj)) {
-      return obj.map((item) => this.smartClone(item));
-    }
+    // Add object to visited set before recursing
+    visited.add(obj);
 
-    // Deep clone plain objects
-    if (typeof obj === "object") {
+    try {
+      // Deep clone arrays, preserving class instances within
+      if (Array.isArray(obj)) {
+        return obj.map((item) => this.smartClone(item, visited));
+      }
+
+      // Deep clone plain objects
       const result: Record<string, any> = {};
       for (const key in obj) {
         if (Object.prototype.hasOwnProperty.call(obj, key)) {
-          result[key] = this.smartClone(obj[key]);
+          result[key] = this.smartClone(obj[key], visited);
         }
       }
       return result;
+    } finally {
+      // Remove from visited set after processing to allow the same object
+      // in different branches (non-circular references)
+      visited.delete(obj);
     }
-
-    // Primitives (string, number, boolean, symbol, bigint) are returned as-is
-    return obj;
   }
 
   /**

packages/task-graph/src/task/TaskEvents.ts

@@ -5,8 +5,8 @@
  */
 
 import { EventParameters, type DataPortSchema } from "@workglow/util";
-import { TaskStatus } from "../common";
 import { TaskAbortedError, TaskError } from "./TaskError";
+import { TaskStatus } from "./TaskTypes";
 
 // ========================================================================
 // Event Handling Types

packages/test/src/test/task/Task.smartClone.test.ts

@@ -0,0 +1,203 @@
+/**
+ * @license
+ * Copyright 2025 Steven Roussey <sroussey@gmail.com>
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import type { IExecuteContext } from "@workglow/task-graph";
+import { Task } from "@workglow/task-graph";
+import type { DataPortSchema } from "@workglow/util";
+import { beforeEach, describe, expect, test } from "vitest";
+
+// Test task class to access private smartClone method
+class TestSmartCloneTask extends Task<{ data: any }, { result: any }> {
+  static readonly type = "TestSmartCloneTask";
+  static readonly category = "Test";
+  static readonly title = "Test Smart Clone Task";
+  static readonly description = "A task for testing smartClone";
+  declare runInputData: { data: any };
+  declare runOutputData: { result: any };
+
+  static inputSchema(): DataPortSchema {
+    return {
+      type: "object",
+      properties: {
+        data: {},
+      },
+      additionalProperties: false,
+    } as const satisfies DataPortSchema;
+  }
+
+  static outputSchema(): DataPortSchema {
+    return {
+      type: "object",
+      properties: {
+        result: {},
+      },
+      additionalProperties: false,
+    } as const satisfies DataPortSchema;
+  }
+
+  async execute(input: { data: any }, context: IExecuteContext): Promise<{ result: any }> {
+    return { result: input.data };
+  }
+
+  // Expose smartClone for testing
+  public testSmartClone(obj: any): any {
+    return (this as any).smartClone(obj);
+  }
+}
+
+describe("Task.smartClone circular reference detection", () => {
+  let task: TestSmartCloneTask;
+
+  beforeEach(() => {
+    task = new TestSmartCloneTask({ data: {} }, { id: "test-task" });
+  });
+
+  test("should handle simple objects without circular references", () => {
+    const obj = { a: 1, b: { c: 2 } };
+    const cloned = task.testSmartClone(obj);
+
+    expect(cloned).toEqual(obj);
+    expect(cloned).not.toBe(obj);
+    expect(cloned.b).not.toBe(obj.b);
+  });
+
+  test("should handle arrays without circular references", () => {
+    const arr = [1, 2, [3, 4]];
+    const cloned = task.testSmartClone(arr);
+
+    expect(cloned).toEqual(arr);
+    expect(cloned).not.toBe(arr);
+    expect(cloned[2]).not.toBe(arr[2]);
+  });
+
+  test("should throw error on object with circular self-reference", () => {
+    const obj: any = { a: 1 };
+    obj.self = obj;
+
+    expect(() => task.testSmartClone(obj)).toThrow("Circular reference detected in input data");
+  });
+
+  test("should throw error on nested circular reference", () => {
+    const obj: any = { a: 1, b: { c: 2 } };
+    obj.b.parent = obj;
+
+    expect(() => task.testSmartClone(obj)).toThrow("Circular reference detected in input data");
+  });
+
+  test("should throw error on array with circular reference", () => {
+    const arr: any = [1, 2, 3];
+    arr.push(arr);
+
+    expect(() => task.testSmartClone(arr)).toThrow("Circular reference detected in input data");
+  });
+
+  test("should throw error on complex circular reference chain", () => {
+    const obj1: any = { name: "obj1" };
+    const obj2: any = { name: "obj2", ref: obj1 };
+    const obj3: any = { name: "obj3", ref: obj2 };
+    obj1.ref = obj3; // Create circular chain
+
+    expect(() => task.testSmartClone(obj1)).toThrow("Circular reference detected in input data");
+  });
+
+  test("should handle same object referenced multiple times (not circular)", () => {
+    const shared = { value: 42 };
+    const obj = { a: shared, b: shared };
+
+    // This should work - same object referenced multiple times is not circular
+    // Each reference gets cloned independently
+    const cloned = task.testSmartClone(obj);
+
+    expect(cloned).toEqual(obj);
+    expect(cloned.a).toEqual(shared);
+    expect(cloned.b).toEqual(shared);
+    // The cloned references should be different objects (deep clone)
+    expect(cloned.a).not.toBe(shared);
+    expect(cloned.b).not.toBe(shared);
+    expect(cloned.a).not.toBe(cloned.b);
+  });
+
+  test("should preserve class instances by reference (no circular check needed)", () => {
+    class CustomClass {
+      constructor(public value: number) {}
+    }
+
+    const instance = new CustomClass(42);
+    const obj = { data: instance };
+
+    const cloned = task.testSmartClone(obj);
+
+    expect(cloned.data).toBe(instance); // Should be same reference
+    expect(cloned.data.value).toBe(42);
+  });
+
+  test("should clone TypedArrays to avoid shared mutation", () => {
+    const typedArray = new Float32Array([1.0, 2.0, 3.0]);
+    const obj = { data: typedArray };
+
+    const cloned = task.testSmartClone(obj);
+
+    expect(cloned.data).not.toBe(typedArray); // Should be a new instance
+    expect(cloned.data).toEqual(typedArray); // But with the same values
+    expect(cloned.data).toBeInstanceOf(Float32Array);
+  });
+
+  test("should handle null and undefined", () => {
+    expect(task.testSmartClone(null)).toBe(null);
+    expect(task.testSmartClone(undefined)).toBe(undefined);
+    expect(task.testSmartClone({ a: null, b: undefined })).toEqual({ a: null, b: undefined });
+  });
+
+  test("should handle primitives", () => {
+    expect(task.testSmartClone(42)).toBe(42);
+    expect(task.testSmartClone("hello")).toBe("hello");
+    expect(task.testSmartClone(true)).toBe(true);
+    expect(task.testSmartClone(false)).toBe(false);
+  });
+
+  test("should clone nested structures without circular references", () => {
+    const obj = {
+      level1: {
+        level2: {
+          level3: {
+            value: "deep",
+          },
+        },
+        array: [1, 2, { nested: true }],
+      },
+    };
+
+    const cloned = task.testSmartClone(obj);
+
+    expect(cloned).toEqual(obj);
+    expect(cloned).not.toBe(obj);
+    expect(cloned.level1).not.toBe(obj.level1);
+    expect(cloned.level1.level2).not.toBe(obj.level1.level2);
+    expect(cloned.level1.array).not.toBe(obj.level1.array);
+    expect(cloned.level1.array[2]).not.toBe(obj.level1.array[2]);
+  });
+
+  test("should handle mixed object and array structures", () => {
+    const obj = {
+      users: [
+        { id: 1, name: "Alice" },
+        { id: 2, name: "Bob" },
+      ],
+      settings: {
+        theme: "dark",
+        features: ["feature1", "feature2"],
+      },
+    };
+
+    const cloned = task.testSmartClone(obj);
+
+    expect(cloned).toEqual(obj);
+    expect(cloned.users).not.toBe(obj.users);
+    expect(cloned.users[0]).not.toBe(obj.users[0]);
+    expect(cloned.settings).not.toBe(obj.settings);
+    expect(cloned.settings.features).not.toBe(obj.settings.features);
+  });
+});