[bugfix] Fix Load Image (from Outputs) node failing on macOS #7200
Conversation
Filter out .DS_Store and placeholder files from remote widget file lists. On macOS, the .DS_Store file (a hidden system file created by Finder) was being included in the output folder file list. When the "Load Image (from Outputs)" node refreshed, it would select .DS_Store as the first file, causing the image preview to fail with an error since .DS_Store is not a valid image file. This fix filters out .DS_Store and the _output_images_will_be_put_here placeholder file from file lists returned by remote widget API calls.
dosubot
bot
added
the
size:S
📝 WalkthroughWalkthroughIntroduces a filtering mechanism to Changes
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🎭 Playwright Test Results⏰ Completed at: 12/06/2025, 03:23:54 AM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 12/06/2025, 03:15:21 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.2 MB (baseline 3.2 MB) • 🔴 +416 BMain entry bundles and manifests
Status: 3 added / 3 removed Graph Workspace — 974 kB (baseline 974 kB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 6.54 kB (baseline 6.54 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 1 added / 1 removed Panels & Settings — 298 kB (baseline 298 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 6 added / 6 removed UI Components — 177 kB (baseline 177 kB) • ⚪ 0 BReusable component library chunks
Status: 6 added / 6 removed Data & Services — 12.5 kB (baseline 12.5 kB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 2 added / 2 removed Utilities & Hooks — 2.94 kB (baseline 2.94 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 1 added / 1 removed Vendor & Third-Party — 8.56 MB (baseline 8.56 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 3.81 MB (baseline 3.81 MB) • ⚪ 0 BBundles that do not match a named category
Status: 17 added / 17 removed |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/renderer/extensions/vueNodes/widgets/composables/useRemoteWidget.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
src/**/*.{vue,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{vue,ts}: Leverage VueUse functions for performance-enhancing styles
Implement proper error handling
Use vue-i18n in composition API for any string literals. Place new translation entries in src/locales/en/main.json
Files:
src/renderer/extensions/vueNodes/widgets/composables/useRemoteWidget.ts
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Use es-toolkit for utility functions
Use TypeScript for type safety
Files:
src/renderer/extensions/vueNodes/widgets/composables/useRemoteWidget.ts
src/**/{services,composables}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/{services,composables}/**/*.{ts,tsx}: Useapi.apiURL()for backend endpoints instead of constructing URLs directly
Useapi.fileURL()for static file access instead of constructing URLs directly
Files:
src/renderer/extensions/vueNodes/widgets/composables/useRemoteWidget.ts
src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.{ts,tsx,vue}: Sanitize HTML with DOMPurify to prevent XSS attacks
Avoid using @ts-expect-error; use proper TypeScript types instead
Use es-toolkit for utility functions instead of other utility libraries
Implement proper TypeScript types throughout the codebase
Files:
src/renderer/extensions/vueNodes/widgets/composables/useRemoteWidget.ts
src/**/{composables,components}/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Clean up subscriptions in state management to prevent memory leaks
Files:
src/renderer/extensions/vueNodes/widgets/composables/useRemoteWidget.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/renderer/extensions/vueNodes/widgets/composables/useRemoteWidget.ts
src/**/{components,composables}/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Use vue-i18n for ALL user-facing strings by adding them to
src/locales/en/main.json
Files:
src/renderer/extensions/vueNodes/widgets/composables/useRemoteWidget.ts
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript exclusively; no new JavaScript code
Files:
src/renderer/extensions/vueNodes/widgets/composables/useRemoteWidget.ts
**/*.{ts,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,vue}: Use camelCase for variable and function names
Indent with 2 spaces (see.prettierrc)
Use single quotes for strings (see.prettierrc)
No trailing semicolons (see.prettierrc)
Maximum line width of 80 characters (see.prettierrc)
Sort and group imports by plugin (runpnpm formatbefore committing)
Never useanytype; use proper TypeScript types instead
Never useas anytype assertions; fix the underlying type issue instead
Avoid code comments unless absolutely necessary; write expressive, self-documenting code instead
When writing new code, ask if there is a simpler way to introduce the same functionality; if yes, choose the simpler approach
Use refactoring to make complex code simpler
Use es-toolkit for utility functions
Use Vite for fast development and building
Implement proper error handling
Write tests for all changes, especially bug fixes to catch future regressions
Files:
src/renderer/extensions/vueNodes/widgets/composables/useRemoteWidget.ts
**/composables/**/use*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name composables with
useXyz.tspattern
Files:
src/renderer/extensions/vueNodes/widgets/composables/useRemoteWidget.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: lint-and-format
- GitHub Check: test
- GitHub Check: setup
- GitHub Check: collect
🔇 Additional comments (1)
src/renderer/extensions/vueNodes/widgets/composables/useRemoteWidget.ts (1)
96-97: Good integration point; consider adding regression tests for filtered filenamesRouting the
response_key-selected data throughfilterFileListensures all relevant array responses get the.DS_Store/ placeholder filtering before caching and before first/refresh selection logic runs. This should prevent the “first item is.DS_Store” issue for the Load Image (from Outputs) node.If there isn’t already coverage, consider adding a small test (or set of tests) that exercises
useRemoteWidgetwith a mocked axios response containing.DS_Storeand_output_images_will_be_put_hereentries, asserting they are not present in the cached/returned list. This will protect against regressions in future refactors.
| // Filter out invalid files like .DS_Store | ||
| const filterFileList = <T>(data: T): T => { | ||
| if (!Array.isArray(data)) return data | ||
| return data.filter((item) => { | ||
| if (typeof item !== 'string') return true | ||
| const basename = item.split('/').pop() ?? '' | ||
| if (['_output_images_will_be_put_here', '.DS_Store'].includes(basename)) { | ||
| return false | ||
| } | ||
| return true | ||
| }) as T | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Runtime filtering logic is correct and safely scoped to string arrays
The filtering logic does what the PR needs: it only touches array responses, removes entries whose basename is .DS_Store or _output_images_will_be_put_here, and leaves non-string and non-array data untouched. This is a good low-risk way to apply the fix across all remote widgets.
If you want to tighten types a bit, you could make this helper explicitly array-focused, e.g. filterFileList<T>(data: T[]): T[] and guard at the call site, to avoid the as T cast on a structurally different shape. Not blocking, but would make the helper’s contract clearer.
🤖 Prompt for AI Agents
In src/renderer/extensions/vueNodes/widgets/composables/useRemoteWidget.ts
around lines 68 to 79, the helper filterFileList is overly generic and uses an
unsafe cast (as T) after array filtering; change its signature to accept and
return arrays (e.g. filterFileList<T>(data: T[]): T[]), remove the Array.isArray
guard inside, and update call sites to only pass arrays (or guard there) so no
type cast is needed and the function’s contract is explicit and type-safe.
There was a problem hiding this comment.
Can we add more constraints to the type here?
There was a problem hiding this comment.
@DrJKL, yes! We can definitely improve the type constraints here. The current signature with <T>(data: T): T and the as T cast is type-unsafe. Here are a few options:
Option 1: Function Overloads (Recommended)
function filterFileList<T>(data: T[]): T[]
function filterFileList<T>(data: T): T
function filterFileList<T>(data: T | T[]): T | T[] {
if (!Array.isArray(data)) return data
return data.filter((item) => {
if (typeof item !== 'string') return true
const basename = item.split('/').pop() ?? ''
return !['_output_images_will_be_put_here', '.DS_Store'].includes(basename)
})
}This removes the unsafe as T cast and makes the contract explicit: arrays in → arrays out, non-arrays pass through.
Option 2: Conditional Types (More Advanced)
type FilteredData<T> = T extends readonly unknown[] ? T : T
const filterFileList = <T>(data: T): FilteredData<T> => {
if (!Array.isArray(data)) return data as FilteredData<T>
return data.filter((item) => {
if (typeof item !== 'string') return true
const basename = item.split('/').pop() ?? ''
return !['_output_images_will_be_put_here', '.DS_Store'].includes(basename)
}) as FilteredData<T>
}Option 3: Constrain to String Arrays
If we know the arrays will always contain strings:
function filterFileList(data: string[]): string[]
function filterFileList<T>(data: T): T
function filterFileList<T>(data: T | string[]): T | string[] {
if (!Array.isArray(data)) return data
return data.filter((item) => {
if (typeof item !== 'string') return true
const basename = item.split('/').pop() ?? ''
return !['_output_images_will_be_put_here', '.DS_Store'].includes(basename)
})
}I recommend Option 1 (function overloads) as it provides the best balance of type safety and clarity without requiring casts. Would you like me to prepare that change?
🧠 Learnings used
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T00:52:35.750Z
Learning: Applies to **/*.{ts,vue} : Never use `as any` type assertions; fix the underlying type issue instead
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T00:52:35.750Z
Learning: Applies to **/*.{ts,vue} : Never use `any` type; use proper TypeScript types instead
There was a problem hiding this comment.
I don't love solving this by adding a denylist in this one location that we'll have to maintain.
There was a problem hiding this comment.
Why don't we know the type of the data we're fetching?
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for the /internal/files/output endpoint definition
rg -n "internal/files/output" -C 3
# Search for file listing or output directory handling
rg -n "files.*output|output.*files" --type py --type js --type ts -C 2
# Look for server-side file serving or API route definitions
rg -n "def.*files|route.*files|@.*\(/internal" --type py -C 3
Length of output: 1850
🏁 Script executed:
#!/bin/bash
# Search for backend server files (Python, typically ComfyUI uses Python)
fd -e py -E node_modules -E dist -E build | head -20
# Look for route definitions or API handlers
rg -n "class.*Server|FastAPI|Flask|@app\." --type py -C 2 | head -50
Length of output: 307
🏁 Script executed:
#!/bin/bash
# Check if there's a backend directory or server code in this repo
fd server.py
fd routes.py
fd api.py
# Look for any Python files that might contain the endpoint
rg -l "/internal/" --type py
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Find where the /internal/files/output endpoint is actually called
rg -n "internal/files" --type ts --type js -C 3
# Look for API route configurations or endpoint definitions
rg -n "route.*=.*['\"]/" --type ts -C 2
# Check if there's any configuration for file filtering
rg -n "filter|exclude|ignore" --type ts src/renderer/extensions/vueNodes/widgets/composables/ -C 2
Length of output: 3291
🏁 Script executed:
#!/bin/bash
# Look at the schema for RemoteWidgetConfig to understand configuration options
rg -n "RemoteWidgetConfig|interface.*Remote" --type ts -C 5
# Check if there are examples of remote widget usage
fd -e json -e ts -e js | xargs rg -l "internal/files" | head -10
Length of output: 9858
🏁 Script executed:
#!/bin/bash
# Check the README or docs for information about backend integration
fd README.md | xargs cat
Length of output: 116772
@DrJKL, you're absolutely right that a hardcoded denylist is not ideal. Here are some better architectural approaches:
1. Backend Filtering (Recommended)
The cleanest solution is to filter these files at the backend /internal/files/output endpoint. This would:
- Handle the issue at the source
- Benefit all clients (not just this frontend)
- Be easier to maintain in one location
- Allow platform-agnostic filtering (hidden files, system files, etc.)
Backend approach could filter:
# In the backend endpoint handler
def should_include_file(filename: str) -> bool:
# Filter hidden files (cross-platform)
if filename.startswith('.'):
return False
# Filter placeholder files
if filename.startswith('_') and 'put_here' in filename.lower():
return False
return True2. Pattern-Based Frontend Filtering (If backend changes not feasible)
Instead of a denylist, use patterns:
const filterFileList = <T>(data: T): T => {
if (!Array.isArray(data)) return data
return data.filter((item) => {
if (typeof item !== 'string') return true
const basename = item.split('/').pop() ?? ''
// Filter any hidden files (starts with .)
if (basename.startsWith('.')) return false
// Filter placeholder files
if (basename.startsWith('_') && basename.includes('put_here')) return false
return true
}) as T
}3. Centralized Configuration
If specific filenames must be filtered, at least centralize them:
// src/constants/fileFilters.ts
export const EXCLUDED_FILENAMES = [
'.DS_Store',
'_output_images_will_be_put_here',
'Thumbs.db', // Windows thumbnail cache
// Add more as needed
]
// Then use everywhere needed
return !EXCLUDED_FILENAMES.includes(basename)Question: Is modifying the backend endpoint (/internal/files/output) an option? If so, I'd strongly recommend option 1. If not, I can help refactor to option 2 or 3.
🧠 Learnings used
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T00:52:35.750Z
Learning: Applies to **/*.{ts,vue} : Never use `as any` type assertions; fix the underlying type issue instead
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the RemoteWidgetConfig schema definition
fd -e ts -e d.ts nodeDefSchema | head -5 | xargs cat
Length of output: 13943
🏁 Script executed:
#!/bin/bash
# Look for usages of useRemoteWidget to understand what types are actually being passed
rg -n -C3 "useRemoteWidget<" --type=ts --type=vue
Length of output: 27
🏁 Script executed:
#!/bin/bash
# Find where RemoteWidgetConfig is used to see if there are different widget types
rg -n -C2 "RemoteWidgetConfig" --type=ts | head -80
Length of output: 4845
🏁 Script executed:
#!/bin/bash
# Look for the /internal/files/output endpoint or similar to understand what routes are used
rg -n "internal/files" --type=ts --type=vue
Length of output: 27
🏁 Script executed:
#!/bin/bash
# Find where combo widgets with remote config are actually created/used
rg -n -B5 -A5 "remote.*route.*internal" --type=ts | head -100
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Look for image_folder or file-related widget handling
rg -n -C4 "image_folder|image_upload" --type=ts | head -80
Length of output: 6607
🏁 Script executed:
#!/bin/bash
# Find ComboWidget implementation to see where remote widgets are instantiated
fd -e ts ComboWidget | head -3 | xargs cat
Length of output: 46376
🏁 Script executed:
#!/bin/bash
# Search for uses of the combo options remote property
ast-grep --pattern 'remote: {
$$$
}'
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check what the backend node definitions look like - search for examples of remote config usage
rg -n "\"remote\"" --type=py -C3 | head -100
Length of output: 2595
🏁 Script executed:
#!/bin/bash
# Look for the actual API endpoint implementation that returns file lists
rg -n "/internal/files|/api/files" --type=py -C5 | head -80
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Find if there are different types of remote widgets being used
rg -n "remoteConfig|remote:" --type=ts -C2 | head -100
Length of output: 7560
@DrJKL, Great question—this gets at a fundamental architectural issue.
Why we don't know the type:
-
RemoteWidgetConfighas no response type information: The schema only specifies runtime configuration (route, refresh interval, query params), but nothing about what shape the response data takes. -
useRemoteWidgetis a generic utility used across many widget types: It handles file lists (for image/video widgets), model checkpoint lists, and potentially other arbitrary data sources. The generic type parameterTis caller-supplied but isn't enforced or validated against what the API actually returns. -
No static type discrimination: When
fetchDatacallsaxios.get, it returnsany. There's no way at the type level to know if we're fetching:- An array of file paths (strings)
- An array of model objects
- A single value
- Something else entirely
The deeper problem:
The current PR applies filterFileList universally to all remote widgets, but the .DS_Store filtering only makes sense for file path arrays. This is why you're uncomfortable—it's a domain-specific fix (file system artifacts) living in a generic utility.
Architectural alternatives:
-
Backend filtering: Have the file-serving endpoint (e.g.,
/internal/files/output) filter out.DS_Storeand placeholder files before returning them. This keeps the frontend generic. -
Response type discrimination in
RemoteWidgetConfig: Add an optionalresponse_typefield (e.g.,"file_list" | "model_list" | "generic") and apply filtering conditionally based on that. -
Widget-type-specific filtering: Move this logic into
useComboWidgetwhere we know we're dealing withimage_folderfile paths, rather than the genericuseRemoteWidget.
Which direction would you prefer? My instinct is option 1 (backend filtering) is cleanest—the backend already knows it's serving a file list and can exclude system files at the source.
🧬 Code Graph Analysis Results
src/schemas/nodeDefSchema.ts (line 258)
export type RemoteWidgetConfig = z.infer<typeof zRemoteWidgetConfig>
🧠 Learnings used
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T00:52:35.750Z
Learning: Applies to **/*.{ts,vue} : Never use `as any` type assertions; fix the underlying type issue instead
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T00:52:35.750Z
Learning: Applies to **/*.{ts,vue} : Never use `any` type; use proper TypeScript types instead
Summary
.DS_Storeand_output_images_will_be_put_hereplaceholder files from remote widget file listsProblem
On macOS, the
.DS_Storefile (a hidden system file automatically created by Finder) was being included in the output folder file list returned by/internal/files/output. When the "Load Image (from Outputs)" node refreshed its file list,.DS_Storewould be selected as the first file, causing the image preview to fail since it's not a valid image.Solution
Added a filter in
useRemoteWidget.tsto exclude.DS_Storeand the_output_images_will_be_put_hereplaceholder file from file lists returned by remote widget API calls.Test plan
Before
After
Screen.Recording.2025-12-06.at.04.10.33.mov
┆Issue is synchronized with this Notion page by Unito