mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-02-26 01:34:07 +00:00
feat: enhance manager dialog with initial pack id support (#9169)
## Summary Adds `initialPackId` support to the manager dialog so callers can deep-link directly to a specific node pack — pre-filling the search query, switching to packs search mode, and auto-selecting the matching pack once results load. ## Changes - **ManagerDialog.vue**: Added `initialPackId` prop; wires it into `useRegistrySearch` (forces `packs` mode and pre-fills query) and uses VueUse `until()` to auto-select the target pack and open the right panel once `resultsWithKeys` is populated (one-shot, never re-triggers). Also fixes a latent bug where the effective initial tab (resolving the persisted tab) was not used when determining the initial search mode and query — previously `initialTab` (the raw prop) was checked directly, which would produce incorrect pre-fill when no tab prop was passed but a Missing tab was persisted. - **useManagerDialog.ts**: Threads `initialPackId` through `show()` into the dialog props - **useManagerState.ts**: Exposes `initialPackId` in `openManager` options and passes it to `managerDialog.show()`; also removes a stale fallback `show(ManagerTab.All)` call that was redundant for the legacy-only error path ### Refactor: remove `executionIdUtil.ts` and distribute its functions - **`getAncestorExecutionIds` / `getParentExecutionIds`** → moved to `src/types/nodeIdentification.ts`: both are pure `NodeExecutionId` string operations with no external dependencies, consistent with the existing `parseNodeExecutionId` / `createNodeExecutionId` helpers already in that file - **`buildSubgraphExecutionPaths`** → moved to `src/platform/workflow/validation/schemas/workflowSchema.ts`: operates entirely on `ComfyNode[]` and `SubgraphDefinition` (both defined there), and `isSubgraphDefinition` is already co-located in the same file - Tests redistributed accordingly: ancestor/parent ID tests into `nodeIdentification.test.ts`, `buildSubgraphExecutionPaths` tests into `workflowSchema.test.ts` ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9169-feat-enhance-manager-dialog-with-initial-pack-id-support-3116d73d365081f7b6a3cbfb2f2755bf) by [Unito](https://www.unito.io)
This commit is contained in:
@@ -1,7 +1,11 @@
|
||||
import fs from 'fs'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import { validateComfyWorkflow } from '@/platform/workflow/validation/schemas/workflowSchema'
|
||||
import {
|
||||
buildSubgraphExecutionPaths,
|
||||
validateComfyWorkflow
|
||||
} from '@/platform/workflow/validation/schemas/workflowSchema'
|
||||
import type { ComfyNode } from '@/platform/workflow/validation/schemas/workflowSchema'
|
||||
import { defaultGraph } from '@/scripts/defaultGraph'
|
||||
|
||||
const WORKFLOW_DIR = 'src/platform/workflow/validation/schemas/__fixtures__'
|
||||
@@ -206,3 +210,67 @@ describe('parseComfyWorkflow', () => {
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
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('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()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -541,3 +541,47 @@ const zNodeData = z.object({
|
||||
|
||||
const zComfyApiWorkflow = z.record(zNodeId, zNodeData)
|
||||
export type ComfyApiWorkflow = z.infer<typeof zComfyApiWorkflow>
|
||||
|
||||
/**
|
||||
* Builds a map from subgraph definition ID to all execution path prefixes
|
||||
* where that definition is instantiated in the workflow.
|
||||
*
|
||||
* "def-A" → ["5", "10"] for each container node instantiating that subgraph definition.
|
||||
* @knipIgnoreUsedByStackedPR
|
||||
*/
|
||||
export function buildSubgraphExecutionPaths(
|
||||
rootNodes: ComfyNode[],
|
||||
allSubgraphDefs: unknown[]
|
||||
): Map<string, string[]> {
|
||||
const subgraphDefMap = new Map(
|
||||
allSubgraphDefs.filter(isSubgraphDefinition).map((s) => [s.id, s])
|
||||
)
|
||||
const pathMap = new Map<string, string[]>()
|
||||
const visited = new Set<string>()
|
||||
|
||||
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
|
||||
}
|
||||
|
||||
@@ -4,6 +4,8 @@ import type { NodeId } from '@/platform/workflow/validation/schemas/workflowSche
|
||||
import {
|
||||
createNodeExecutionId,
|
||||
createNodeLocatorId,
|
||||
getAncestorExecutionIds,
|
||||
getParentExecutionIds,
|
||||
isNodeExecutionId,
|
||||
isNodeLocatorId,
|
||||
parseNodeExecutionId,
|
||||
@@ -204,4 +206,30 @@ describe('nodeIdentification', () => {
|
||||
expect(parsed).toEqual(nodeIds)
|
||||
})
|
||||
})
|
||||
|
||||
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'])
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -121,3 +121,30 @@ export function parseNodeExecutionId(id: string): NodeId[] | null {
|
||||
export function createNodeExecutionId(nodeIds: NodeId[]): NodeExecutionId {
|
||||
return nodeIds.join(':')
|
||||
}
|
||||
|
||||
/**
|
||||
* 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)
|
||||
}
|
||||
|
||||
@@ -1,98 +0,0 @@
|
||||
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()
|
||||
})
|
||||
})
|
||||
@@ -1,71 +0,0 @@
|
||||
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<string, string[]> {
|
||||
const subgraphDefMap = new Map(
|
||||
allSubgraphDefs.filter(isSubgraphDefinition).map((s) => [s.id, s])
|
||||
)
|
||||
const pathMap = new Map<string, string[]>()
|
||||
const visited = new Set<string>()
|
||||
|
||||
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
|
||||
}
|
||||
@@ -165,7 +165,7 @@
|
||||
</template>
|
||||
|
||||
<script setup lang="ts">
|
||||
import { whenever } from '@vueuse/core'
|
||||
import { until, whenever } from '@vueuse/core'
|
||||
import { merge, stubTrue } from 'es-toolkit/compat'
|
||||
import type { AutoCompleteOptionSelectEvent } from 'primevue/autocomplete'
|
||||
import {
|
||||
@@ -211,8 +211,9 @@ import { useManagerState } from '@/workbench/extensions/manager/composables/useM
|
||||
import { useComfyManagerStore } from '@/workbench/extensions/manager/stores/comfyManagerStore'
|
||||
import { ManagerTab } from '@/workbench/extensions/manager/types/comfyManagerTypes'
|
||||
|
||||
const { initialTab, onClose } = defineProps<{
|
||||
const { initialTab, initialPackId, onClose } = defineProps<{
|
||||
initialTab?: ManagerTab
|
||||
initialPackId?: string
|
||||
onClose: () => void
|
||||
}>()
|
||||
|
||||
@@ -347,8 +348,14 @@ const {
|
||||
sortOptions
|
||||
} = useRegistrySearch({
|
||||
initialSortField: initialState.sortField,
|
||||
initialSearchMode: initialState.searchMode,
|
||||
initialSearchQuery: initialState.searchQuery
|
||||
initialSearchMode:
|
||||
initialPackId && initialTabId !== ManagerTab.Missing
|
||||
? 'packs'
|
||||
: initialState.searchMode,
|
||||
initialSearchQuery:
|
||||
initialTabId === ManagerTab.Missing
|
||||
? ''
|
||||
: (initialPackId ?? initialState.searchQuery)
|
||||
})
|
||||
pageNumber.value = 0
|
||||
|
||||
@@ -475,6 +482,19 @@ watch(
|
||||
}
|
||||
)
|
||||
|
||||
// Auto-select the pack matching initialPackId once
|
||||
if (initialPackId) {
|
||||
until(resultsWithKeys)
|
||||
.toMatch((packs) => packs.some((p) => p.id === initialPackId))
|
||||
.then((packs) => {
|
||||
const target = packs.find((p) => p.id === initialPackId)
|
||||
if (target && selectedNodePacks.value.length === 0) {
|
||||
selectedNodePacks.value = [target]
|
||||
isRightPanelOpen.value = true
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
const getLoadingCount = () => {
|
||||
switch (selectedTab.value?.id) {
|
||||
case ManagerTab.AllInstalled:
|
||||
|
||||
@@ -13,13 +13,14 @@ export function useManagerDialog() {
|
||||
dialogStore.closeDialog({ key: DIALOG_KEY })
|
||||
}
|
||||
|
||||
function show(initialTab?: ManagerTab) {
|
||||
function show(initialTab?: ManagerTab, initialPackId?: string) {
|
||||
dialogService.showLayoutDialog({
|
||||
key: DIALOG_KEY,
|
||||
component: ManagerDialog,
|
||||
props: {
|
||||
onClose: hide,
|
||||
initialTab
|
||||
initialTab,
|
||||
initialPackId
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
@@ -8,7 +8,7 @@ import { useSettingsDialog } from '@/platform/settings/composables/useSettingsDi
|
||||
import { useCommandStore } from '@/stores/commandStore'
|
||||
import { useSystemStatsStore } from '@/stores/systemStatsStore'
|
||||
import { useManagerDialog } from '@/workbench/extensions/manager/composables/useManagerDialog'
|
||||
import { ManagerTab } from '@/workbench/extensions/manager/types/comfyManagerTypes'
|
||||
import type { ManagerTab } from '@/workbench/extensions/manager/types/comfyManagerTypes'
|
||||
|
||||
export enum ManagerUIState {
|
||||
DISABLED = 'disabled',
|
||||
@@ -143,6 +143,7 @@ export function useManagerState() {
|
||||
*/
|
||||
const openManager = async (options?: {
|
||||
initialTab?: ManagerTab
|
||||
initialPackId?: string
|
||||
legacyCommand?: string
|
||||
showToastOnLegacyError?: boolean
|
||||
isLegacyOnly?: boolean
|
||||
@@ -181,16 +182,14 @@ export function useManagerState() {
|
||||
|
||||
case ManagerUIState.NEW_UI:
|
||||
if (options?.isLegacyOnly) {
|
||||
// Legacy command is not available in NEW_UI mode
|
||||
useToastStore().add({
|
||||
severity: 'error',
|
||||
summary: t('g.error'),
|
||||
detail: t('manager.legacyMenuNotAvailable'),
|
||||
life: 3000
|
||||
})
|
||||
await managerDialog.show(ManagerTab.All)
|
||||
} else {
|
||||
await managerDialog.show(options?.initialTab)
|
||||
managerDialog.show(options?.initialTab, options?.initialPackId)
|
||||
}
|
||||
break
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user