[feat] integrate asset browser with widget system (#5629)

## Summary

Add asset browser dialog integration for combo widgets with full
animation support and proper state management.

(Thank you Claude from saving me me from merge conflict hell on this
one.)

## Changes

- Widget integration: combo widgets now use AssetBrowserModal for
eligible asset types
- Dialog animations: added animateHide() for smooth close transitions
- Async operations: proper sequencing of widget updates and dialog
animations
- Service layer: added getAssetsForNodeType() and getAssetDetails()
methods
- Type safety: comprehensive TypeScript types and error handling
- Test coverage: unit tests for all new functionality
- Bonus: fixed the hardcoded labels in AssetFilterBar

Widget behavior:
- Shows asset browser button for eligible widgets when asset API enabled
- Handles asset selection with proper callback sequencing
- Maintains widget value updates and litegraph notification

## Review Focus

I will call out some stuff inline.

## Screenshots


https://github.com/user-attachments/assets/9d3a72cf-d2b0-445f-8022-4c49daa04637

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-5629-feat-integrate-asset-browser-with-widget-system-2726d73d365081a9a98be9a2307aee0b)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: GitHub Action <action@github.com>
This commit is contained in:
Arjan Singh
2025-09-20 11:44:18 -07:00
committed by GitHub
parent b3c939ff15
commit fd12591756
21 changed files with 701 additions and 102 deletions

View File

