Skip to content

Commit 320a836

Browse files
committed
fix: address CodeRabbit review feedback
- Assert cleanup response in E2E test beforeEach - Guard downloadUrl type in fetchCivitaiMetadata predicate - Pass directory resolver unconditionally for OSS model categorization - Add abort controller to OSS branch for stale workflow protection
1 parent 5a9db74 commit 320a836

File tree

3 files changed

+24
-11
lines changed

3 files changed

+24
-11
lines changed

browser_tests/tests/dialog.spec.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,11 @@ test.describe('Missing models in Error Tab', () => {
9595
'Comfy.RightSidePanel.ShowErrorsTab',
9696
true
9797
)
98-
await comfyPage.page.evaluate((url: string) => {
99-
return fetch(`${url}/api/devtools/cleanup_fake_model`)
98+
const cleanupOk = await comfyPage.page.evaluate(async (url: string) => {
99+
const response = await fetch(`${url}/api/devtools/cleanup_fake_model`)
100+
return response.ok
100101
}, comfyPage.url)
102+
expect(cleanupOk).toBeTruthy()
101103
})
102104

103105
test('Should show error overlay with missing models when workflow has missing models', async ({

src/platform/missingModel/missingModelDownload.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,9 +97,14 @@ async function fetchCivitaiMetadata(url: string): Promise<ModelMetadata> {
9797
if (!res.ok) return { fileSize: null, gatedRepoUrl: null }
9898

9999
const data: CivitaiModelVersionResponse = await res.json()
100-
const matchingFile = data.files?.find(
101-
(file) => file.downloadUrl.startsWith(url) && file.downloadUrl
102-
)
100+
const matchingFile = data.files?.find((file) => {
101+
const downloadUrl = file.downloadUrl
102+
return (
103+
typeof downloadUrl === 'string' &&
104+
downloadUrl.length > 0 &&
105+
downloadUrl.startsWith(url)
106+
)
107+
})
103108
const fileSize = matchingFile?.sizeKB ? matchingFile.sizeKB * 1024 : null
104109
return { fileSize, gatedRepoUrl: null }
105110
} catch {

src/scripts/app.ts

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1421,15 +1421,16 @@ export class ComfyApp {
14211421
): Promise<{ missingModels: ModelFile[] }> {
14221422
const missingModelStore = useMissingModelStore()
14231423

1424+
const getDirectory = (nodeType: string) =>
1425+
useModelToNodeStore().getCategoryForNodeType(nodeType)
1426+
14241427
const candidates = scanAllModelCandidates(
14251428
this.rootGraph,
14261429
isCloud
14271430
? (nodeType, widgetName) =>
14281431
assetService.shouldUseAssetBrowser(nodeType, widgetName)
14291432
: () => false,
1430-
isCloud
1431-
? (nodeType) => useModelToNodeStore().getCategoryForNodeType(nodeType)
1432-
: undefined
1433+
getDirectory
14331434
)
14341435

14351436
const modelStore = useModelStore()
@@ -1503,11 +1504,14 @@ export class ComfyApp {
15031504
})
15041505
})
15051506
} else {
1507+
const controller =
1508+
missingModelStore.createVerificationAbortController()
15061509
const confirmed = enrichedCandidates.filter((c) => c.isMissing === true)
15071510
if (confirmed.length) {
15081511
api
15091512
.getFolderPaths()
15101513
.then((paths) => {
1514+
if (controller.signal.aborted) return
15111515
missingModelStore.setFolderPaths(paths)
15121516
})
15131517
.catch((err) => {
@@ -1517,17 +1521,19 @@ export class ComfyApp {
15171521
)
15181522
})
15191523
.finally(() => {
1524+
if (controller.signal.aborted) return
15201525
useExecutionErrorStore().surfaceMissingModels(confirmed)
15211526
})
15221527

15231528
void Promise.allSettled(
15241529
confirmed
15251530
.filter((c) => c.url)
15261531
.map(async (c) => {
1527-
const { fetchModelMetadata } =
1528-
await import('@/platform/missingModel/missingModelDownload')
1532+
const { fetchModelMetadata } = await import(
1533+
'@/platform/missingModel/missingModelDownload'
1534+
)
15291535
const metadata = await fetchModelMetadata(c.url!)
1530-
if (metadata.fileSize !== null) {
1536+
if (!controller.signal.aborted && metadata.fileSize !== null) {
15311537
missingModelStore.setFileSize(c.url!, metadata.fileSize)
15321538
}
15331539
})

0 commit comments

Comments
 (0)