Fix cyclic prototype errors with subgraphNodes (#5637)

To accomplish this, it pulls WidgetLocator information from the node
owning the widget.

This `node` property does not exist on all IBaseWidget. `toConcrete` was
used to instead have a BaseWidget which is guaranteed to have a node
property. The issue that was missed, is that a widget which lacks this
information (such as most implemented by custom nodes) sets the node
value to the argument which was passed. Here that is the reference to
the subgraph node. Sometimes, this `#setWidget` call is made multiple
times, and when this occurs, the `input.widget` has itself set as the
protoyep, throwing an error.

This is resolved by instead taking an additional input which is
unambiguous.

For reference, this is a near minimal workflow using comfy_mtb that
replicates the issue

[cyclic.json](https://github.com/user-attachments/files/22412187/cyclic.json)

Special thanks to @melMass for assistance discovering this issue.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-5637-Fix-cyclic-prototype-errors-with-subgraphNodes-2726d73d365081fea356f5197e4c2b42)
by [Unito](https://www.unito.io)

scripts/collect-i18n-general.ts

@@ -2,6 +2,7 @@ import * as fs from 'fs'
 
 import { comfyPageFixture as test } from '../browser_tests/fixtures/ComfyPage'
 import { CORE_MENU_COMMANDS } from '../src/constants/coreMenuCommands'
+import { DESKTOP_DIALOGS } from '../src/constants/desktopDialogs'
 import { SERVER_CONFIG_ITEMS } from '../src/constants/serverConfig'
 import type { FormItem, SettingParams } from '../src/platform/settings/types'
 import type { ComfyCommandImpl } from '../src/stores/commandStore'
@@ -131,6 +132,23 @@ test('collect-i18n-general', async ({ comfyPage }) => {
     ])
   )
 
+  // Desktop Dialogs
+  const allDesktopDialogsLocale = Object.fromEntries(
+    Object.values(DESKTOP_DIALOGS).map((dialog) => [
+      normalizeI18nKey(dialog.id),
+      {
+        title: dialog.title,
+        message: dialog.message,
+        buttons: Object.fromEntries(
+          dialog.buttons.map((button) => [
+            normalizeI18nKey(button.label),
+            button.label
+          ])
+        )
+      }
+    ])
+  )
+
   fs.writeFileSync(
     localePath,
     JSON.stringify(
@@ -144,7 +162,8 @@ test('collect-i18n-general', async ({ comfyPage }) => {
           ...allSettingCategoriesLocale
         },
         serverConfigItems: allServerConfigsLocale,
-        serverConfigCategories: allServerConfigCategoriesLocale
+        serverConfigCategories: allServerConfigCategoriesLocale,
+        desktopDialogs: allDesktopDialogsLocale
       },
       null,
       2

src/assets/css/style.css

@@ -66,6 +66,8 @@
   --color-charcoal-700: #202121;
   --color-charcoal-800: #171718;
 
+  --color-neutral-550: #636363;
+
   --color-stone-100: #444444;
   --color-stone-200: #828282;
   --color-stone-300: #bbbbbb;
@@ -103,6 +105,10 @@
   --color-danger-100: #c02323;
   --color-danger-200: #d62952;
 
+  --color-coral-red-600: #973a40;
+  --color-coral-red-500: #c53f49;
+  --color-coral-red-400: #dd424e;
+
   --color-bypass: #6a246a;
   --color-error: #962a2a;
 

src/composables/useManagerState.ts

@@ -42,7 +42,12 @@ export function useManagerState() {
       )
 
       // Check command line args first (highest priority)
-      if (systemStats.value?.system?.argv?.includes('--disable-manager')) {
+      // --enable-manager flag enables the manager (opposite of old --disable-manager)
+      const hasEnableManager =
+        systemStats.value?.system?.argv?.includes('--enable-manager')
+
+      // If --enable-manager is NOT present, manager is disabled
+      if (!hasEnableManager) {
         return ManagerUIState.DISABLED
       }
 

src/constants/desktopDialogs.ts

@@ -0,0 +1,75 @@
+export interface DialogAction {
+  readonly label: string
+  readonly action: 'openUrl' | 'close' | 'cancel'
+  readonly url?: string
+  readonly severity?: 'danger' | 'primary' | 'secondary' | 'warn'
+  readonly returnValue: string
+}
+
+interface DesktopDialog {
+  readonly title: string
+  readonly message: string
+  readonly buttons: DialogAction[]
+}
+
+export const DESKTOP_DIALOGS = {
+  /** Shown when a corrupt venv is detected. */
+  reinstallVenv: {
+    title: 'Reinstall ComfyUI (Fresh Start)?',
+    message: `Sorry, we can't launch ComfyUI because some installed packages aren't compatible.
+
+Click Reinstall to restore ComfyUI and get back up and running.
+
+Please note: if you've added custom nodes, you'll need to reinstall them after this process.`,
+    buttons: [
+      {
+        label: 'Learn More',
+        action: 'openUrl',
+        url: 'https://docs.comfy.org',
+        returnValue: 'openDocs'
+      },
+      {
+        label: 'Reinstall',
+        action: 'close',
+        severity: 'danger',
+        returnValue: 'resetVenv'
+      }
+    ]
+  },
+  /** A dialog that is shown when an invalid dialog ID is provided. */
+  invalidDialog: {
+    title: 'Invalid Dialog',
+    message: `Invalid dialog ID was provided.`,
+    buttons: [
+      {
+        label: 'Close',
+        action: 'cancel',
+        returnValue: 'cancel'
+      }
+    ]
+  }
+} as const satisfies { [K: string]: DesktopDialog }
+
+/** The ID of a desktop dialog. */
+type DesktopDialogId = keyof typeof DESKTOP_DIALOGS
+
+/**
+ * Checks if {@link id} is a valid dialog ID.
+ * @param id The string to check
+ * @returns `true` if the ID is a valid dialog ID, otherwise `false`
+ */
+function isDialogId(id: unknown): id is DesktopDialogId {
+  return typeof id === 'string' && id in DESKTOP_DIALOGS
+}
+
+/**
+ * Gets the dialog with the given ID.
+ * @param dialogId The ID of the dialog to get
+ * @returns The dialog with the given ID
+ */
+export function getDialog(
+  dialogId: string | string[]
+): DesktopDialog & { id: DesktopDialogId } {
+  const id = isDialogId(dialogId) ? dialogId : 'invalidDialog'
+  return { id, ...structuredClone(DESKTOP_DIALOGS[id]) }
+}

src/lib/litegraph/src/subgraph/SubgraphNode.ts

@@ -4,11 +4,14 @@ import { LGraphCanvas } from '@/lib/litegraph/src/LGraphCanvas'
 import { LGraphNode } from '@/lib/litegraph/src/LGraphNode'
 import { LLink, type ResolvedConnection } from '@/lib/litegraph/src/LLink'
 import { RecursionError } from '@/lib/litegraph/src/infrastructure/RecursionError'
-import type { ISubgraphInput } from '@/lib/litegraph/src/interfaces'
-import {
-  type INodeInputSlot,
-  type ISlotType,
-  type NodeId
+import type {
+  ISubgraphInput,
+  IWidgetLocator
+} from '@/lib/litegraph/src/interfaces'
+import type {
+  INodeInputSlot,
+  ISlotType,
+  NodeId
 } from '@/lib/litegraph/src/litegraph'
 import { NodeInputSlot } from '@/lib/litegraph/src/node/NodeInputSlot'
 import { NodeOutputSlot } from '@/lib/litegraph/src/node/NodeOutputSlot'
@@ -78,9 +81,10 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph {
         const existingInput = this.inputs.find((i) => i.name == name)
         if (existingInput) {
           const linkId = subgraphInput.linkIds[0]
-          const { inputNode } = subgraph.links[linkId].resolve(subgraph)
+          const { inputNode, input } = subgraph.links[linkId].resolve(subgraph)
           const widget = inputNode?.widgets?.find?.((w) => w.name == name)
-          if (widget) this.#setWidget(subgraphInput, existingInput, widget)
+          if (widget)
+            this.#setWidget(subgraphInput, existingInput, widget, input?.widget)
           return
         }
         const input = this.addInput(name, type)
@@ -185,13 +189,14 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph {
 
     subgraphInput.events.addEventListener(
       'input-connected',
-      () => {
+      (e) => {
         if (input._widget) return
 
         const widget = subgraphInput._widget
         if (!widget) return
 
-        this.#setWidget(subgraphInput, input, widget)
+        const widgetLocator = e.detail.input.widget
+        this.#setWidget(subgraphInput, input, widget, widgetLocator)
       },
       { signal }
     )
@@ -301,7 +306,7 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph {
         const widget = resolved.inputNode.getWidgetFromSlot(resolved.input)
         if (!widget) continue
 
-        this.#setWidget(subgraphInput, input, widget)
+        this.#setWidget(subgraphInput, input, widget, resolved.input.widget)
         break
       }
     }
@@ -310,11 +315,13 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph {
   #setWidget(
     subgraphInput: Readonly<SubgraphInput>,
     input: INodeInputSlot,
-    widget: Readonly<IBaseWidget>
+    widget: Readonly<IBaseWidget>,
+    inputWidget: IWidgetLocator | undefined
   ) {
     // Use the first matching widget
-    const targetWidget = toConcreteWidget(widget, this)
-    const promotedWidget = targetWidget.createCopyForNode(this)
+    const promotedWidget = toConcreteWidget(widget, this).createCopyForNode(
+      this
+    )
 
     Object.assign(promotedWidget, {
       get name() {
@@ -372,11 +379,9 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph {
     // NOTE: This code creates linked chains of prototypes for passing across
     // multiple levels of subgraphs. As part of this, it intentionally avoids
     // creating new objects. Have care when making changes.
-    const backingInput =
-      targetWidget.node.findInputSlot(widget.name, true)?.widget ?? {}
     input.widget ??= { name: subgraphInput.name }
     input.widget.name = subgraphInput.name
-    Object.setPrototypeOf(input.widget, backingInput)
+    if (inputWidget) Object.setPrototypeOf(input.widget, inputWidget)
 
     input._widget = promotedWidget
   }

src/router.ts

@@ -116,6 +116,12 @@ const router = createRouter({
           name: 'DesktopUpdateView',
           component: () => import('@/views/DesktopUpdateView.vue'),
           beforeEnter: guardElectronAccess
+        },
+        {
+          path: 'desktop-dialog/:dialogId',
+          name: 'DesktopDialogView',
+          component: () => import('@/views/DesktopDialogView.vue'),
+          beforeEnter: guardElectronAccess
         }
       ]
     }

src/views/DesktopDialogView.vue

@@ -0,0 +1,70 @@
+<template>
+  <div class="w-full h-full flex flex-col rounded-lg p-6 justify-between">
+    <h1 class="font-inter font-semibold text-xl m-0 italic">
+      {{ t(`desktopDialogs.${id}.title`, title) }}
+    </h1>
+    <p class="whitespace-pre-wrap">
+      {{ t(`desktopDialogs.${id}.message`, message) }}
+    </p>
+    <div class="flex w-full gap-2">
+      <Button
+        v-for="button in buttons"
+        :key="button.label"
+        class="rounded-lg first:mr-auto"
+        :label="
+          t(
+            `desktopDialogs.${id}.buttons.${normalizeI18nKey(button.label)}`,
+            button.label
+          )
+        "
+        :severity="button.severity ?? 'secondary'"
+        @click="handleButtonClick(button)"
+      />
+    </div>
+  </div>
+</template>
+
+<script setup lang="ts">
+import Button from 'primevue/button'
+import { useRoute } from 'vue-router'
+
+import { type DialogAction, getDialog } from '@/constants/desktopDialogs'
+import { t } from '@/i18n'
+import { electronAPI } from '@/utils/envUtil'
+import { normalizeI18nKey } from '@/utils/formatUtil'
+
+const route = useRoute()
+const { id, title, message, buttons } = getDialog(route.params.dialogId)
+
+const handleButtonClick = async (button: DialogAction) => {
+  await electronAPI().Dialog.clickButton(button.returnValue)
+}
+</script>
+
+<style scoped>
+@reference '../assets/css/style.css';
+
+.p-button-secondary {
+  @apply text-white border-none bg-neutral-600;
+}
+
+.p-button-secondary:hover {
+  @apply bg-neutral-550;
+}
+
+.p-button-secondary:active {
+  @apply bg-neutral-500;
+}
+
+.p-button-danger {
+  @apply bg-coral-red-600;
+}
+
+.p-button-danger:hover {
+  @apply bg-coral-red-500;
+}
+
+.p-button-danger:active {
+  @apply bg-coral-red-400;
+}
+</style>

tests-ui/tests/composables/useManagerState.test.ts

@@ -56,10 +56,10 @@ describe('useManagerState', () => {
   })
 
   describe('managerUIState property', () => {
-    it('should return DISABLED state when --disable-manager is present', () => {
+    it('should return DISABLED state when --enable-manager is NOT present', () => {
       vi.mocked(useSystemStatsStore).mockReturnValue({
         systemStats: ref({
-          system: { argv: ['python', 'main.py', '--disable-manager'] }
+          system: { argv: ['python', 'main.py'] } // No --enable-manager flag
         }),
         isInitialized: ref(true)
       } as any)
@@ -76,7 +76,14 @@ describe('useManagerState', () => {
     it('should return LEGACY_UI state when --enable-manager-legacy-ui is present', () => {
       vi.mocked(useSystemStatsStore).mockReturnValue({
         systemStats: ref({
-          system: { argv: ['python', 'main.py', '--enable-manager-legacy-ui'] }
+          system: {
+            argv: [
+              'python',
+              'main.py',
+              '--enable-manager',
+              '--enable-manager-legacy-ui'
+            ]
+          } // Both flags needed
         }),
         isInitialized: ref(true)
       } as any)
@@ -92,7 +99,9 @@ describe('useManagerState', () => {
 
     it('should return NEW_UI state when client and server both support v4', () => {
       vi.mocked(useSystemStatsStore).mockReturnValue({
-        systemStats: ref({ system: { argv: ['python', 'main.py'] } }),
+        systemStats: ref({
+          system: { argv: ['python', 'main.py', '--enable-manager'] }
+        }), // Need --enable-manager
         isInitialized: ref(true)
       } as any)
       vi.mocked(api.getClientFeatureFlags).mockReturnValue({
@@ -114,7 +123,9 @@ describe('useManagerState', () => {
 
     it('should return LEGACY_UI state when server supports v4 but client does not', () => {
       vi.mocked(useSystemStatsStore).mockReturnValue({
-        systemStats: ref({ system: { argv: ['python', 'main.py'] } }),
+        systemStats: ref({
+          system: { argv: ['python', 'main.py', '--enable-manager'] }
+        }), // Need --enable-manager
         isInitialized: ref(true)
       } as any)
       vi.mocked(api.getClientFeatureFlags).mockReturnValue({
@@ -136,7 +147,9 @@ describe('useManagerState', () => {
 
     it('should return LEGACY_UI state when legacy manager extension exists', () => {
       vi.mocked(useSystemStatsStore).mockReturnValue({
-        systemStats: ref({ system: { argv: ['python', 'main.py'] } }),
+        systemStats: ref({
+          system: { argv: ['python', 'main.py', '--enable-manager'] }
+        }), // Need --enable-manager
         isInitialized: ref(true)
       } as any)
       vi.mocked(api.getClientFeatureFlags).mockReturnValue({})
@@ -155,7 +168,9 @@ describe('useManagerState', () => {
 
     it('should return NEW_UI state when server feature flags are undefined', () => {
       vi.mocked(useSystemStatsStore).mockReturnValue({
-        systemStats: ref({ system: { argv: ['python', 'main.py'] } }),
+        systemStats: ref({
+          system: { argv: ['python', 'main.py', '--enable-manager'] }
+        }), // Need --enable-manager
         isInitialized: ref(true)
       } as any)
       vi.mocked(api.getClientFeatureFlags).mockReturnValue({})
@@ -175,7 +190,9 @@ describe('useManagerState', () => {
 
     it('should return LEGACY_UI state when server does not support v4', () => {
       vi.mocked(useSystemStatsStore).mockReturnValue({
-        systemStats: ref({ system: { argv: ['python', 'main.py'] } }),
+        systemStats: ref({
+          system: { argv: ['python', 'main.py', '--enable-manager'] }
+        }), // Need --enable-manager
         isInitialized: ref(true)
       } as any)
       vi.mocked(api.getClientFeatureFlags).mockReturnValue({})
@@ -212,14 +229,17 @@ describe('useManagerState', () => {
 
       const managerState = useManagerState()
 
-      expect(managerState.managerUIState.value).toBe(ManagerUIState.NEW_UI)
+      // When systemStats is null, we can't check for --enable-manager flag, so manager is disabled
+      expect(managerState.managerUIState.value).toBe(ManagerUIState.DISABLED)
     })
   })
 
   describe('helper properties', () => {
     it('isManagerEnabled should return true when state is not DISABLED', () => {
       vi.mocked(useSystemStatsStore).mockReturnValue({
-        systemStats: ref({ system: { argv: ['python', 'main.py'] } }),
+        systemStats: ref({
+          system: { argv: ['python', 'main.py', '--enable-manager'] }
+        }), // Need --enable-manager
         isInitialized: ref(true)
       } as any)
       vi.mocked(api.getClientFeatureFlags).mockReturnValue({
@@ -237,7 +257,7 @@ describe('useManagerState', () => {
     it('isManagerEnabled should return false when state is DISABLED', () => {
       vi.mocked(useSystemStatsStore).mockReturnValue({
         systemStats: ref({
-          system: { argv: ['python', 'main.py', '--disable-manager'] }
+          system: { argv: ['python', 'main.py'] } // No --enable-manager flag means disabled
         }),
         isInitialized: ref(true)
       } as any)
@@ -252,7 +272,9 @@ describe('useManagerState', () => {
 
     it('isNewManagerUI should return true when state is NEW_UI', () => {
       vi.mocked(useSystemStatsStore).mockReturnValue({
-        systemStats: ref({ system: { argv: ['python', 'main.py'] } }),
+        systemStats: ref({
+          system: { argv: ['python', 'main.py', '--enable-manager'] }
+        }), // Need --enable-manager
         isInitialized: ref(true)
       } as any)
       vi.mocked(api.getClientFeatureFlags).mockReturnValue({
@@ -270,7 +292,14 @@ describe('useManagerState', () => {
     it('isLegacyManagerUI should return true when state is LEGACY_UI', () => {
       vi.mocked(useSystemStatsStore).mockReturnValue({
         systemStats: ref({
-          system: { argv: ['python', 'main.py', '--enable-manager-legacy-ui'] }
+          system: {
+            argv: [
+              'python',
+              'main.py',
+              '--enable-manager',
+              '--enable-manager-legacy-ui'
+            ]
+          } // Both flags needed
         }),
         isInitialized: ref(true)
       } as any)
@@ -285,7 +314,9 @@ describe('useManagerState', () => {
 
     it('shouldShowInstallButton should return true only for NEW_UI', () => {
       vi.mocked(useSystemStatsStore).mockReturnValue({
-        systemStats: ref({ system: { argv: ['python', 'main.py'] } }),
+        systemStats: ref({
+          system: { argv: ['python', 'main.py', '--enable-manager'] }
+        }), // Need --enable-manager
         isInitialized: ref(true)
       } as any)
       vi.mocked(api.getClientFeatureFlags).mockReturnValue({
@@ -302,7 +333,9 @@ describe('useManagerState', () => {
 
     it('shouldShowManagerButtons should return true when not DISABLED', () => {
       vi.mocked(useSystemStatsStore).mockReturnValue({
-        systemStats: ref({ system: { argv: ['python', 'main.py'] } }),
+        systemStats: ref({
+          system: { argv: ['python', 'main.py', '--enable-manager'] }
+        }), // Need --enable-manager
         isInitialized: ref(true)
       } as any)
       vi.mocked(api.getClientFeatureFlags).mockReturnValue({