@@ -1,10 +1,33 @@
import { describe, expect, it } from 'vitest'
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { nextTick } from 'vue'
import { useAssetBrowser } from '@/platform/assets/composables/useAssetBrowser'
import type { AssetItem } from '@/platform/assets/schemas/assetSchema'
import { assetService } from '@/platform/assets/services/assetService'
vi.mock('@/platform/assets/services/assetService', () => ({
assetService: {
getAssetDetails: vi.fn()
}
}))
vi.mock('@/i18n', () => ({
t: (key: string) => {
const translations: Record<string, string> = {
'assetBrowser.allModels': 'All Models',
'assetBrowser.assets': 'Assets',
'assetBrowser.unknown': 'unknown'
}
return translations[key] || key
},
d: (date: Date) => date.toLocaleDateString()
}))
describe('useAssetBrowser', () => {
beforeEach(() => {
vi.restoreAllMocks()
})
// Test fixtures - minimal data focused on functionality being tested
const createApiAsset = (overrides: Partial<AssetItem> = {}): AssetItem => ({
id: 'test-id',
@@ -26,8 +49,8 @@ describe('useAssetBrowser', () => {
user_metadata: { description: 'Test model' }
})
const { transformAssetForDisplay } = useAssetBrowser([apiAsset])
const result = transformAssetForDisplay(apiAsset)
const { filteredAssets } = useAssetBrowser([apiAsset])
const result = filteredAssets.value[0] // Get the transformed asset from filteredAssets
// Preserves API properties
expect(result.id).toBe(apiAsset.id)
@@ -49,15 +72,13 @@ describe('useAssetBrowser', () => {
user_metadata: undefined
})
const { transformAssetForDisplay } = useAssetBrowser([apiAsset])
const result = transformAssetForDisplay(apiAsset)
const { filteredAssets } = useAssetBrowser([apiAsset])
const result = filteredAssets.value[0]
expect(result.description).toBe('loras model')
})
it('formats various file sizes correctly', () => {
const { transformAssetForDisplay } = useAssetBrowser([])
const testCases = [
{ size: 512, expected: '512 B' },
{ size: 1536, expected: '1.5 KB' },
@@ -67,7 +88,8 @@ describe('useAssetBrowser', () => {
testCases.forEach(({ size, expected }) => {
const asset = createApiAsset({ size })
const result = transformAssetForDisplay(asset)
const { filteredAssets } = useAssetBrowser([asset])
const result = filteredAssets.value[0]
expect(result.formattedSize).toBe(expected)
})
})
@@ -236,18 +258,182 @@ describe('useAssetBrowser', () => {
})
})
describe('Asset Selection', () => {
it('returns selected asset UUID for efficient handling', () => {
describe('Async Asset Selection with Detail Fetching', () => {
it('should fetch asset details and call onSelect with filename when provided', async () => {
const onSelectSpy = vi.fn()
const asset = createApiAsset({
id: 'test-uuid-123',
name: 'selected_model.safetensors'
id: 'asset-123',
name: 'test-model.safetensors'
})
const { selectAsset, transformAssetForDisplay } = useAssetBrowser([asset])
const displayAsset = transformAssetForDisplay(asset)
const result = selectAsset(displayAsset)
const detailAsset = createApiAsset({
id: 'asset-123',
name: 'test-model.safetensors',
user_metadata: { filename: 'checkpoints/test-model.safetensors' }
})
vi.mocked(assetService.getAssetDetails).mockResolvedValue(detailAsset)
expect(result).toBe('test-uuid-123')
const { selectAssetWithCallback } = useAssetBrowser([asset])
await selectAssetWithCallback(asset.id, onSelectSpy)
expect(assetService.getAssetDetails).toHaveBeenCalledWith('asset-123')
expect(onSelectSpy).toHaveBeenCalledWith(
'checkpoints/test-model.safetensors'
)
})
it('should handle missing user_metadata.filename as error', async () => {
const consoleErrorSpy = vi
.spyOn(console, 'error')
.mockImplementation(() => {})
const onSelectSpy = vi.fn()
const asset = createApiAsset({ id: 'asset-456' })
const detailAsset = createApiAsset({
id: 'asset-456',
user_metadata: { filename: '' } // Invalid empty filename
})
vi.mocked(assetService.getAssetDetails).mockResolvedValue(detailAsset)
const { selectAssetWithCallback } = useAssetBrowser([asset])
await selectAssetWithCallback(asset.id, onSelectSpy)
expect(assetService.getAssetDetails).toHaveBeenCalledWith('asset-456')
expect(onSelectSpy).not.toHaveBeenCalled()
expect(consoleErrorSpy).toHaveBeenCalledWith(
'Invalid asset filename:',
expect.arrayContaining([
expect.objectContaining({
message: 'Filename cannot be empty'
})
]),
'for asset:',
'asset-456'
)
})
it('should handle API errors gracefully', async () => {
const consoleErrorSpy = vi
.spyOn(console, 'error')
.mockImplementation(() => {})
const onSelectSpy = vi.fn()
const asset = createApiAsset({ id: 'asset-789' })
const apiError = new Error('API Error')
vi.mocked(assetService.getAssetDetails).mockRejectedValue(apiError)
const { selectAssetWithCallback } = useAssetBrowser([asset])
await selectAssetWithCallback(asset.id, onSelectSpy)
expect(assetService.getAssetDetails).toHaveBeenCalledWith('asset-789')
expect(onSelectSpy).not.toHaveBeenCalled()
expect(consoleErrorSpy).toHaveBeenCalledWith(
expect.stringContaining('Failed to fetch asset details for asset-789'),
apiError
)
})
it('should not fetch details when no callback provided', async () => {
const asset = createApiAsset({ id: 'asset-no-callback' })
const { selectAssetWithCallback } = useAssetBrowser([asset])
await selectAssetWithCallback(asset.id)
expect(assetService.getAssetDetails).not.toHaveBeenCalled()
})
})
describe('Filename Validation Security', () => {
const createValidationTest = (filename: string) => {
const testAsset = createApiAsset({ id: 'validation-test' })
const detailAsset = createApiAsset({
id: 'validation-test',
user_metadata: { filename }
})
return { testAsset, detailAsset }
}
it('accepts valid file paths with forward slashes', async () => {
const onSelectSpy = vi.fn()
const { testAsset, detailAsset } = createValidationTest(
'models/checkpoints/v1/test-model.safetensors'
)
vi.mocked(assetService.getAssetDetails).mockResolvedValue(detailAsset)
const { selectAssetWithCallback } = useAssetBrowser([testAsset])
await selectAssetWithCallback(testAsset.id, onSelectSpy)
expect(onSelectSpy).toHaveBeenCalledWith(
'models/checkpoints/v1/test-model.safetensors'
)
})
it('rejects directory traversal attacks', async () => {
const consoleErrorSpy = vi
.spyOn(console, 'error')
.mockImplementation(() => {})
const onSelectSpy = vi.fn()
const maliciousPaths = [
'../malicious-model.safetensors',
'models/../../../etc/passwd',
'/etc/passwd'
]
for (const path of maliciousPaths) {
const { testAsset, detailAsset } = createValidationTest(path)
vi.mocked(assetService.getAssetDetails).mockResolvedValue(detailAsset)
const { selectAssetWithCallback } = useAssetBrowser([testAsset])
await selectAssetWithCallback(testAsset.id, onSelectSpy)
expect(onSelectSpy).not.toHaveBeenCalled()
expect(consoleErrorSpy).toHaveBeenCalledWith(
'Invalid asset filename:',
expect.arrayContaining([
expect.objectContaining({
message: 'Path must not start with / or contain ..'
})
]),
'for asset:',
'validation-test'
)
}
})
it('rejects invalid filename characters', async () => {
const consoleErrorSpy = vi
.spyOn(console, 'error')
.mockImplementation(() => {})
const onSelectSpy = vi.fn()
const invalidChars = ['\\', ':', '*', '?', '"', '<', '>', '|']
for (const char of invalidChars) {
const { testAsset, detailAsset } = createValidationTest(
`bad${char}filename.safetensors`
)
vi.mocked(assetService.getAssetDetails).mockResolvedValue(detailAsset)
const { selectAssetWithCallback } = useAssetBrowser([testAsset])
await selectAssetWithCallback(testAsset.id, onSelectSpy)
expect(onSelectSpy).not.toHaveBeenCalled()
expect(consoleErrorSpy).toHaveBeenCalledWith(
'Invalid asset filename:',
expect.arrayContaining([
expect.objectContaining({
message: 'Invalid filename characters'
})
]),
'for asset:',
'validation-test'
)
}
})
})

View File

@@ -6,11 +6,18 @@ import { useDialogStore } from '@/stores/dialogStore'
// Mock the dialog store
vi.mock('@/stores/dialogStore')
// Mock the asset service
vi.mock('@/platform/assets/services/assetService', () => ({
assetService: {
getAssetsForNodeType: vi.fn().mockResolvedValue([])
}
}))
// Test factory functions
interface AssetBrowserProps {
nodeType: string
inputName: string
onAssetSelected?: ReturnType<typeof vi.fn>
onAssetSelected?: (filename: string) => void
}
function createAssetBrowserProps(
@@ -25,7 +32,7 @@ function createAssetBrowserProps(
describe('useAssetBrowserDialog', () => {
describe('Asset Selection Flow', () => {
it('auto-closes dialog when asset is selected', () => {
it('auto-closes dialog when asset is selected', async () => {
// Create fresh mocks for this test
const mockShowDialog = vi.fn()
const mockCloseDialog = vi.fn()
@@ -41,7 +48,7 @@ describe('useAssetBrowserDialog', () => {
const onAssetSelected = vi.fn()
const props = createAssetBrowserProps({ onAssetSelected })
assetBrowserDialog.show(props)
await assetBrowserDialog.show(props)
// Get the onSelect handler that was passed to the dialog
const dialogCall = mockShowDialog.mock.calls[0][0]
@@ -50,14 +57,14 @@ describe('useAssetBrowserDialog', () => {
// Simulate asset selection
onSelectHandler('selected-asset-path')
// Should call the original callback and close dialog
// Should call the original callback and trigger hide animation
expect(onAssetSelected).toHaveBeenCalledWith('selected-asset-path')
expect(mockCloseDialog).toHaveBeenCalledWith({
key: 'global-asset-browser'
})
})
it('closes dialog when close handler is called', () => {
it('closes dialog when close handler is called', async () => {
// Create fresh mocks for this test
const mockShowDialog = vi.fn()
const mockCloseDialog = vi.fn()
@@ -72,7 +79,7 @@ describe('useAssetBrowserDialog', () => {
const assetBrowserDialog = useAssetBrowserDialog()
const props = createAssetBrowserProps()
assetBrowserDialog.show(props)
await assetBrowserDialog.show(props)
// Get the onClose handler that was passed to the dialog
const dialogCall = mockShowDialog.mock.calls[0][0]