diff --git a/packages/task-graph/src/task/Task.ts b/packages/task-graph/src/task/Task.ts index 1854bf912..90d3f7b08 100644 --- a/packages/task-graph/src/task/Task.ts +++ b/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 = 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 = {}; 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; } /** diff --git a/packages/task-graph/src/task/TaskEvents.ts b/packages/task-graph/src/task/TaskEvents.ts index 6eee2280c..383389ec5 100644 --- a/packages/task-graph/src/task/TaskEvents.ts +++ b/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 diff --git a/packages/test/src/test/task/Task.smartClone.test.ts b/packages/test/src/test/task/Task.smartClone.test.ts new file mode 100644 index 000000000..ac926ce7b --- /dev/null +++ b/packages/test/src/test/task/Task.smartClone.test.ts @@ -0,0 +1,203 @@ +/** + * @license + * Copyright 2025 Steven Roussey + * 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); + }); +});