-
Notifications
You must be signed in to change notification settings - Fork 406
sync devtools nodes with test ComfyUI dir in test global setup and add dev nodes for all new widget types #6623
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 4 commits
fcdb5dd
9985101
4699859
6596aa8
5b33d3f
8fc5e87
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| import fs from 'fs-extra' | ||
| import path from 'path' | ||
| import { fileURLToPath } from 'url' | ||
|
|
||
| export function syncDevtools(targetComfyDir: string) { | ||
| if (!targetComfyDir) { | ||
| console.warn('syncDevtools skipped: TEST_COMFYUI_DIR not set') | ||
| return | ||
| } | ||
|
|
||
| const moduleDir = | ||
| typeof __dirname !== 'undefined' | ||
| ? __dirname | ||
| : path.dirname(fileURLToPath(import.meta.url)) | ||
|
|
||
| const devtoolsSrc = path.resolve(moduleDir, '..', '..', 'tools', 'devtools') | ||
|
|
||
| if (!fs.pathExistsSync(devtoolsSrc)) { | ||
| console.warn( | ||
| `syncDevtools skipped: source directory not found at ${devtoolsSrc}` | ||
| ) | ||
| return | ||
| } | ||
|
|
||
| const devtoolsDest = path.resolve( | ||
| targetComfyDir, | ||
| 'custom_nodes', | ||
| 'ComfyUI_devtools' | ||
| ) | ||
|
|
||
| console.warn(`syncDevtools: copying ${devtoolsSrc} -> ${devtoolsDest}`) | ||
|
|
||
| try { | ||
| fs.removeSync(devtoolsDest) | ||
| fs.ensureDirSync(devtoolsDest) | ||
| fs.copySync(devtoolsSrc, devtoolsDest, { overwrite: true }) | ||
| console.warn('syncDevtools: copy complete') | ||
| } catch (error) { | ||
| console.error(`Failed to sync DevTools to ${devtoolsDest}:`, error) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [quality] medium Priority Issue: Error logging uses console.error but function continues execution after failure |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,15 +2,18 @@ | |
| <WidgetLayoutField :widget="widget"> | ||
| <FormSelectButton | ||
| v-model="localValue" | ||
| :options="widget.options?.values || []" | ||
| :options="selectOptions" | ||
| class="w-full" | ||
| @update:model-value="onChange" | ||
| /> | ||
| </WidgetLayoutField> | ||
| </template> | ||
|
|
||
| <script setup lang="ts"> | ||
| import { computed } from 'vue' | ||
|
|
||
| import { useStringWidgetValue } from '@/composables/graph/useWidgetValue' | ||
| import { isSelectButtonInputSpec } from '@/schemas/nodeDef/nodeDefSchemaV2' | ||
| import type { SimplifiedWidget } from '@/types/simplifiedWidget' | ||
|
|
||
| import FormSelectButton from './form/FormSelectButton.vue' | ||
|
|
@@ -31,4 +34,13 @@ const { localValue, onChange } = useStringWidgetValue( | |
| props.modelValue, | ||
| emit | ||
| ) | ||
|
|
||
| // Extract spec options directly | ||
| const selectOptions = computed(() => { | ||
| const spec = props.widget.spec | ||
| if (!spec || !isSelectButtonInputSpec(spec)) { | ||
| return [] | ||
| } | ||
| return spec.options?.values || [] | ||
|
||
| }) | ||
| </script> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,11 +20,8 @@ import { computed } from 'vue' | |
|
|
||
| import { useWidgetValue } from '@/composables/graph/useWidgetValue' | ||
| import { useTransformCompatOverlayProps } from '@/composables/useTransformCompatOverlayProps' | ||
| import { isTreeSelectInputSpec } from '@/schemas/nodeDef/nodeDefSchemaV2' | ||
| import type { SimplifiedWidget } from '@/types/simplifiedWidget' | ||
| import { | ||
| PANEL_EXCLUDED_PROPS, | ||
| filterWidgetProps | ||
| } from '@/utils/widgetPropFilter' | ||
|
|
||
| import WidgetLayoutField from './layout/WidgetLayoutField.vue' | ||
|
|
||
|
|
@@ -57,15 +54,29 @@ const { localValue, onChange } = useWidgetValue({ | |
| // Transform compatibility props for overlay positioning | ||
| const transformCompatProps = useTransformCompatOverlayProps() | ||
|
|
||
| // TreeSelect specific excluded props | ||
| const TREE_SELECT_EXCLUDED_PROPS = [ | ||
| ...PANEL_EXCLUDED_PROPS, | ||
| 'inputClass', | ||
| 'inputStyle' | ||
| ] as const | ||
| const combinedProps = computed(() => { | ||
| const spec = props.widget.spec | ||
| if (!spec || !isTreeSelectInputSpec(spec)) { | ||
| return { | ||
| ...props.widget.options, | ||
| ...transformCompatProps.value | ||
| } | ||
| } | ||
|
|
||
| const combinedProps = computed(() => ({ | ||
| ...filterWidgetProps(props.widget.options, TREE_SELECT_EXCLUDED_PROPS), | ||
| ...transformCompatProps.value | ||
| })) | ||
| const specOptions = spec.options || {} | ||
| return { | ||
| // Include runtime props like disabled | ||
| ...props.widget.options, | ||
| // PrimeVue TreeSelect expects 'options' to be an array of tree nodes | ||
| options: (specOptions.values as TreeNode[]) || [], | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [performance] medium Priority Issue: Missing memoization for expensive tree node casting |
||
| // Convert 'multiple' to PrimeVue's 'selectionMode' | ||
| selectionMode: (specOptions.multiple ? 'multiple' : 'single') as | ||
|
||
| | 'single' | ||
| | 'multiple' | ||
| | 'checkbox', | ||
| // Pass through other props like placeholder | ||
| placeholder: specOptions.placeholder, | ||
| ...transformCompatProps.value | ||
| } | ||
| }) | ||
| </script> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -45,7 +45,8 @@ const zColorInputSpec = zBaseInputOptions.extend({ | |
| isOptional: z.boolean().optional(), | ||
| options: z | ||
| .object({ | ||
| default: z.string().optional() | ||
| default: z.string().optional(), | ||
| format: z.enum(['hex', 'rgb', 'hsl', 'hsb']).optional() | ||
| }) | ||
| .optional() | ||
| }) | ||
|
|
@@ -54,7 +55,13 @@ const zFileUploadInputSpec = zBaseInputOptions.extend({ | |
| type: z.literal('FILEUPLOAD'), | ||
| name: z.string(), | ||
| isOptional: z.boolean().optional(), | ||
| options: z.record(z.unknown()).optional() | ||
| options: z | ||
| .object({ | ||
| accept: z.string().optional(), | ||
| extensions: z.array(z.string()).optional(), | ||
| tooltip: z.string().optional() | ||
| }) | ||
| .optional() | ||
| }) | ||
|
|
||
| const zImageInputSpec = zBaseInputOptions.extend({ | ||
|
|
@@ -89,7 +96,8 @@ const zTreeSelectInputSpec = zBaseInputOptions.extend({ | |
| options: z | ||
| .object({ | ||
| multiple: z.boolean().optional(), | ||
| values: z.array(z.unknown()).optional() | ||
| values: z.array(z.unknown()).optional(), | ||
| placeholder: z.string().optional() | ||
| }) | ||
| .optional() | ||
| }) | ||
|
|
@@ -123,7 +131,9 @@ const zGalleriaInputSpec = zBaseInputOptions.extend({ | |
| isOptional: z.boolean().optional(), | ||
| options: z | ||
| .object({ | ||
| images: z.array(z.string()).optional() | ||
| images: z.array(z.string()).optional(), | ||
| showThumbnails: z.boolean().optional(), | ||
| showItemNavigators: z.boolean().optional() | ||
| }) | ||
| .optional() | ||
| }) | ||
|
|
@@ -262,3 +272,39 @@ export const isChartInputSpec = ( | |
| ): inputSpec is ChartInputSpec => { | ||
| return inputSpec.type === 'CHART' | ||
| } | ||
|
|
||
| export const isTreeSelectInputSpec = ( | ||
| inputSpec: InputSpec | ||
| ): inputSpec is TreeSelectInputSpec => { | ||
| return inputSpec.type === 'TREESELECT' | ||
| } | ||
|
|
||
| export const isSelectButtonInputSpec = ( | ||
| inputSpec: InputSpec | ||
| ): inputSpec is SelectButtonInputSpec => { | ||
| return inputSpec.type === 'SELECTBUTTON' | ||
| } | ||
|
|
||
| export const isMultiSelectInputSpec = ( | ||
| inputSpec: InputSpec | ||
| ): inputSpec is MultiSelectInputSpec => { | ||
| return inputSpec.type === 'MULTISELECT' | ||
| } | ||
|
|
||
| export const isGalleriaInputSpec = ( | ||
| inputSpec: InputSpec | ||
| ): inputSpec is GalleriaInputSpec => { | ||
| return inputSpec.type === 'GALLERIA' | ||
| } | ||
|
|
||
| export const isColorInputSpec = ( | ||
| inputSpec: InputSpec | ||
| ): inputSpec is ColorInputSpec => { | ||
| return inputSpec.type === 'COLOR' | ||
| } | ||
|
|
||
| export const isFileUploadInputSpec = ( | ||
| inputSpec: InputSpec | ||
| ): inputSpec is FileUploadInputSpec => { | ||
| return inputSpec.type === 'FILEUPLOAD' | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [quality] medium Priority Issue: Missing type guard for ImageInputSpec - no matching isImageInputSpec function exported |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[security] high Priority
Issue: Directory traversal vulnerability - fs.copySync without path validation
Context: The devtoolsDest path is constructed from user input without validation, potentially allowing directory traversal attacks
Suggestion: Validate and sanitize the targetComfyDir path before using it in file operations. Use path.resolve() and check that the resolved path stays within expected boundaries