diff --git a/src/components/dialog/content/manager/PackVersionSelectorPopover.vue b/src/components/dialog/content/manager/PackVersionSelectorPopover.vue index 478fa3f79..488e994ae 100644 --- a/src/components/dialog/content/manager/PackVersionSelectorPopover.vue +++ b/src/components/dialog/content/manager/PackVersionSelectorPopover.vue @@ -252,20 +252,25 @@ const getVersionData = (version: string) => { nodePack.supported_comfyui_frontend_version, supported_python_version: (latestVersionData && 'supported_python_version' in latestVersionData - ? latestVersionData.supported_python_version as string | undefined + ? (latestVersionData.supported_python_version as string | undefined) : undefined) ?? ('supported_python_version' in nodePack - ? nodePack.supported_python_version as string | undefined + ? (nodePack.supported_python_version as string | undefined) : undefined), is_banned: (latestVersionData && 'is_banned' in latestVersionData - ? latestVersionData.is_banned as boolean | undefined - : undefined) ?? ('is_banned' in nodePack ? nodePack.is_banned as boolean | undefined : false), + ? (latestVersionData.is_banned as boolean | undefined) + : undefined) ?? + ('is_banned' in nodePack + ? (nodePack.is_banned as boolean | undefined) + : false), has_registry_data: (latestVersionData && 'has_registry_data' in latestVersionData - ? latestVersionData.has_registry_data as boolean | undefined + ? (latestVersionData.has_registry_data as boolean | undefined) : undefined) ?? - ('has_registry_data' in nodePack ? nodePack.has_registry_data as boolean | undefined : false) + ('has_registry_data' in nodePack + ? (nodePack.has_registry_data as boolean | undefined) + : false) } } @@ -281,7 +286,7 @@ const getVersionData = (version: string) => { nodePack.supported_comfyui_frontend_version, // Use latest known requirement supported_python_version: 'supported_python_version' in nodePack - ? nodePack.supported_python_version as string | undefined + ? (nodePack.supported_python_version as string | undefined) : undefined, is_banned: false, // Nightly versions from repositories are typically not banned has_registry_data: false // Nightly doesn't come from registry @@ -299,12 +304,15 @@ const getVersionData = (version: string) => { versionData.supported_comfyui_frontend_version, supported_python_version: 'supported_python_version' in versionData - ? versionData.supported_python_version as string | undefined + ? (versionData.supported_python_version as string | undefined) : undefined, - is_banned: 'is_banned' in versionData ? versionData.is_banned as boolean | undefined : false, + is_banned: + 'is_banned' in versionData + ? (versionData.is_banned as boolean | undefined) + : false, has_registry_data: 'has_registry_data' in versionData - ? versionData.has_registry_data as boolean | undefined + ? (versionData.has_registry_data as boolean | undefined) : false } } @@ -318,11 +326,16 @@ const getVersionData = (version: string) => { nodePack.supported_comfyui_frontend_version, supported_python_version: 'supported_python_version' in nodePack - ? nodePack.supported_python_version as string | undefined + ? (nodePack.supported_python_version as string | undefined) : undefined, - is_banned: 'is_banned' in nodePack ? nodePack.is_banned as boolean | undefined : false, + is_banned: + 'is_banned' in nodePack + ? (nodePack.is_banned as boolean | undefined) + : false, has_registry_data: - 'has_registry_data' in nodePack ? nodePack.has_registry_data as boolean | undefined : false + 'has_registry_data' in nodePack + ? (nodePack.has_registry_data as boolean | undefined) + : false } } diff --git a/src/components/dialog/content/manager/button/PackEnableToggle.vue b/src/components/dialog/content/manager/button/PackEnableToggle.vue index ebcd08b5b..6cefe2639 100644 --- a/src/components/dialog/content/manager/button/PackEnableToggle.vue +++ b/src/components/dialog/content/manager/button/PackEnableToggle.vue @@ -42,7 +42,7 @@ const { nodePack, hasConflict } = defineProps<{ const { t } = useI18n() const { isPackEnabled, enablePack, disablePack } = useComfyManagerStore() -const conflictStore = useConflictDetectionStore() +const { getConflictsForPackageByID } = useConflictDetectionStore() const { showNodeConflictDialog } = useDialogService() const { acknowledgeConflict, isConflictAcknowledged } = useConflictAcknowledgment() @@ -87,7 +87,7 @@ const handleToggle = async (enable: boolean, skipConflictCheck = false) => { // Check for conflicts when enabling if (enable && hasConflict && !skipConflictCheck) { - const conflicts = conflictStore.getConflictsForPackage(nodePack.id || '') + const conflicts = getConflictsForPackageByID(nodePack.id || '') if (conflicts) { // Check if conflicts have been acknowledged const hasUnacknowledgedConflicts = conflicts.conflicts.some( @@ -141,7 +141,7 @@ const onToggle = debounce( // Show conflict modal when warning icon is clicked const showConflictModal = () => { - const conflicts = conflictStore.getConflictsForPackage(nodePack.id || '') + const conflicts = getConflictsForPackageByID(nodePack.id || '') if (conflicts) { showNodeConflictDialog({ conflictedPackages: [conflicts], diff --git a/src/components/dialog/content/manager/infoPanel/InfoTabs.vue b/src/components/dialog/content/manager/infoPanel/InfoTabs.vue index c57744517..f08b4f4fe 100644 --- a/src/components/dialog/content/manager/infoPanel/InfoTabs.vue +++ b/src/components/dialog/content/manager/infoPanel/InfoTabs.vue @@ -62,12 +62,15 @@ const nodeNames = computed(() => { const activeTab = ref('description') // Watch for compatibility issues and automatically switch to warning tab -watchEffect(() => { - if (hasCompatibilityIssues) { - activeTab.value = 'warning' - } else if (activeTab.value === 'warning') { - // If currently on warning tab but no issues, switch to description - activeTab.value = 'description' - } -}, { flush: 'post' }) +watchEffect( + () => { + if (hasCompatibilityIssues) { + activeTab.value = 'warning' + } else if (activeTab.value === 'warning') { + // If currently on warning tab but no issues, switch to description + activeTab.value = 'description' + } + }, + { flush: 'post' } +) diff --git a/src/components/dialog/content/manager/packCard/PackCardFooter.vue b/src/components/dialog/content/manager/packCard/PackCardFooter.vue index e2df88a79..4d8826dd0 100644 --- a/src/components/dialog/content/manager/packCard/PackCardFooter.vue +++ b/src/components/dialog/content/manager/packCard/PackCardFooter.vue @@ -44,20 +44,16 @@ const formattedDownloads = computed(() => nodePack.downloads ? n(nodePack.downloads) : '' ) -const conflictStore = useConflictDetectionStore() +const { getConflictsForPackageByID } = useConflictDetectionStore() const { checkVersionCompatibility } = useConflictDetection() -// TODO: Package version mismatch issue - Package IDs include version suffixes (@1_0_3) -// but UI searches without version. This causes conflict detection failures. -// Once getConflictsForPackage is improved to handle version matching properly, -// all the complex fallback logic below can be removed. const hasConflict = computed(() => { if (!nodePack.id) return false // For installed packages, check conflicts from store if (isInstalled.value) { // Try exact match first - let conflicts = conflictStore.getConflictsForPackage(nodePack.id) + let conflicts = getConflictsForPackageByID(nodePack.id) if (conflicts) return true return false diff --git a/src/components/dialog/content/manager/packCard/PackCardHeader.vue b/src/components/dialog/content/manager/packCard/PackCardHeader.vue index f30484387..6e41e0227 100644 --- a/src/components/dialog/content/manager/packCard/PackCardHeader.vue +++ b/src/components/dialog/content/manager/packCard/PackCardHeader.vue @@ -46,13 +46,13 @@ const { nodePack } = defineProps<{ }>() const { isPackInstalled } = useComfyManagerStore() -const conflictStore = useConflictDetectionStore() +const { getConflictsForPackageByID } = useConflictDetectionStore() const isInstalled = computed(() => isPackInstalled(nodePack?.id)) const packageConflicts = computed(() => { if (!nodePack.id || !isInstalled.value) return null // For installed packages, check conflicts from store - return conflictStore.getConflictsForPackage(nodePack.id) + return getConflictsForPackageByID(nodePack.id) }) diff --git a/src/composables/nodePack/useInstalledPacks.ts b/src/composables/nodePack/useInstalledPacks.ts index fd671d33b..60e6f5fdd 100644 --- a/src/composables/nodePack/useInstalledPacks.ts +++ b/src/composables/nodePack/useInstalledPacks.ts @@ -20,7 +20,10 @@ export const useInstalledPacks = (options: UseNodePacksOptions = {}) => { packs.filter((pack) => comfyManagerStore.isPackInstalled(pack.id)) const startFetchInstalled = async () => { - await comfyManagerStore.refreshInstalledList() + // Only refresh if store doesn't have data yet + if (comfyManagerStore.installedPacksIds.size === 0) { + await comfyManagerStore.refreshInstalledList() + } await startFetch() } diff --git a/src/composables/useConflictDetection.ts b/src/composables/useConflictDetection.ts index 2dd13e12c..3c7ad1022 100644 --- a/src/composables/useConflictDetection.ts +++ b/src/composables/useConflictDetection.ts @@ -1,10 +1,12 @@ import { uniqBy } from 'lodash' import { computed, getCurrentInstance, onUnmounted, readonly, ref } from 'vue' +import { useInstalledPacks } from '@/composables/nodePack/useInstalledPacks' import { useConflictAcknowledgment } from '@/composables/useConflictAcknowledgment' import config from '@/config' import { useComfyManagerService } from '@/services/comfyManagerService' import { useComfyRegistryService } from '@/services/comfyRegistryService' +import { useComfyManagerStore } from '@/stores/comfyManagerStore' import { useConflictDetectionStore } from '@/stores/conflictDetectionStore' import { useSystemStatsStore } from '@/stores/systemStatsStore' import type { SystemStats } from '@/types' @@ -20,7 +22,6 @@ import type { SupportedOS, SystemEnvironment } from '@/types/conflictDetectionTypes' -import type { components as ManagerComponents } from '@/types/generatedManagerTypes' import { cleanVersion, satisfiesVersion, @@ -32,6 +33,16 @@ import { * Error-resilient and asynchronous to avoid affecting other components. */ export function useConflictDetection() { + // Store references + const managerStore = useComfyManagerStore() + + // Use installed packs composable instead of direct API calls + const { + startFetchInstalled, + installedPacks, + isReady: installedPacksReady + } = useInstalledPacks() + // State management const isDetecting = ref(false) const lastDetectionTime = ref(null) @@ -178,15 +189,17 @@ export function useConflictDetection() { */ async function fetchPackageRequirements(): Promise { try { - // Step 1: Get locally installed packages - const comfyManagerService = useComfyManagerService() - const installedNodes: - | ManagerComponents['schemas']['InstalledPacksResponse'] - | null = await comfyManagerService.listInstalledPacks() + // Step 1: Use installed packs composable instead of direct API calls + // This follows Christian's architecture suggestion to use computed properties + await startFetchInstalled() // Ensure data is loaded - if (!installedNodes) { + if ( + !installedPacksReady.value || + !installedPacks.value || + installedPacks.value.length === 0 + ) { console.warn( - '[ConflictDetection] Unable to fetch installed package information' + '[ConflictDetection] No installed packages available from useInstalledPacks' ) return [] } @@ -200,34 +213,34 @@ export function useConflictDetection() { // Step 4: Fetch version-specific data in chunks to avoid overwhelming the Registry API // - Each chunk processes up to 30 packages concurrently // - Results are stored in versionDataMap for later use - const entries = Object.entries(installedNodes) - const chunkSize = 30 // 청크 크기 + // - Use installedPacks from useInstalledPacks composable + const chunkSize = 30 const versionDataMap = new Map< string, components['schemas']['NodeVersion'] >() - for (let i = 0; i < entries.length; i += chunkSize) { - const chunk = entries.slice(i, i + chunkSize) + for (let i = 0; i < installedPacks.value.length; i += chunkSize) { + const chunk = installedPacks.value.slice(i, i + chunkSize) - const fetchTasks = chunk.map(async ([packageName, nodeInfo]) => { - const typedNodeInfo: ManagerComponents['schemas']['ManagerPackInstalled'] = - nodeInfo - const version = typedNodeInfo.ver || 'latest' + const fetchTasks = chunk.map(async (pack) => { + // Use pack.id which should be the normalized ID from Registry + const packageId = pack.id || '' + const version = pack.latest_version?.version || 'latest' try { const versionData = await registryService.getPackByVersion( - packageName, + packageId, version, abortController.value?.signal ) if (versionData) { - versionDataMap.set(packageName, versionData) + versionDataMap.set(packageId, versionData) } } catch (error) { console.warn( - `[ConflictDetection] Failed to fetch version data for ${packageName}@${version}:`, + `[ConflictDetection] Failed to fetch version data for ${packageId}@${version}:`, error ) } @@ -237,21 +250,24 @@ export function useConflictDetection() { } // Step 5: Combine local installation data with Registry version data + // Use installedPacks from useInstalledPacks composable const requirements: NodePackRequirements[] = [] - for (const [packageName, nodeInfo] of Object.entries(installedNodes)) { - const typedNodeInfo: ManagerComponents['schemas']['ManagerPackInstalled'] = - nodeInfo - const versionData = versionDataMap.get(packageName) + for (const pack of installedPacks.value) { + const packageId = pack.id || '' + const versionData = versionDataMap.get(packageId) + + // Check if package is enabled using store method + const isEnabled = managerStore.isPackEnabled(packageId) if (versionData) { // Combine local installation data with version-specific Registry data const requirement: NodePackRequirements = { // Basic package info - package_id: packageName, - package_name: packageName, // We don't need to fetch node info separately - installed_version: typedNodeInfo.ver || 'unknown', - is_enabled: typedNodeInfo.enabled, + package_id: packageId, + package_name: pack.name || packageId, + installed_version: pack.latest_version?.version || 'unknown', + is_enabled: isEnabled, // Version-specific compatibility data supported_comfyui_version: versionData.supported_comfyui_version, @@ -266,12 +282,11 @@ export function useConflictDetection() { registry_status: undefined, // Node status - not critical for conflict detection version_status: versionData.status, is_banned: - versionData.status === 'NodeVersionStatusBanned' || - !typedNodeInfo.enabled, + versionData.status === 'NodeVersionStatusBanned' || !isEnabled, ban_reason: versionData.status === 'NodeVersionStatusBanned' ? 'Version is banned in Registry' - : !typedNodeInfo.enabled + : !isEnabled ? 'Package is disabled locally' : undefined, @@ -283,19 +298,17 @@ export function useConflictDetection() { requirements.push(requirement) } else { console.warn( - `[ConflictDetection] No Registry data found for ${packageName}, using fallback` + `[ConflictDetection] No Registry data found for ${packageId}, using fallback` ) // Create fallback requirement without Registry data const fallbackRequirement: NodePackRequirements = { - package_id: packageName, - package_name: packageName, - installed_version: typedNodeInfo.ver || 'unknown', - is_enabled: typedNodeInfo.enabled, - is_banned: !typedNodeInfo.enabled, - ban_reason: !typedNodeInfo.enabled - ? 'Package is disabled locally' - : undefined, + package_id: packageId, + package_name: pack.name || packageId, + installed_version: pack.latest_version?.version || 'unknown', + is_enabled: isEnabled, + is_banned: !isEnabled, + ban_reason: !isEnabled ? 'Package is disabled locally' : undefined, registry_fetch_time: new Date().toISOString(), has_registry_data: false } @@ -415,13 +428,15 @@ export function useConflictDetection() { try { const comfyManagerService = useComfyManagerService() - // Get installed packages first - const installedPacks = await comfyManagerService.listInstalledPacks( - abortController.value?.signal - ) - - if (!installedPacks) { - console.warn('[ConflictDetection] No installed packages found') + // Use installed packs from useInstalledPacks composable + if ( + !installedPacksReady.value || + !installedPacks.value || + installedPacks.value.length === 0 + ) { + console.warn( + '[ConflictDetection] No installed packages available from useInstalledPacks' + ) return {} } @@ -429,27 +444,27 @@ export function useConflictDetection() { // Check each installed package for import failures // Process in smaller batches to avoid overwhelming the API - const packageNames = Object.keys(installedPacks) + const packageIds = installedPacks.value.map((pack) => pack.id) const batchSize = 10 - for (let i = 0; i < packageNames.length; i += batchSize) { - const batch = packageNames.slice(i, i + batchSize) + for (let i = 0; i < packageIds.length; i += batchSize) { + const batch = packageIds.slice(i, i + batchSize) const batchResults = await Promise.allSettled( - batch.map(async (packageName) => { + batch.map(async (packageId) => { try { - // Try to get import failure info for this package + // Try to get import failure info for this package using normalized ID const failInfo = await comfyManagerService.getImportFailInfo( - { cnr_id: packageName }, + { cnr_id: packageId }, abortController.value?.signal ) if (failInfo) { console.log( - `[ConflictDetection] Import failure found for ${packageName}:`, + `[ConflictDetection] Import failure found for ${packageId}:`, failInfo ) - return { packageName, failInfo } + return { packageId, failInfo } } } catch (error) { // If API returns 400, it means no import failure info available @@ -462,7 +477,7 @@ export function useConflictDetection() { } console.warn( - `[ConflictDetection] Failed to check import failure for ${packageName}:`, + `[ConflictDetection] Failed to check import failure for ${packageId}:`, error ) } @@ -473,8 +488,8 @@ export function useConflictDetection() { // Process batch results batchResults.forEach((result) => { if (result.status === 'fulfilled' && result.value) { - const { packageName, failInfo } = result.value - importFailures[packageName] = failInfo + const { packageId, failInfo } = result.value + importFailures[packageId || ''] = failInfo } }) } @@ -504,7 +519,7 @@ export function useConflictDetection() { } // Process import failures - for (const [packageName, failureInfo] of Object.entries(importFailInfo)) { + for (const [packageId, failureInfo] of Object.entries(importFailInfo)) { if (failureInfo && typeof failureInfo === 'object') { // Extract error information from Manager API response const errorMsg = failureInfo.msg || 'Unknown import error' @@ -522,15 +537,15 @@ export function useConflictDetection() { ] results.push({ - package_id: packageName, - package_name: packageName, + package_id: packageId, + package_name: packageId, has_conflict: true, conflicts, is_compatible: false }) console.warn( - `[ConflictDetection] Python import failure detected for ${packageName}:`, + `[ConflictDetection] Python import failure detected for ${packageId}:`, { path: modulePath, error: errorMsg, @@ -949,11 +964,14 @@ function mergeConflictsByPackageName( const mergedMap = new Map() conflicts.forEach((conflict) => { - const packageName = conflict.package_name + // Normalize package name by removing version suffix (@1_0_3) for consistent merging + const normalizedPackageName = conflict.package_name.includes('@') + ? conflict.package_name.substring(0, conflict.package_name.indexOf('@')) + : conflict.package_name - if (mergedMap.has(packageName)) { + if (mergedMap.has(normalizedPackageName)) { // Package already exists, merge conflicts - const existing = mergedMap.get(packageName)! + const existing = mergedMap.get(normalizedPackageName)! // Combine all conflicts, avoiding duplicates using lodash uniqBy for O(n) performance const allConflicts = [...existing.conflicts, ...conflict.conflicts] @@ -963,16 +981,20 @@ function mergeConflictsByPackageName( `${conflict.type}|${conflict.current_value}|${conflict.required_value}` ) - // Update the existing entry - mergedMap.set(packageName, { + // Update the existing entry with normalized package name + mergedMap.set(normalizedPackageName, { ...existing, + package_name: normalizedPackageName, conflicts: uniqueConflicts, has_conflict: uniqueConflicts.length > 0, is_compatible: uniqueConflicts.length === 0 }) } else { - // New package, add as-is - mergedMap.set(packageName, conflict) + // New package, add with normalized package name + mergedMap.set(normalizedPackageName, { + ...conflict, + package_name: normalizedPackageName + }) } }) diff --git a/src/constants/coreSettings.ts b/src/constants/coreSettings.ts index b53933bd5..f79d779b7 100644 --- a/src/constants/coreSettings.ts +++ b/src/constants/coreSettings.ts @@ -14,13 +14,6 @@ import type { SettingParams } from '@/types/settingTypes' * when they are no longer needed. */ export const CORE_SETTINGS: SettingParams[] = [ - { - id: 'Comfy.Memory.AllowManualUnload', - name: 'Allow manual unload of models and execution cache via user command', - type: 'hidden', - defaultValue: true, - versionAdded: '1.18.0' - }, { id: 'Comfy.Validation.Workflows', name: 'Validate workflows', diff --git a/src/stores/conflictDetectionStore.ts b/src/stores/conflictDetectionStore.ts index ad56ec0ab..ce96f1a82 100644 --- a/src/stores/conflictDetectionStore.ts +++ b/src/stores/conflictDetectionStore.ts @@ -16,7 +16,7 @@ export const useConflictDetectionStore = defineStore( conflictedPackages.value.some((pkg) => pkg.has_conflict) ) - const getConflictsForPackage = computed( + const getConflictsForPackageByID = computed( () => (packageId: string) => conflictedPackages.value.find((pkg) => pkg.package_id === packageId) ) @@ -57,7 +57,7 @@ export const useConflictDetectionStore = defineStore( lastDetectionTime, // Getters hasConflicts, - getConflictsForPackage, + getConflictsForPackageByID, bannedPackages, securityPendingPackages, // Actions diff --git a/tests-ui/tests/stores/conflictDetectionStore.test.ts b/tests-ui/tests/stores/conflictDetectionStore.test.ts index 709bae183..8ea649608 100644 --- a/tests-ui/tests/stores/conflictDetectionStore.test.ts +++ b/tests-ui/tests/stores/conflictDetectionStore.test.ts @@ -83,12 +83,12 @@ describe('useConflictDetectionStore', () => { }) }) - describe('getConflictsForPackage', () => { + describe('getConflictsForPackageByID', () => { it('should find package by exact ID match', () => { const store = useConflictDetectionStore() store.setConflictedPackages(mockConflictedPackages) - const result = store.getConflictsForPackage('ComfyUI-Manager') + const result = store.getConflictsForPackageByID('ComfyUI-Manager') expect(result).toBeDefined() expect(result?.package_id).toBe('ComfyUI-Manager') @@ -99,7 +99,7 @@ describe('useConflictDetectionStore', () => { const store = useConflictDetectionStore() store.setConflictedPackages(mockConflictedPackages) - const result = store.getConflictsForPackage('non-existent-package') + const result = store.getConflictsForPackageByID('non-existent-package') expect(result).toBeUndefined() })