feat: show missing node packs in Errors Tab with install support#9213
Merged
christian-byrne merged 14 commits intomainfrom Feb 26, 2026
Merged
feat: show missing node packs in Errors Tab with install support#9213christian-byrne merged 14 commits intomainfrom
christian-byrne merged 14 commits intomainfrom
Conversation
…tation - App & WorkflowService: Document the temporary coexistence of the Missing Nodes Modal and Errors Tab, noting that the modal will be removed once Node Replacement is implemented. - Error Handling: Collect `cnr_id` and `execution_id` when processing missing nodes to provide sufficient context for the Errors Tab. - ExecutionErrorStore: Enforce strict `NodeExecutionId` typing in `applyNodeError`. - Clean up obsolete comments and reorganize imports across error stores and app script.
- Add MissingPackGroupRow component to display missing node packs as grouped, collapsible rows with per-pack install and manager-search buttons. - Implement buildMissingNodeGroups() in useErrorGroups.ts to aggregate missing node types by cnrId and resolve pack info asynchronously. - Add locate-on-canvas support for individual missing node instances. - Fix deferred-warnings path: call setMissingNodeTypes before showErrorOverlay so the Errors Tab has data to display. - Add required i18n keys for missing node pack UI strings.
jaeone94
added
enhancement
dosubot
bot
added
the
size:XL
🎨 Storybook: ✅ Built — View Storybook |
🎭 Playwright: ✅ 545 passed, 0 failed · 6 flaky📊 Browser Reports
|
Contributor
📝 WalkthroughWalkthroughAdds missing-node detection and storage, resolves missing-node pack IDs, and surfaces a MissingNode UI with install/locate/apply flows in the right-side panel; updates types, stores, workflow wiring, tests, and i18n to support the feature. Changes
Sequence DiagramsequenceDiagram
participant App as App / Workflow
participant WorkflowSvc as WorkflowService
participant ErrorStore as ExecutionErrorStore
participant ErrorGroups as useErrorGroups
participant RightPanel as RightSidePanel
participant UI as MissingNodeCard/PackGroupRow
participant Manager as ComfyManagerStore
App->>WorkflowSvc: detect missing nodes (collectMissingNodesAndModels)
WorkflowSvc->>ErrorStore: surfaceMissingNodes / setMissingNodeTypes(...)
ErrorStore->>ErrorStore: store missingNodesError\ncompute activeMissingNodeGraphIds
RightPanel->>ErrorStore: read activeMissingNodeGraphIds & error refs
RightPanel->>ErrorGroups: request grouped errors (includes missing nodes)
ErrorGroups->>ErrorGroups: async resolve pack IDs via registry\nbuild missingPackGroups
RightPanel->>UI: render MissingNodeCard with missingPackGroups
UI->>Manager: query install state / request install
UI->>RightPanel: emit locateNode / openManagerInfo
Manager-->>UI: respond (installing/installed)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📦 Bundle: 4.43 MB gzip 🔴 +5.09 kBDetailsSummary
Category Glance App Entry Points — 17.9 kB (baseline 17.9 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 1 MB (baseline 986 kB) • 🔴 +18.6 kBGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 72.1 kB (baseline 72.1 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 9 added / 9 removed Panels & Settings — 435 kB (baseline 435 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 10 added / 10 removed User & Accounts — 16 kB (baseline 16 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 5 added / 5 removed Editors & Dialogs — 736 B (baseline 736 B) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 1 added / 1 removed UI Components — 47.1 kB (baseline 47.1 kB) • ⚪ 0 BReusable component library chunks
Status: 5 added / 5 removed Data & Services — 2.55 MB (baseline 2.54 MB) • 🔴 +5.38 kBStores, services, APIs, and repositories
Status: 13 added / 13 removed Utilities & Hooks — 55.5 kB (baseline 55.5 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 12 added / 12 removed Vendor & Third-Party — 8.84 MB (baseline 8.84 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 7.74 MB (baseline 7.74 MB) • 🔴 +746 BBundles that do not match a named category
Status: 55 added / 55 removed |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/platform/workflow/core/services/workflowService.ts (1)
472-485:⚠️ Potential issue | 🟡 MinorMissing-node panel population is silently skipped when
ShowMissingNodesWarningis disabled.
setMissingNodeTypesandshowErrorOverlayare both inside themissingNodeTypes?.length && settingStore.get('Comfy.Workflow.ShowMissingNodesWarning')guard. If a user turns off the "show missing nodes dialog" setting, the Errors Tab will also not reflect missing nodes — even whenShowErrorsTabis enabled. The two settings control fundamentally different UX surfaces (modal dialog vs. right panel), so coupling them means a user can end up with the panel silently empty.Consider calling
setMissingNodeTypesunconditionally whenmissingNodeTypes?.lengthis truthy, gating only the dialog and the overlay behind their respective settings:🔧 Proposed fix
- if ( - missingNodeTypes?.length && - settingStore.get('Comfy.Workflow.ShowMissingNodesWarning') - ) { - missingNodesDialog.show({ missingNodeTypes }) - - // For now, we'll make them coexist. - // Once the Node Replacement feature is implemented in TabErrors - // we'll remove the modal display and direct users to the error tab. - executionErrorStore.setMissingNodeTypes(missingNodeTypes) - if (settingStore.get('Comfy.RightSidePanel.ShowErrorsTab')) { - executionErrorStore.showErrorOverlay() - } - } + if (missingNodeTypes?.length) { + executionErrorStore.setMissingNodeTypes(missingNodeTypes) + + if (settingStore.get('Comfy.Workflow.ShowMissingNodesWarning')) { + missingNodesDialog.show({ missingNodeTypes }) + } + + // For now, we'll make them coexist. + // Once the Node Replacement feature is implemented in TabErrors + // we'll remove the modal display and direct users to the error tab. + if (settingStore.get('Comfy.RightSidePanel.ShowErrorsTab')) { + executionErrorStore.showErrorOverlay() + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/platform/workflow/core/services/workflowService.ts` around lines 472 - 485, The current guard couples the dialog setting with panel population; ensure missing node types are always recorded by calling executionErrorStore.setMissingNodeTypes(missingNodeTypes) whenever missingNodeTypes?.length is truthy, then separately gate missingNodesDialog.show(...) behind settingStore.get('Comfy.Workflow.ShowMissingNodesWarning') and gate executionErrorStore.showErrorOverlay() behind settingStore.get('Comfy.RightSidePanel.ShowErrorsTab'); update the block around missingNodeTypes, missingNodesDialog.show, executionErrorStore.setMissingNodeTypes and executionErrorStore.showErrorOverlay so the dialog and overlay checks are independent while setMissingNodeTypes runs unconditionally when nodes exist.src/scripts/app.ts (1)
31-37:⚠️ Potential issue | 🟠 MajorSplit mixed type/value import from
workflowSchema.This import uses inline
typealongside runtime imports. Split into separateimport typeand value imports to match repo rules.The types (
ComfyApiWorkflow,ComfyWorkflowJSON,ModelFile,NodeId) should be in a dedicatedimport typestatement, with the value imports (isSubgraphDefinition,buildSubgraphExecutionPaths) in a separateimportstatement, both from the same module.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/scripts/app.ts` around lines 31 - 37, Split the mixed type/value import from workflowSchema into two imports: move ComfyApiWorkflow, ComfyWorkflowJSON, ModelFile, and NodeId into an "import type" statement, and keep isSubgraphDefinition and buildSubgraphExecutionPaths in a separate runtime "import" from the same module; update the existing import line that currently mixes these symbols so type-only names use import type and the value names remain a normal import to satisfy the repo rule.
🧹 Nitpick comments (2)
src/components/rightSidePanel/parameters/SectionWidgets.vue (1)
134-138: ExtractuseSettingStore()to setup scope rather than calling it insidecomputed().Calling composables inside a
computed()getter is non-idiomatic. While Pinia returns the same singleton store instance, this pattern can confuse static analysis tools, makes the dependency graph less explicit, and mirrors an existing pre-existing issue innavigateToErrorTab. A singlesettingStoreref at setup level covers both usages.♻️ Proposed refactor
const canvasStore = useCanvasStore() const executionErrorStore = useExecutionErrorStore() const rightSidePanelStore = useRightSidePanelStore() const nodeDefStore = useNodeDefStore() +const settingStore = useSettingStore() const { t } = useI18n() ... -const showSeeError = computed( - () => - nodeHasError.value && - useSettingStore().get('Comfy.RightSidePanel.ShowErrorsTab') -) +const showSeeError = computed( + () => + nodeHasError.value && + settingStore.get('Comfy.RightSidePanel.ShowErrorsTab') +) ... function navigateToErrorTab() { if (!targetNode.value) return - if (!useSettingStore().get('Comfy.RightSidePanel.ShowErrorsTab')) return + if (!settingStore.get('Comfy.RightSidePanel.ShowErrorsTab')) return rightSidePanelStore.focusedErrorNodeId = String(targetNode.value.id) rightSidePanelStore.openPanel('errors') }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/rightSidePanel/parameters/SectionWidgets.vue` around lines 134 - 138, Extract useSettingStore() at the setup scope and reuse that instance instead of calling it inside computed/getters: create a const (e.g. settingStore = useSettingStore()) in setup and replace the inline calls in showSeeError (the computed) and in navigateToErrorTab so both reference settingStore.get('Comfy.RightSidePanel.ShowErrorsTab'); this makes the dependency explicit and avoids calling the composable inside computed.src/components/rightSidePanel/errors/TabErrors.test.ts (1)
21-22: Add behavioral tests for missing-node pack scenarios.The mock is correctly expanded to include
mapAllNodes, but there are no new tests covering the new UI paths introduced in this PR — specifically thatMissingPackGroupRowrenders whenmissingNodeTypesare present in the store, that the install button triggers the correct action, and thatMissingNodeCardappears for unresolvable nodes. These are critical new behaviors with no test coverage.Would you like me to draft test cases for the
MissingPackGroupRowrendering and interaction paths? I can open a new issue to track this if preferred.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/rightSidePanel/errors/TabErrors.test.ts` around lines 21 - 22, Add behavioral tests in TabErrors.test.ts to cover the new missing-node pack UI paths: mock the store/selectors so mapAllNodes returns nodes with missingNodeTypes and ensure MissingPackGroupRow renders when missingNodeTypes is present; simulate clicking the install button in MissingPackGroupRow and assert the expected action/handler is invoked (e.g., the installPack/installMissingNodes action or the component method wired to the button); also add a test where mapAllNodes returns unresolvable nodes and assert MissingNodeCard is rendered for those nodes. Use the existing vi mocks (forEachNode, mapAllNodes) and component render helpers to locate and interact with MissingPackGroupRow and MissingNodeCard.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/rightSidePanel/errors/MissingPackGroupRow.vue`:
- Around line 77-84: The Locate button is rendered even when no nodeId exists
(so handleLocateNode is a no-op for string-only nodeType); update the template
in MissingPackGroupRow.vue to only render the Button when a valid nodeId is
available (e.g., add a v-if checking nodeId or the computed that derives it) and
apply the same guard for the other occurrence mentioned (lines ~236-240); keep
the click handler as-is (handleLocateNode) but hide the control when nodeId is
falsy so the UI doesn't show a non-working action.
- Around line 27-35: The icon-only Buttons (the Button component used with
v-if="showInfoButton && group.packId !== null" that calls
emit('openManagerInfo', group.packId ?? '') and the other similar Buttons at
lines 36-50 and 77-84) currently have no accessible name; add an explicit
accessible label by adding an aria-label or aria-labelledby prop to each Button
(e.g., aria-label="Open pack info" or a localized string) so screen readers get
context, ensuring the label succinctly describes the action invoked by the
Button that uses the icon-only markup.
In `@src/scripts/app.ts`:
- Around line 1101-1111: The missing-node state
(executionErrorStore.setMissingNodeTypes) and overlay display
(executionErrorStore.showErrorOverlay) are incorrectly nested inside the
modal-gated check for Comfy.Workflow.ShowMissingNodesWarning; extract the
useExecutionErrorStore() call and always call
setMissingNodeTypes(missingNodeTypes) and, if Comfy.RightSidePanel.ShowErrorsTab
is enabled, showErrorOverlay() outside of the useMissingNodesDialog() modal
branch so that missing-node data is always sent to the Errors Tab regardless of
the modal setting (keep the modal invocation useMissingNodesDialog().show({
missingNodeTypes }) inside the original conditional).
---
Outside diff comments:
In `@src/platform/workflow/core/services/workflowService.ts`:
- Around line 472-485: The current guard couples the dialog setting with panel
population; ensure missing node types are always recorded by calling
executionErrorStore.setMissingNodeTypes(missingNodeTypes) whenever
missingNodeTypes?.length is truthy, then separately gate
missingNodesDialog.show(...) behind
settingStore.get('Comfy.Workflow.ShowMissingNodesWarning') and gate
executionErrorStore.showErrorOverlay() behind
settingStore.get('Comfy.RightSidePanel.ShowErrorsTab'); update the block around
missingNodeTypes, missingNodesDialog.show,
executionErrorStore.setMissingNodeTypes and executionErrorStore.showErrorOverlay
so the dialog and overlay checks are independent while setMissingNodeTypes runs
unconditionally when nodes exist.
In `@src/scripts/app.ts`:
- Around line 31-37: Split the mixed type/value import from workflowSchema into
two imports: move ComfyApiWorkflow, ComfyWorkflowJSON, ModelFile, and NodeId
into an "import type" statement, and keep isSubgraphDefinition and
buildSubgraphExecutionPaths in a separate runtime "import" from the same module;
update the existing import line that currently mixes these symbols so type-only
names use import type and the value names remain a normal import to satisfy the
repo rule.
---
Nitpick comments:
In `@src/components/rightSidePanel/errors/TabErrors.test.ts`:
- Around line 21-22: Add behavioral tests in TabErrors.test.ts to cover the new
missing-node pack UI paths: mock the store/selectors so mapAllNodes returns
nodes with missingNodeTypes and ensure MissingPackGroupRow renders when
missingNodeTypes is present; simulate clicking the install button in
MissingPackGroupRow and assert the expected action/handler is invoked (e.g., the
installPack/installMissingNodes action or the component method wired to the
button); also add a test where mapAllNodes returns unresolvable nodes and assert
MissingNodeCard is rendered for those nodes. Use the existing vi mocks
(forEachNode, mapAllNodes) and component render helpers to locate and interact
with MissingPackGroupRow and MissingNodeCard.
In `@src/components/rightSidePanel/parameters/SectionWidgets.vue`:
- Around line 134-138: Extract useSettingStore() at the setup scope and reuse
that instance instead of calling it inside computed/getters: create a const
(e.g. settingStore = useSettingStore()) in setup and replace the inline calls in
showSeeError (the computed) and in navigateToErrorTab so both reference
settingStore.get('Comfy.RightSidePanel.ShowErrorsTab'); this makes the
dependency explicit and avoids calling the composable inside computed.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
src/components/rightSidePanel/RightSidePanel.vuesrc/components/rightSidePanel/errors/ErrorNodeCard.vuesrc/components/rightSidePanel/errors/MissingNodeCard.vuesrc/components/rightSidePanel/errors/MissingPackGroupRow.vuesrc/components/rightSidePanel/errors/TabErrors.test.tssrc/components/rightSidePanel/errors/TabErrors.vuesrc/components/rightSidePanel/errors/types.tssrc/components/rightSidePanel/errors/useErrorGroups.tssrc/components/rightSidePanel/layout/PropertiesAccordionItem.vuesrc/components/rightSidePanel/parameters/SectionWidgets.vuesrc/locales/en/main.jsonsrc/platform/workflow/core/services/workflowService.tssrc/platform/workflow/validation/schemas/workflowSchema.tssrc/renderer/extensions/vueNodes/components/LGraphNode.vuesrc/scripts/app.tssrc/stores/executionErrorStore.tssrc/types/comfy.tssrc/types/nodeIdentification.ts
💤 Files with no reviewable changes (2)
- src/platform/workflow/validation/schemas/workflowSchema.ts
- src/types/nodeIdentification.ts
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/scripts/app.ts (1)
1102-1115: LGTM — previous gating issue is resolved.
setMissingNodeTypesand the conditionalshowErrorOverlaynow execute unconditionally, independent of theShowMissingNodesWarningsetting, while the modal dialog remains behind its own guard.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/scripts/app.ts` around lines 1102 - 1115, Ensure the behavior is intentional: keep useExecutionErrorStore().setMissingNodeTypes(missingNodeTypes) executed unconditionally and keep the conditional call to executionErrorStore.showErrorOverlay() (guarded by useSettingStore().get('Comfy.RightSidePanel.ShowErrorsTab')) outside the ShowMissingNodesWarning guard, while leaving the modal dialog call useMissingNodesDialog().show({ missingNodeTypes }) inside the if (useSettingStore().get('Comfy.Workflow.ShowMissingNodesWarning')) block; verify these placements in the showMissingNodesError method so the error tab state is always updated but the modal remains optional.
🧹 Nitpick comments (1)
src/scripts/app.ts (1)
1213-1218: PreferconstoverletforcnrId.
cnrIdis aletthat is set once inside an if/else-if and never mutated again, violating the "avoid mutable state; prefer immutability and assignment at point of declaration" guideline.♻️ Proposed refactor
- let cnrId: string | undefined - if (typeof n.properties?.cnr_id === 'string') { - cnrId = n.properties.cnr_id - } else if (typeof n.properties?.aux_id === 'string') { - cnrId = n.properties.aux_id - } + const cnrId = + typeof n.properties?.cnr_id === 'string' + ? n.properties.cnr_id + : typeof n.properties?.aux_id === 'string' + ? n.properties.aux_id + : undefinedAs per coding guidelines: "Avoid mutable state; prefer immutability and assignment at point of declaration."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/scripts/app.ts` around lines 1213 - 1218, Replace the mutable let cnrId with an immutable const by assigning it at declaration using a conditional expression that checks n.properties?.cnr_id and falls back to n.properties?.aux_id; update references to cnrId unchanged (symbols: cnrId, n.properties?.cnr_id, n.properties?.aux_id) so the variable is initialized once and not mutated later.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/scripts/app.ts`:
- Around line 1102-1115: Ensure the behavior is intentional: keep
useExecutionErrorStore().setMissingNodeTypes(missingNodeTypes) executed
unconditionally and keep the conditional call to
executionErrorStore.showErrorOverlay() (guarded by
useSettingStore().get('Comfy.RightSidePanel.ShowErrorsTab')) outside the
ShowMissingNodesWarning guard, while leaving the modal dialog call
useMissingNodesDialog().show({ missingNodeTypes }) inside the if
(useSettingStore().get('Comfy.Workflow.ShowMissingNodesWarning')) block; verify
these placements in the showMissingNodesError method so the error tab state is
always updated but the modal remains optional.
---
Nitpick comments:
In `@src/scripts/app.ts`:
- Around line 1213-1218: Replace the mutable let cnrId with an immutable const
by assigning it at declaration using a conditional expression that checks
n.properties?.cnr_id and falls back to n.properties?.aux_id; update references
to cnrId unchanged (symbols: cnrId, n.properties?.cnr_id, n.properties?.aux_id)
so the variable is initialized once and not mutated later.
…s_type When the /prompt endpoint returns a missing_node_type error with class_type null, resolve the graph node via its execution ID to recover class_type, node_title, cnrId, and nodeId. This ensures the Errors Tab shows the correct pack group and the Locate button for prompt-error-sourced missing nodes. Extract getCnrIdFromProperties as a shared helper and consolidate cnrId utilities in missingNodeErrorUtil.
jaeone94
requested review from
christian-byrne,
ltdrdata and
viva-jinyi
as code owners
dosubot
bot
removed
the
size:XL
dosubot
bot
added
the
size:XXL
…thub.com/Comfy-Org/ComfyUI_frontend into feat/install-missing-node-from-error-tab
Contributor
christian-byrne
left a comment
christian-byrne
left a comment
There was a problem hiding this comment.
Took a look through the full diff. Overall the feature design is solid - grouping missing nodes by pack, async registry resolution, and install support are all well thought-out. Left a few comments below.
…ndling - Extract coexist logic into executionErrorStore.surfaceMissingNodes() to eliminate duplicate setMissingNodeTypes + showErrorOverlay blocks in app.showMissingNodesError and workflowService.showPendingWarnings - Replace ErrorGroup interface with a discriminated union type so that execution groups always carry cards and missing_node groups carry none; remove the placeholder card that buildMissingNodeGroups was creating solely to satisfy the old shape - Fix async pack resolution in useErrorGroups to cancel stale watch callbacks via onCleanup and resolve packs in parallel with Promise.all instead of sequentially - Fix Missing Node Packs count badge to use missingPackGroups.length so the unknown group counts as 1 instead of showing (0) when all nodes are unresolved - Remove @knipIgnoreUsedByStackedPR annotations that are no longer needed now that the stacked PRs have landed - Add unit tests for setMissingNodeTypes covering all three deduplication strategies: string value, nodeId key, and type key fallback
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/rightSidePanel/errors/useErrorGroups.ts`:
- Around line 392-422: The watch callback can leave entries stuck as RESOLVING
on cancellation or when one lookup rejects; update the toResolve logic and
result handling: change the filter for toResolve to include nodes whose
asyncResolvedIds entry is absent OR currently RESOLVING (e.g. const toResolve =
pending.filter(n => { const v = asyncResolvedIds.value.get(n.type); return v ===
undefined || v === RESOLVING; })), perform lookups with Promise.allSettled (or
per-item try/catch) instead of Promise.all so a single rejection doesn't abort
all, map rejected results to a safe fallback (e.g. packId = null) and always
replace RESOLVING entries in asyncResolvedIds with the settled result for each
type (while still respecting the cancelled flag before committing the final
Map), keeping references to asyncResolvedIds, RESOLVING, inferPackFromNodeName,
toResolve, results, and cancelled to locate changes.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/components/rightSidePanel/errors/TabErrors.vuesrc/components/rightSidePanel/errors/types.tssrc/components/rightSidePanel/errors/useErrorGroups.tssrc/platform/workflow/core/services/workflowService.tssrc/scripts/app.tssrc/stores/executionErrorStore.tssrc/stores/executionStore.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/platform/workflow/core/services/workflowService.ts
…ures - Replace Promise.all with Promise.allSettled so a single failed lookup doesn't leave other pack types stuck in RESOLVING forever - Clean up RESOLVING markers in onCleanup so cancelled watch callbacks don't permanently block retries on the next run - Change filter from .has() to .get() === undefined so entries cleared by cancellation are eligible for retry
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/components/rightSidePanel/errors/useErrorGroups.ts (2)
477-481: Make the sort comparator total for equal keys.When both values are
null, returning1violates comparator symmetry.
Returning0for equal keys is safer and deterministic.🔧 Suggested tweak
return Array.from(map.entries()) .sort(([packIdA], [packIdB]) => { + if (packIdA === packIdB) return 0 // null (Unknown Pack) always goes last if (packIdA === null) return 1 if (packIdB === null) return -1 return packIdA.localeCompare(packIdB) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/rightSidePanel/errors/useErrorGroups.ts` around lines 477 - 481, The comparator passed to .sort in useErrorGroups.ts is not total: when both packIdA and packIdB are null (or equal) it currently returns 1 which breaks symmetry; update the comparator used in the sort (the function taking [packIdA], [packIdB]) to explicitly handle both-null and equal packId cases by returning 0 for equality, keep the existing logic for one-null cases, and otherwise use packIdA.localeCompare(packIdB) so the comparator is deterministic and symmetric.
395-420: Deduplicate by node type before calling registry inference.Current logic resolves per missing-node occurrence, so repeated node instances
with the sametypecan trigger redundant concurrent calls to
inferPackFromNodeName.♻️ Suggested refactor
- const toResolve = pending.filter( - (n) => asyncResolvedIds.value.get(n.type) === undefined - ) - if (!toResolve.length) return - - const resolvingTypes = toResolve.map((n) => n.type) + const toResolveTypes = Array.from( + new Set( + pending + .map((n) => n.type) + .filter((type) => asyncResolvedIds.value.get(type) === undefined) + ) + ) + if (!toResolveTypes.length) return + + const resolvingTypes = toResolveTypes @@ - const results = await Promise.allSettled( - toResolve.map(async (n) => ({ - type: n.type, - packId: (await inferPackFromNodeName.call(n.type))?.id ?? null - })) - ) + const results = await Promise.allSettled( + toResolveTypes.map(async (type) => ({ + type, + packId: (await inferPackFromNodeName.call(type))?.id ?? null + })) + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/rightSidePanel/errors/useErrorGroups.ts` around lines 395 - 420, The code currently calls inferPackFromNodeName for each pending node instance, causing redundant concurrent inferences for duplicate node types; change the flow to deduplicate by node.type before marking RESOLVING and before calling inferPackFromNodeName: compute a uniqueTypes array from toResolve (e.g., via Set on toResolve.map(n => n.type)), use uniqueTypes when building resolvingTypes and when setting asyncResolvedIds to RESOLVING, then call Promise.allSettled over uniqueTypes and map each settled result back to its type (type + packId) so you can apply the resolved packId to all pending nodes of that type; keep references to asyncResolvedIds, RESOLVING, toResolve, inferPackFromNodeName and onCleanup when making these changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/rightSidePanel/errors/useErrorGroups.ts`:
- Around line 477-481: The comparator passed to .sort in useErrorGroups.ts is
not total: when both packIdA and packIdB are null (or equal) it currently
returns 1 which breaks symmetry; update the comparator used in the sort (the
function taking [packIdA], [packIdB]) to explicitly handle both-null and equal
packId cases by returning 0 for equality, keep the existing logic for one-null
cases, and otherwise use packIdA.localeCompare(packIdB) so the comparator is
deterministic and symmetric.
- Around line 395-420: The code currently calls inferPackFromNodeName for each
pending node instance, causing redundant concurrent inferences for duplicate
node types; change the flow to deduplicate by node.type before marking RESOLVING
and before calling inferPackFromNodeName: compute a uniqueTypes array from
toResolve (e.g., via Set on toResolve.map(n => n.type)), use uniqueTypes when
building resolvingTypes and when setting asyncResolvedIds to RESOLVING, then
call Promise.allSettled over uniqueTypes and map each settled result back to its
type (type + packId) so you can apply the resolved packId to all pending nodes
of that type; keep references to asyncResolvedIds, RESOLVING, toResolve,
inferPackFromNodeName and onCleanup when making these changes.
christian-byrne
pushed a commit
that referenced
this pull request
Feb 27, 2026
## Summary Surfaces missing node pack information in the Errors Tab, grouped by registry pack, with one-click install support via ComfyUI Manager. ## Changes - **What**: Errors Tab now groups missing nodes by their registry pack and shows a `MissingPackGroupRow` with pack name, node/pack counts, and an Install button that triggers Manager installation. A `MissingNodeCard` shows individual unresolvable nodes that have no associated pack. `useErrorGroups` was extended to resolve missing node types to their registry packs using the `/api/workflow/missing_nodes` endpoint. `executionErrorStore` was refactored to track missing node types separately from execution errors and expose them reactively. - **Breaking**: None ## Review Focus - `useErrorGroups.ts` — the new `resolveMissingNodePacks` logic fetches pack metadata and maps node types to pack IDs; edge cases around partial resolution (some nodes have a pack, some don't) produce both `MissingPackGroupRow` and `MissingNodeCard` entries - `executionErrorStore.ts` — the store now separates `missingNodeTypes` state from `errors`; the deferred-warnings path in `app.ts` now calls `setMissingNodeTypes` so the Errors Tab is populated even when a workflow loads without executing ## Screenshots (if applicable) https://github.com/user-attachments/assets/97f8d009-0cac-4739-8740-fd3333b5a85b ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9213-feat-show-missing-node-packs-in-Errors-Tab-with-install-support-3126d73d36508197bc4bf8ebfd2125c8) by [Unito](https://www.unito.io)
This was referenced Mar 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Surfaces missing node pack information in the Errors Tab, grouped by registry pack, with one-click install support via ComfyUI Manager.
Changes
MissingPackGroupRowwith pack name, node/pack counts, and an Install button that triggers Manager installation. AMissingNodeCardshows individual unresolvable nodes that have no associated pack.useErrorGroupswas extended to resolve missing node types to their registry packs using the/api/workflow/missing_nodesendpoint.executionErrorStorewas refactored to track missing node types separately from execution errors and expose them reactively.Review Focus
useErrorGroups.ts— the newresolveMissingNodePackslogic fetches pack metadata and maps node types to pack IDs; edge cases around partial resolution (some nodes have a pack, some don't) produce bothMissingPackGroupRowandMissingNodeCardentriesexecutionErrorStore.ts— the store now separatesmissingNodeTypesstate fromerrors; the deferred-warnings path inapp.tsnow callssetMissingNodeTypesso the Errors Tab is populated even when a workflow loads without executingScreenshots (if applicable)
2026-02-26.000449.mp4
┆Issue is synchronized with this Notion page by Unito