From 65ffbb8a239d3f7005f1f462215377fb51c45266 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 4 Jan 2026 00:51:57 +0000 Subject: [PATCH 1/4] Initial plan From c5d2529cde61cc5397a3ddd65ce81560e9e1be03 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 4 Jan 2026 00:56:43 +0000 Subject: [PATCH 2/4] Add circular reference detection to smartClone method Co-authored-by: sroussey <127349+sroussey@users.noreply.github.com> --- .../src/task/Task.smartClone.test.ts | 209 ++++++++++++++++++ packages/task-graph/src/task/Task.ts | 40 ++-- 2 files changed, 236 insertions(+), 13 deletions(-) create mode 100644 packages/task-graph/src/task/Task.smartClone.test.ts diff --git a/packages/task-graph/src/task/Task.smartClone.test.ts b/packages/task-graph/src/task/Task.smartClone.test.ts new file mode 100644 index 000000000..7e723b287 --- /dev/null +++ b/packages/task-graph/src/task/Task.smartClone.test.ts @@ -0,0 +1,209 @@ +/** + * @license + * Copyright 2025 Steven Roussey + * SPDX-License-Identifier: Apache-2.0 + */ + +import type { DataPortSchema } from "@workglow/util"; +import { beforeEach, describe, expect, test } from "vitest"; +import type { IExecuteContext } from "./ITask"; +import { Task } from "./Task"; + +// 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 preserve TypedArrays by reference", () => { + const typedArray = new Float32Array([1.0, 2.0, 3.0]); + const obj = { data: typedArray }; + + const cloned = task.testSmartClone(obj); + + expect(cloned.data).toBe(typedArray); // Should be same reference + }); + + 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); + }); +}); diff --git a/packages/task-graph/src/task/Task.ts b/packages/task-graph/src/task/Task.ts index 271373f72..7caff083b 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,28 @@ 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 already visited (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. " + + "Consider using class instances which are preserved by reference." + ); + } + // Preserve TypedArrays (Float32Array, Int8Array, etc.) by reference // These are often large and cloning them is expensive if (ArrayBuffer.isView(obj)) { @@ -411,31 +427,29 @@ 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 } } + // Add object to visited set before recursing + visited.add(obj); + // Deep clone arrays, preserving class instances within if (Array.isArray(obj)) { - return obj.map((item) => this.smartClone(item)); + return obj.map((item) => this.smartClone(item, visited)); } // Deep clone plain objects - if (typeof obj === "object") { - const result: Record = {}; - for (const key in obj) { - if (Object.prototype.hasOwnProperty.call(obj, key)) { - result[key] = this.smartClone(obj[key]); - } + const result: Record = {}; + for (const key in obj) { + if (Object.prototype.hasOwnProperty.call(obj, key)) { + result[key] = this.smartClone(obj[key], visited); } - return result; } - - // Primitives (string, number, boolean, symbol, bigint) are returned as-is - return obj; + return result; } /** From 2df995e94a0ff9616bae960328c6b131f199a1fc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 4 Jan 2026 00:58:14 +0000 Subject: [PATCH 3/4] Fix circular reference detection to handle shared references correctly Co-authored-by: sroussey <127349+sroussey@users.noreply.github.com> --- packages/task-graph/src/task/Task.ts | 31 ++++++++++++++++------------ 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/packages/task-graph/src/task/Task.ts b/packages/task-graph/src/task/Task.ts index 7caff083b..3ab7bc6fb 100644 --- a/packages/task-graph/src/task/Task.ts +++ b/packages/task-graph/src/task/Task.ts @@ -397,7 +397,7 @@ 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 already visited (for circular reference detection) + * @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, visited: WeakSet = new WeakSet()): any { @@ -414,8 +414,7 @@ export class Task< if (visited.has(obj)) { throw new Error( "Circular reference detected in input data. " + - "Cannot clone objects with circular references. " + - "Consider using class instances which are preserved by reference." + "Cannot clone objects with circular references." ); } @@ -437,19 +436,25 @@ export class Task< // Add object to visited set before recursing visited.add(obj); - // Deep clone arrays, preserving class instances within - if (Array.isArray(obj)) { - return obj.map((item) => this.smartClone(item, visited)); - } + 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], 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], visited); + } } + return result; + } finally { + // Remove from visited set after processing to allow the same object + // in different branches (non-circular references) + visited.delete(obj); } - return result; } /** From a9aa2fd0f605b7d608f1efcd32d3207a7d55cb53 Mon Sep 17 00:00:00 2001 From: Steven Roussey Date: Sun, 4 Jan 2026 02:05:31 +0000 Subject: [PATCH 4/4] Refactor TaskEvents to import TaskStatus from TaskTypes and add unit tests for smartClone method - Updated TaskEvents to import TaskStatus from the correct module. - Added comprehensive unit tests for the smartClone method, including cases for circular reference detection and handling various data structures. --- packages/task-graph/src/task/TaskEvents.ts | 2 +- .../src/test}/task/Task.smartClone.test.ts | 74 +++++++++---------- 2 files changed, 35 insertions(+), 41 deletions(-) rename packages/{task-graph/src => test/src/test}/task/Task.smartClone.test.ts (85%) 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/task-graph/src/task/Task.smartClone.test.ts b/packages/test/src/test/task/Task.smartClone.test.ts similarity index 85% rename from packages/task-graph/src/task/Task.smartClone.test.ts rename to packages/test/src/test/task/Task.smartClone.test.ts index 7e723b287..ac926ce7b 100644 --- a/packages/task-graph/src/task/Task.smartClone.test.ts +++ b/packages/test/src/test/task/Task.smartClone.test.ts @@ -4,10 +4,10 @@ * 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"; -import type { IExecuteContext } from "./ITask"; -import { Task } from "./Task"; // Test task class to access private smartClone method class TestSmartCloneTask extends Task<{ data: any }, { result: any }> { @@ -58,7 +58,7 @@ describe("Task.smartClone circular reference detection", () => { 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); @@ -67,7 +67,7 @@ describe("Task.smartClone circular reference detection", () => { 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]); @@ -76,28 +76,22 @@ describe("Task.smartClone circular reference detection", () => { 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" - ); + + 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" - ); + + 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" - ); + + expect(() => task.testSmartClone(arr)).toThrow("Circular reference detected in input data"); }); test("should throw error on complex circular reference chain", () => { @@ -105,20 +99,18 @@ describe("Task.smartClone circular reference detection", () => { 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" - ); + + 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); @@ -132,23 +124,25 @@ describe("Task.smartClone circular reference detection", () => { 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 preserve TypedArrays by reference", () => { + 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).toBe(typedArray); // Should be same reference + + 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", () => { @@ -169,15 +163,15 @@ describe("Task.smartClone circular reference detection", () => { level1: { level2: { level3: { - value: "deep" - } + value: "deep", + }, }, - array: [1, 2, { nested: true }] - } + 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); @@ -190,16 +184,16 @@ describe("Task.smartClone circular reference detection", () => { const obj = { users: [ { id: 1, name: "Alice" }, - { id: 2, name: "Bob" } + { id: 2, name: "Bob" }, ], settings: { theme: "dark", - features: ["feature1", "feature2"] - } + 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]);