From 09989b7aff7cd90ba92890b4ba9d7ab86c9c742b Mon Sep 17 00:00:00 2001 From: jaeone94 <89377375+jaeone94@users.noreply.github.com> Date: Tue, 24 Feb 2026 18:39:44 +0900 Subject: [PATCH] [refactor] Extract manager composables and execution utils (#9163) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Extracts inline logic from manager components into dedicated composables and utilities, and adds a cyclic subgraph fix. ## Changes - **`usePackInstall`**: New composable extracted from `PackInstallButton.vue` — handles conflict detection, payload construction, and `Promise.allSettled`-based batch installation - **`useApplyChanges`**: New shared composable extracted from `ManagerProgressToast.vue` — manages ComfyUI restart flow with reconnect timeout and post-reconnect refresh - **`executionIdUtil`**: New utility (`getAncestorExecutionIds`, `getParentExecutionIds`, `buildSubgraphExecutionPaths`) with unit tests; fixes infinite recursion on cyclic subgraph definitions ## Review Focus - `useApplyChanges` reconnect timeout (2 min) and setting restore logic - `buildSubgraphExecutionPaths` visited-set guard for cyclic subgraph defs ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9163-refactor-Extract-manager-composables-and-execution-utils-3116d73d365081f293d3d5484775ad48) by [Unito](https://www.unito.io) --- src/utils/executionIdUtil.test.ts | 98 +++++++++++++++ src/utils/executionIdUtil.ts | 71 +++++++++++ .../components/ManagerProgressToast.vue | 58 +-------- .../manager/button/PackInstallButton.vue | 95 ++------------ .../composables/nodePack/usePackInstall.ts | 116 +++++++++++++++++ .../manager/composables/useApplyChanges.ts | 118 ++++++++++++++++++ 6 files changed, 417 insertions(+), 139 deletions(-) create mode 100644 src/utils/executionIdUtil.test.ts create mode 100644 src/utils/executionIdUtil.ts create mode 100644 src/workbench/extensions/manager/composables/nodePack/usePackInstall.ts create mode 100644 src/workbench/extensions/manager/composables/useApplyChanges.ts diff --git a/src/utils/executionIdUtil.test.ts b/src/utils/executionIdUtil.test.ts new file mode 100644 index 0000000000..9d275311df --- /dev/null +++ b/src/utils/executionIdUtil.test.ts @@ -0,0 +1,98 @@ +import { describe, expect, it } from 'vitest' + +import type { ComfyNode } from '@/platform/workflow/validation/schemas/workflowSchema' +import { + buildSubgraphExecutionPaths, + getAncestorExecutionIds, + getParentExecutionIds +} from '@/utils/executionIdUtil' + +function node(id: number, type: string): ComfyNode { + return { id, type } as ComfyNode +} + +function subgraphDef(id: string, nodes: ComfyNode[]) { + return { id, name: id, nodes, inputNode: {}, outputNode: {} } +} + +describe('getAncestorExecutionIds', () => { + it('returns only itself for a root node', () => { + expect(getAncestorExecutionIds('65')).toEqual(['65']) + }) + + it('returns all ancestors including self for nested IDs', () => { + expect(getAncestorExecutionIds('65:70')).toEqual(['65', '65:70']) + expect(getAncestorExecutionIds('65:70:63')).toEqual([ + '65', + '65:70', + '65:70:63' + ]) + }) +}) + +describe('getParentExecutionIds', () => { + it('returns empty for a root node', () => { + expect(getParentExecutionIds('65')).toEqual([]) + }) + + it('returns all ancestors excluding self for nested IDs', () => { + expect(getParentExecutionIds('65:70')).toEqual(['65']) + expect(getParentExecutionIds('65:70:63')).toEqual(['65', '65:70']) + }) +}) + +describe('buildSubgraphExecutionPaths', () => { + it('returns empty map when there are no subgraph definitions', () => { + expect(buildSubgraphExecutionPaths([node(5, 'SomeNode')], [])).toEqual( + new Map() + ) + }) + + it('returns empty map when no root node matches a subgraph type', () => { + const def = subgraphDef('def-A', []) + expect( + buildSubgraphExecutionPaths([node(5, 'UnrelatedNode')], [def]) + ).toEqual(new Map()) + }) + + it('maps a single subgraph instance to its execution path', () => { + const def = subgraphDef('def-A', []) + const result = buildSubgraphExecutionPaths([node(5, 'def-A')], [def]) + expect(result.get('def-A')).toEqual(['5']) + }) + + it('collects multiple instances of the same subgraph type', () => { + const def = subgraphDef('def-A', []) + const result = buildSubgraphExecutionPaths( + [node(5, 'def-A'), node(10, 'def-A')], + [def] + ) + expect(result.get('def-A')).toEqual(['5', '10']) + }) + + it('builds nested execution paths for subgraphs within subgraphs', () => { + const innerDef = subgraphDef('def-B', []) + const outerDef = subgraphDef('def-A', [node(70, 'def-B')]) + const result = buildSubgraphExecutionPaths( + [node(5, 'def-A')], + [outerDef, innerDef] + ) + expect(result.get('def-A')).toEqual(['5']) + expect(result.get('def-B')).toEqual(['5:70']) + }) + + it('does not recurse infinitely on self-referential subgraph definitions', () => { + const cyclicDef = subgraphDef('def-A', [node(70, 'def-A')]) + expect(() => + buildSubgraphExecutionPaths([node(5, 'def-A')], [cyclicDef]) + ).not.toThrow() + }) + + it('does not recurse infinitely on mutually cyclic subgraph definitions', () => { + const defA = subgraphDef('def-A', [node(70, 'def-B')]) + const defB = subgraphDef('def-B', [node(80, 'def-A')]) + expect(() => + buildSubgraphExecutionPaths([node(5, 'def-A')], [defA, defB]) + ).not.toThrow() + }) +}) diff --git a/src/utils/executionIdUtil.ts b/src/utils/executionIdUtil.ts new file mode 100644 index 0000000000..17fcb8f10d --- /dev/null +++ b/src/utils/executionIdUtil.ts @@ -0,0 +1,71 @@ +import type { NodeExecutionId } from '@/types/nodeIdentification' +import type { ComfyNode } from '@/platform/workflow/validation/schemas/workflowSchema' +import { isSubgraphDefinition } from '@/platform/workflow/validation/schemas/workflowSchema' + +/** + * Returns all ancestor execution IDs for a given execution ID, including itself. + * + * Example: "65:70:63" → ["65", "65:70", "65:70:63"] + * @knipIgnoreUsedByStackedPR + */ +export function getAncestorExecutionIds( + executionId: string | NodeExecutionId +): NodeExecutionId[] { + const parts = executionId.split(':') + return Array.from({ length: parts.length }, (_, i) => + parts.slice(0, i + 1).join(':') + ) +} + +/** + * Returns all ancestor execution IDs for a given execution ID, excluding itself. + * + * Example: "65:70:63" → ["65", "65:70"] + * @knipIgnoreUsedByStackedPR + */ +export function getParentExecutionIds( + executionId: string | NodeExecutionId +): NodeExecutionId[] { + return getAncestorExecutionIds(executionId).slice(0, -1) +} + +/** + * "def-A" → ["5", "10"] for each container node instantiating that subgraph definition. + * @knipIgnoreUsedByStackedPR + */ +export function buildSubgraphExecutionPaths( + rootNodes: ComfyNode[], + allSubgraphDefs: unknown[] +): Map { + const subgraphDefMap = new Map( + allSubgraphDefs.filter(isSubgraphDefinition).map((s) => [s.id, s]) + ) + const pathMap = new Map() + const visited = new Set() + + const build = (nodes: ComfyNode[], parentPrefix: string) => { + for (const n of nodes ?? []) { + if (typeof n.type !== 'string' || !subgraphDefMap.has(n.type)) continue + const path = parentPrefix ? `${parentPrefix}:${n.id}` : String(n.id) + const existing = pathMap.get(n.type) + if (existing) { + existing.push(path) + } else { + pathMap.set(n.type, [path]) + } + + if (visited.has(n.type)) continue + visited.add(n.type) + + const innerDef = subgraphDefMap.get(n.type) + if (innerDef) { + build(innerDef.nodes, path) + } + + visited.delete(n.type) + } + } + + build(rootNodes, '') + return pathMap +} diff --git a/src/workbench/extensions/manager/components/ManagerProgressToast.vue b/src/workbench/extensions/manager/components/ManagerProgressToast.vue index 5218852402..69e05537af 100644 --- a/src/workbench/extensions/manager/components/ManagerProgressToast.vue +++ b/src/workbench/extensions/manager/components/ManagerProgressToast.vue @@ -1,5 +1,5 @@