[Manager] Fix package version matching in conflict detection (#4530)

Co-authored-by: Jin Yi <jin12cc@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
comfy-jinyi
2025-07-25 14:10:55 +09:00
committed by Jin Yi
parent 9a8bd34170
commit e581925903
10 changed files with 143 additions and 113 deletions

View File

@@ -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
}
}

View File

@@ -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],

View File

@@ -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' }
)
</script>

View File

@@ -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

View File

@@ -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)
})
</script>

View File

@@ -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()
}

View File

@@ -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<string | null>(null)
@@ -178,15 +189,17 @@ export function useConflictDetection() {
*/
async function fetchPackageRequirements(): Promise<NodePackRequirements[]> {
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<string, ConflictDetectionResult>()
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
})
}
})

View File

@@ -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',

View File

@@ -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

View File

@@ -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()
})