Skip to content

Commit e2de4b1

Browse files
viva-jinyiclaude
andauthored
Fix version detection for disabled packs (#5395)
* fix: normalize pack IDs to fix version detection for disabled packs When a pack is disabled, ComfyUI-Manager returns it with a version suffix (e.g., "ComfyUI-GGUF@1_1_4") while enabled packs don't have this suffix. This inconsistency caused disabled packs to incorrectly show as having updates available even when they were on the latest version. Changes: - Add normalizePackId utility to consistently remove version suffixes - Apply normalization in refreshInstalledList and WebSocket updates - Use the utility across conflict detection and node help modules - Ensure pack version info is preserved in the object's ver field This fixes the "Update Available" indicator incorrectly showing for disabled packs that are already on the latest version. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * feature: test code added * test: packUtils test code added * test: address PR review feedback for test improvements - Remove unnecessary .not.toThrow() assertion in useManagerQueue test - Add clarifying comments for version normalization test logic - Replace 'as any' with vi.mocked() for better type safety --------- Co-authored-by: Claude <[email protected]>
1 parent 0d3b155 commit e2de4b1

File tree

8 files changed

+450
-12
lines changed

8 files changed

+450
-12
lines changed

src/composables/useConflictDetection.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import type {
2222
NodePackRequirements,
2323
SystemEnvironment
2424
} from '@/types/conflictDetectionTypes'
25+
import { normalizePackId } from '@/utils/packUtils'
2526
import {
2627
cleanVersion,
2728
satisfiesVersion,
@@ -874,9 +875,7 @@ function mergeConflictsByPackageName(
874875

875876
conflicts.forEach((conflict) => {
876877
// Normalize package name by removing version suffix (@1_0_3) for consistent merging
877-
const normalizedPackageName = conflict.package_name.includes('@')
878-
? conflict.package_name.substring(0, conflict.package_name.indexOf('@'))
879-
: conflict.package_name
878+
const normalizedPackageName = normalizePackId(conflict.package_name)
880879

881880
if (mergedMap.has(normalizedPackageName)) {
882881
// Package already exists, merge conflicts

src/composables/useManagerQueue.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { Ref, computed, ref } from 'vue'
55
import { app } from '@/scripts/app'
66
import { useDialogService } from '@/services/dialogService'
77
import { components } from '@/types/generatedManagerTypes'
8+
import { normalizePackKeys } from '@/utils/packUtils'
89

910
type ManagerTaskHistory = Record<
1011
string,
@@ -98,7 +99,8 @@ export const useManagerQueue = (
9899
taskHistory.value = filterHistoryByClientId(state.history)
99100

100101
if (state.installed_packs) {
101-
installedPacks.value = state.installed_packs
102+
// Normalize pack keys to ensure consistent access
103+
installedPacks.value = normalizePackKeys(state.installed_packs)
102104
}
103105
updateProcessingState()
104106
}

src/stores/comfyManagerStore.ts

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import { useEventListener, whenever } from '@vueuse/core'
2-
import { mapKeys } from 'es-toolkit/compat'
32
import { defineStore } from 'pinia'
43
import { v4 as uuidv4 } from 'uuid'
54
import { ref, watch } from 'vue'
@@ -14,6 +13,7 @@ import { useComfyManagerService } from '@/services/comfyManagerService'
1413
import { useDialogService } from '@/services/dialogService'
1514
import { TaskLog } from '@/types/comfyManagerTypes'
1615
import { components } from '@/types/generatedManagerTypes'
16+
import { normalizePackKeys } from '@/utils/packUtils'
1717

1818
type InstallPackParams = components['schemas']['InstallPackParams']
1919
type InstalledPacksResponse = components['schemas']['InstalledPacksResponse']
@@ -185,12 +185,8 @@ export const useComfyManagerStore = defineStore('comfyManager', () => {
185185
const refreshInstalledList = async () => {
186186
const packs = await managerService.listInstalledPacks()
187187
if (packs) {
188-
// The keys are 'cleaned' by stripping the version suffix.
189-
// The pack object itself (the value) still contains the version info.
190-
const packsWithCleanedKeys = mapKeys(packs, (_value, key) => {
191-
return key.split('@')[0]
192-
})
193-
installedPacks.value = packsWithCleanedKeys
188+
// Normalize pack keys to ensure consistent access
189+
installedPacks.value = normalizePackKeys(packs)
194190
}
195191
isStale.value = false
196192
}

src/utils/nodeHelpUtil.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
import type { ComfyNodeDefImpl } from '@/stores/nodeDefStore'
22
import { NodeSourceType, getNodeSource } from '@/types/nodeSource'
3+
import { normalizePackId } from '@/utils/packUtils'
34

45
export function extractCustomNodeName(
56
pythonModule: string | undefined
67
): string | null {
78
const modules = pythonModule?.split('.') || []
89
if (modules.length >= 2 && modules[0] === 'custom_nodes') {
9-
return modules[1].split('@')[0]
10+
// Use normalizePackId to remove version suffix
11+
return normalizePackId(modules[1])
1012
}
1113
return null
1214
}

src/utils/packUtils.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import { mapKeys } from 'es-toolkit/compat'
2+
3+
/**
4+
* Normalizes a pack ID by removing the version suffix.
5+
*
6+
* ComfyUI-Manager returns pack IDs in different formats:
7+
* - Enabled packs: "packname" (without version)
8+
* - Disabled packs: "packname@1_0_3" (with version suffix)
9+
* - Latest versions from registry: "packname" (without version)
10+
*
11+
* Since the pack object itself contains the version info (ver field),
12+
* we normalize all pack IDs to just the base name for consistent access.
13+
* This ensures we can always find a pack by its base name (nodePack.id)
14+
* regardless of its enabled/disabled state.
15+
*
16+
* @param packId - The pack ID that may contain a version suffix
17+
* @returns The normalized pack ID without version suffix
18+
*/
19+
export function normalizePackId(packId: string): string {
20+
return packId.split('@')[0]
21+
}
22+
23+
/**
24+
* Normalizes all keys in a pack record by removing version suffixes.
25+
* This is used when receiving pack data from the server to ensure
26+
* consistent key format across the application.
27+
*
28+
* @param packs - Record of packs with potentially versioned keys
29+
* @returns Record with normalized keys
30+
*/
31+
export function normalizePackKeys<T>(
32+
packs: Record<string, T>
33+
): Record<string, T> {
34+
return mapKeys(packs, (_value, key) => normalizePackId(key))
35+
}

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

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,5 +161,62 @@ describe('useManagerQueue', () => {
161161
expect(taskHistory.value).toHaveProperty('task1')
162162
expect(taskHistory.value).not.toHaveProperty('task2')
163163
})
164+
165+
it('normalizes pack IDs when updating installed packs', () => {
166+
const queue = createManagerQueue()
167+
168+
const mockState = {
169+
history: {},
170+
running_queue: [],
171+
pending_queue: [],
172+
installed_packs: {
173+
'ComfyUI-GGUF@1_1_4': {
174+
enabled: false,
175+
cnr_id: 'ComfyUI-GGUF',
176+
ver: '1.1.4'
177+
},
178+
'test-pack': {
179+
enabled: true,
180+
cnr_id: 'test-pack',
181+
ver: '2.0.0'
182+
}
183+
}
184+
}
185+
186+
queue.updateTaskState(mockState)
187+
188+
// Packs should be accessible by normalized keys
189+
expect(installedPacks.value['ComfyUI-GGUF']).toEqual({
190+
enabled: false,
191+
cnr_id: 'ComfyUI-GGUF',
192+
ver: '1.1.4'
193+
})
194+
expect(installedPacks.value['test-pack']).toEqual({
195+
enabled: true,
196+
cnr_id: 'test-pack',
197+
ver: '2.0.0'
198+
})
199+
200+
// Version suffixed keys should not exist after normalization
201+
// The pack should be accessible by its base name only (without @version)
202+
expect(installedPacks.value['ComfyUI-GGUF@1_1_4']).toBeUndefined()
203+
})
204+
205+
it('handles empty installed_packs gracefully', () => {
206+
const queue = createManagerQueue()
207+
208+
const mockState: any = {
209+
history: {},
210+
running_queue: [],
211+
pending_queue: [],
212+
installed_packs: undefined
213+
}
214+
215+
// Just call the function - if it throws, the test will fail automatically
216+
queue.updateTaskState(mockState)
217+
218+
// installedPacks should remain unchanged
219+
expect(installedPacks.value).toEqual({})
220+
})
164221
})
165222
})

tests-ui/tests/store/comfyManagerStore.test.ts

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -439,4 +439,97 @@ describe('useComfyManagerStore', () => {
439439
expect(store.isPackInstalling('pack-3')).toBe(false)
440440
})
441441
})
442+
443+
describe('refreshInstalledList with pack ID normalization', () => {
444+
it('normalizes pack IDs by removing version suffixes', async () => {
445+
const mockPacks = {
446+
'ComfyUI-GGUF@1_1_4': {
447+
enabled: false,
448+
cnr_id: 'ComfyUI-GGUF',
449+
ver: '1.1.4',
450+
aux_id: undefined
451+
},
452+
'ComfyUI-Manager': {
453+
enabled: true,
454+
cnr_id: 'ComfyUI-Manager',
455+
ver: '2.0.0',
456+
aux_id: undefined
457+
}
458+
}
459+
460+
vi.mocked(mockManagerService.listInstalledPacks).mockResolvedValue(
461+
mockPacks
462+
)
463+
464+
const store = useComfyManagerStore()
465+
await store.refreshInstalledList()
466+
467+
// Both packs should be accessible by their base name
468+
expect(store.installedPacks['ComfyUI-GGUF']).toEqual({
469+
enabled: false,
470+
cnr_id: 'ComfyUI-GGUF',
471+
ver: '1.1.4',
472+
aux_id: undefined
473+
})
474+
expect(store.installedPacks['ComfyUI-Manager']).toEqual({
475+
enabled: true,
476+
cnr_id: 'ComfyUI-Manager',
477+
ver: '2.0.0',
478+
aux_id: undefined
479+
})
480+
481+
// Version suffixed keys should not exist
482+
expect(store.installedPacks['ComfyUI-GGUF@1_1_4']).toBeUndefined()
483+
})
484+
485+
it('handles duplicate keys after normalization', async () => {
486+
const mockPacks = {
487+
'test-pack': {
488+
enabled: true,
489+
cnr_id: 'test-pack',
490+
ver: '1.0.0',
491+
aux_id: undefined
492+
},
493+
'test-pack@1_1_0': {
494+
enabled: false,
495+
cnr_id: 'test-pack',
496+
ver: '1.1.0',
497+
aux_id: undefined
498+
}
499+
}
500+
501+
vi.mocked(mockManagerService.listInstalledPacks).mockResolvedValue(
502+
mockPacks
503+
)
504+
505+
const store = useComfyManagerStore()
506+
await store.refreshInstalledList()
507+
508+
// The normalized key should exist (last one wins with mapKeys)
509+
expect(store.installedPacks['test-pack']).toBeDefined()
510+
expect(store.installedPacks['test-pack'].ver).toBe('1.1.0')
511+
})
512+
513+
it('preserves version information for disabled packs', async () => {
514+
const mockPacks = {
515+
'disabled-pack@2_0_0': {
516+
enabled: false,
517+
cnr_id: 'disabled-pack',
518+
ver: '2.0.0',
519+
aux_id: undefined
520+
}
521+
}
522+
523+
vi.mocked(mockManagerService.listInstalledPacks).mockResolvedValue(
524+
mockPacks
525+
)
526+
527+
const store = useComfyManagerStore()
528+
await store.refreshInstalledList()
529+
530+
// Pack should be accessible by base name with version preserved
531+
expect(store.getInstalledPackVersion('disabled-pack')).toBe('2.0.0')
532+
expect(store.isPackInstalled('disabled-pack')).toBe(true)
533+
})
534+
})
442535
})

0 commit comments

Comments
 (0)