[feat] Allow opening workflows with same name as new tabs #7104
Conversation
When importing a workflow file that has the same name as an existing open workflow, create a new tab with a unique suffix (2), (3), etc. instead of silently failing or replacing the existing workflow. - Add forceNew parameter to createTemporary() in workflowStore - Always create new temporary workflow when importing via file picker - Remove logic that looked up persisted workflows by name during import
dosubot
bot
added
the
size:M
📝 WalkthroughWalkthroughPrefers reopening an existing persisted workflow by path during restoration (resetting it and restoring its changeTracker); if none matches, creates a fresh temporary workflow via Changes
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 12/03/2025, 02:40:29 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Test Results⏰ Completed at: 12/03/2025, 02:48:53 AM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.21 MB (baseline 3.21 MB) • 🔴 +563 BMain entry bundles and manifests
Status: 3 added / 3 removed Graph Workspace — 929 kB (baseline 929 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 — 169 kB (baseline 169 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: 0
🧹 Nitpick comments (1)
src/platform/workflow/management/stores/workflowStore.ts (1)
368-372:createTemporaryforceNew flag + conflict-free paths look correct; consider documenting the new paramThe new
forceNewparameter, combined withgetUnconflictedPath, cleanly separates behaviors:
forceNew === true: always creates a freshComfyWorkflowwith a unique path underComfyWorkflow.basePath, enabling multiple open tabs with the same base filename without clobberingworkflowLookup.forceNew === false: preserves the prior reuse path by locating an existing workflow byfullFilenameand, when it’s a non-workflows/directory with achangeTracker, resetting that tracker withworkflowDataand returning the existing instance.This matches the PR goal while keeping older call sites' semantics intact.
Two small follow-ups you may want to consider:
- Update the local
WorkflowStoreinterface’screateTemporarysignature to include the optionalforceNew?: booleanparameter so any consumers typed againstWorkflowStore(rather thanReturnType<typeof useWorkflowStore>) get accurate IntelliSense and avoid confusion.- Add a focused test around
createTemporarywithforceNew: trueto assert that repeated calls with the samepathyield distinctpaths (e.g.foo.json,foo (2).json) and don’t reuse the same workflow instance.Also applies to: 376-390
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/platform/workflow/core/services/workflowService.ts(2 hunks)src/platform/workflow/management/stores/workflowStore.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (14)
**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{vue,ts,tsx}: Leverage VueUse functions for performance-enhancing utilities
Use vue-i18n in Composition API for any string literals and place new translation entries in src/locales/en/main.json
Files:
src/platform/workflow/management/stores/workflowStore.tssrc/platform/workflow/core/services/workflowService.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursorrules)
Use es-toolkit for utility functions
Files:
src/platform/workflow/management/stores/workflowStore.tssrc/platform/workflow/core/services/workflowService.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Use TypeScript for type safety
**/*.{ts,tsx}: Never useanytype - use proper TypeScript types
Never useas anytype assertions - fix the underlying type issue
Files:
src/platform/workflow/management/stores/workflowStore.tssrc/platform/workflow/core/services/workflowService.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (.cursorrules)
Implement proper error handling in components and services
**/*.{ts,tsx,js,vue}: Use 2-space indentation, single quotes, no semicolons, and maintain 80-character line width as configured in.prettierrc
Organize imports by sorting and grouping by plugin, and runpnpm formatbefore committing
Files:
src/platform/workflow/management/stores/workflowStore.tssrc/platform/workflow/core/services/workflowService.ts
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/platform/workflow/management/stores/workflowStore.tssrc/platform/workflow/core/services/workflowService.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/platform/workflow/management/stores/workflowStore.tssrc/platform/workflow/core/services/workflowService.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use camelCase for variable and setting names in TypeScript/Vue files
Files:
src/platform/workflow/management/stores/workflowStore.tssrc/platform/workflow/core/services/workflowService.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,vue}: Useconst settingStore = useSettingStore()andsettingStore.get('Comfy.SomeSetting')to retrieve settings in TypeScript/Vue files
Useawait settingStore.set('Comfy.SomeSetting', newValue)to update settings in TypeScript/Vue files
Check server capabilities usingapi.serverSupportsFeature('feature_name')before using enhanced features
Useapi.getServerFeature('config_name', defaultValue)to retrieve server feature configurationEnforce ESLint rules for Vue + TypeScript including: no floating promises, no unused imports, and i18n raw text restrictions in templates
Files:
src/platform/workflow/management/stores/workflowStore.tssrc/platform/workflow/core/services/workflowService.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Define dynamic setting defaults using runtime context with functions in settings configuration
UsedefaultsByInstallVersionproperty for gradual feature rollout based on version in settings configuration
Files:
src/platform/workflow/management/stores/workflowStore.tssrc/platform/workflow/core/services/workflowService.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/platform/workflow/management/stores/workflowStore.tssrc/platform/workflow/core/services/workflowService.ts
src/**/stores/**/*.{ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/stores/**/*.{ts,tsx}: Maintain clear public interfaces and restrict extension access in stores
Use TypeScript for type safety in state management stores
Files:
src/platform/workflow/management/stores/workflowStore.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/platform/workflow/management/stores/workflowStore.tssrc/platform/workflow/core/services/workflowService.ts
**/stores/*Store.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name Pinia stores with the
*Store.tssuffix
Files:
src/platform/workflow/management/stores/workflowStore.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/platform/workflow/core/services/workflowService.ts
🧠 Learnings (3)
📚 Learning: 2025-11-24T19:47:14.779Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:14.779Z
Learning: Applies to **/*.{ts,tsx,vue} : Use `const settingStore = useSettingStore()` and `settingStore.get('Comfy.SomeSetting')` to retrieve settings in TypeScript/Vue files
Applied to files:
src/platform/workflow/core/services/workflowService.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{test,spec}.{ts,tsx} : When writing tests for subgraph-related code, always import from the barrel export at `@/lib/litegraph/src/litegraph` to avoid circular dependency issues
Applied to files:
src/platform/workflow/core/services/workflowService.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{test,spec}.{ts,tsx} : Use provided test helpers `createTestSubgraph` and `createTestSubgraphNode` from `./fixtures/subgraphHelpers` for consistent subgraph test setup
Applied to files:
src/platform/workflow/core/services/workflowService.ts
🧬 Code graph analysis (1)
src/platform/workflow/management/stores/workflowStore.ts (2)
src/stores/userFileStore.ts (1)
createTemporary(64-66)src/platform/workflow/validation/schemas/workflowSchema.ts (1)
ComfyWorkflowJSON(456-458)
🪛 ESLint
src/platform/workflow/core/services/workflowService.ts
[error] 10-10: Unable to resolve path to module '@/platform/workflow/management/stores/workflowStore'.
(import-x/no-unresolved)
⏰ 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: test
- GitHub Check: lint-and-format
- GitHub Check: setup
- GitHub Check: collect
🔇 Additional comments (2)
src/platform/workflow/core/services/workflowService.ts (2)
9-10: Type-only ComfyWorkflow import looks good; check ESLint resolver configSwitching to
import type { ComfyWorkflow }while keepinguseWorkflowStoreas a value import is correct and avoids pulling the class into the runtime bundle unnecessarily. Static analysis, however, reportsimport-x/no-unresolvedfor this module path; please confirm that the ESLint import resolver is configured to understand the@alias and that the path matches the actual file location sopnpm lintpasses cleanly.
315-319: Force creation of a new temporary workflow for string/null loadsRouting the
value === null || typeof value === 'string'case throughworkflowStore.createTemporary(path ? appendJsonExt(path) : undefined, workflowData, true)ensures that imports and other string-based loads always get a fresh temporary workflow with a conflict-free path, which aligns with the “multiple tabs with same name” goal. The use ofappendJsonExtis also consistent with save paths elsewhere.Please sanity-check the flows that hit this branch (e.g. file picker imports,
loadApiJson,importA1111, andduplicateWorkflow) to confirm they now open separate tabs with suffixes instead of reusing or overwriting an existing one.
When importing a workflow file that has the same name as an existing open workflow, create a new tab with a unique suffix (2), (3), etc. instead of silently failing or replacing the existing workflow. - Add forceNew parameter to createTemporary() in workflowStore - Always create new temporary workflow when importing via file picker - Remove logic that looked up persisted workflows by name during import
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/platform/workflow/core/services/workflowService.ts (1)
333-337: Consider adding clarity for the boolean parameter.The
trueargument representsforceNewper the PR changes. While the implementation is correct, consider improving readability:const tempWorkflow = workflowStore.createTemporary( path ? appendJsonExt(path) : undefined, workflowData, - true + /* forceNew */ true )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/platform/workflow/core/services/workflowService.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{vue,ts,tsx}: Leverage VueUse functions for performance-enhancing utilities
Use vue-i18n in Composition API for any string literals and place new translation entries in src/locales/en/main.json
Files:
src/platform/workflow/core/services/workflowService.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursorrules)
Use es-toolkit for utility functions
Files:
src/platform/workflow/core/services/workflowService.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Use TypeScript for type safety
**/*.{ts,tsx}: Never useanytype - use proper TypeScript types
Never useas anytype assertions - fix the underlying type issue
Files:
src/platform/workflow/core/services/workflowService.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (.cursorrules)
Implement proper error handling in components and services
**/*.{ts,tsx,js,vue}: Use 2-space indentation, single quotes, no semicolons, and maintain 80-character line width as configured in.prettierrc
Organize imports by sorting and grouping by plugin, and runpnpm formatbefore committing
Files:
src/platform/workflow/core/services/workflowService.ts
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/platform/workflow/core/services/workflowService.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/platform/workflow/core/services/workflowService.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use camelCase for variable and setting names in TypeScript/Vue files
Files:
src/platform/workflow/core/services/workflowService.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,vue}: Useconst settingStore = useSettingStore()andsettingStore.get('Comfy.SomeSetting')to retrieve settings in TypeScript/Vue files
Useawait settingStore.set('Comfy.SomeSetting', newValue)to update settings in TypeScript/Vue files
Check server capabilities usingapi.serverSupportsFeature('feature_name')before using enhanced features
Useapi.getServerFeature('config_name', defaultValue)to retrieve server feature configurationEnforce ESLint rules for Vue + TypeScript including: no floating promises, no unused imports, and i18n raw text restrictions in templates
Files:
src/platform/workflow/core/services/workflowService.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Define dynamic setting defaults using runtime context with functions in settings configuration
UsedefaultsByInstallVersionproperty for gradual feature rollout based on version in settings configuration
Files:
src/platform/workflow/core/services/workflowService.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/platform/workflow/core/services/workflowService.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/platform/workflow/core/services/workflowService.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/platform/workflow/core/services/workflowService.ts
🧠 Learnings (3)
📚 Learning: 2025-11-24T19:47:14.779Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:14.779Z
Learning: Applies to **/*.{ts,tsx,vue} : Use `const settingStore = useSettingStore()` and `settingStore.get('Comfy.SomeSetting')` to retrieve settings in TypeScript/Vue files
Applied to files:
src/platform/workflow/core/services/workflowService.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{test,spec}.{ts,tsx} : When writing tests for subgraph-related code, always import from the barrel export at `@/lib/litegraph/src/litegraph` to avoid circular dependency issues
Applied to files:
src/platform/workflow/core/services/workflowService.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{test,spec}.{ts,tsx} : Use provided test helpers `createTestSubgraph` and `createTestSubgraphNode` from `./fixtures/subgraphHelpers` for consistent subgraph test setup
Applied to files:
src/platform/workflow/core/services/workflowService.ts
🪛 ESLint
src/platform/workflow/core/services/workflowService.ts
[error] 10-10: Unable to resolve path to module '@/platform/workflow/management/stores/workflowStore'.
(import-x/no-unresolved)
🪛 GitHub Actions: CI: Size Data
src/platform/workflow/core/services/workflowService.ts
[error] 318-318: TS1361: 'ComfyWorkflow' cannot be used as a value because it was imported using 'import type'.
🪛 GitHub Check: collect
src/platform/workflow/core/services/workflowService.ts
[failure] 318-318:
'ComfyWorkflow' cannot be used as a value because it was imported using 'import type'.
⏰ 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: deploy-and-comment
- GitHub Check: lint-and-format
- GitHub Check: setup
- GitHub Check: test
🔇 Additional comments (1)
src/platform/workflow/core/services/workflowService.ts (1)
315-331: Logic for workflow restoration vs. re-import looks correct.The conditional properly distinguishes between:
- Restoration case (persisted but not loaded) → reuse existing workflow
- Re-import case (already loaded) → fall through to create new tab with suffix
Once the import issue on line 9 is fixed, this logic should work as intended.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/platform/workflow/core/services/workflowService.ts (1)
318-333: Consider adding error handling for the restoration flow.The path-based restoration logic correctly implements the PR objective. However, the async operations (
openWorkflow,reset,restore) could fail and there's no error handling to gracefully recover or inform the user.Consider wrapping the restoration flow in a try-catch block:
if (existingWorkflow?.isPersisted && !existingWorkflow.isLoaded) { + try { const loadedWorkflow = await workflowStore.openWorkflow(existingWorkflow) loadedWorkflow.changeTracker.reset(workflowData) loadedWorkflow.changeTracker.restore() return + } catch (error) { + console.error('Failed to restore workflow:', error) + // Fall through to create new temporary workflow + } }Based on coding guidelines requiring proper error handling in services.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/platform/workflow/core/services/workflowService.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{vue,ts,tsx}: Leverage VueUse functions for performance-enhancing utilities
Use vue-i18n in Composition API for any string literals and place new translation entries in src/locales/en/main.json
Files:
src/platform/workflow/core/services/workflowService.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursorrules)
Use es-toolkit for utility functions
Files:
src/platform/workflow/core/services/workflowService.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Use TypeScript for type safety
**/*.{ts,tsx}: Never useanytype - use proper TypeScript types
Never useas anytype assertions - fix the underlying type issue
Files:
src/platform/workflow/core/services/workflowService.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (.cursorrules)
Implement proper error handling in components and services
**/*.{ts,tsx,js,vue}: Use 2-space indentation, single quotes, no semicolons, and maintain 80-character line width as configured in.prettierrc
Organize imports by sorting and grouping by plugin, and runpnpm formatbefore committing
Files:
src/platform/workflow/core/services/workflowService.ts
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/platform/workflow/core/services/workflowService.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/platform/workflow/core/services/workflowService.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use camelCase for variable and setting names in TypeScript/Vue files
Files:
src/platform/workflow/core/services/workflowService.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,vue}: Useconst settingStore = useSettingStore()andsettingStore.get('Comfy.SomeSetting')to retrieve settings in TypeScript/Vue files
Useawait settingStore.set('Comfy.SomeSetting', newValue)to update settings in TypeScript/Vue files
Check server capabilities usingapi.serverSupportsFeature('feature_name')before using enhanced features
Useapi.getServerFeature('config_name', defaultValue)to retrieve server feature configurationEnforce ESLint rules for Vue + TypeScript including: no floating promises, no unused imports, and i18n raw text restrictions in templates
Files:
src/platform/workflow/core/services/workflowService.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Define dynamic setting defaults using runtime context with functions in settings configuration
UsedefaultsByInstallVersionproperty for gradual feature rollout based on version in settings configuration
Files:
src/platform/workflow/core/services/workflowService.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/platform/workflow/core/services/workflowService.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/platform/workflow/core/services/workflowService.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/platform/workflow/core/services/workflowService.ts
🧠 Learnings (1)
📚 Learning: 2025-11-24T19:47:14.779Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:14.779Z
Learning: Applies to **/*.{ts,tsx} : Never use `as any` type assertions - fix the underlying type issue
Applied to files:
src/platform/workflow/core/services/workflowService.ts
🧬 Code graph analysis (1)
src/platform/workflow/core/services/workflowService.ts (2)
src/platform/workflow/management/stores/workflowStore.ts (1)
ComfyWorkflow(35-139)packages/shared-frontend-utils/src/formatUtil.ts (1)
appendJsonExt(28-33)
⏰ 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). (3)
- GitHub Check: test
- GitHub Check: setup
- GitHub Check: lint-and-format
🔇 Additional comments (1)
src/platform/workflow/core/services/workflowService.ts (1)
9-12: Import fix correctly resolves the previous type-only import issue.The runtime import of
ComfyWorkflownow properly supports both type annotations and access to the staticbasePathproperty at line 320, addressing the TS1361 error flagged in the previous review.
…n parameter Replace boolean parameter with two distinct methods (createTemporary and createNewTemporary) to make intent explicit and improve code readability. This addresses code review feedback about avoiding bare boolean arguments as function parameters.
dosubot
bot
added
size:L
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/platform/workflow/management/stores/workflowStore.ts (1)
401-418: Use strict equality and clarify the comment.Two minor suggestions:
- Line 404: Use
===instead of==for strict equality comparison- The comment "Try to reuse existing persisted workflow" is misleading — the condition actually reuses loaded workflows that are not in the workflows directory
- // Try to reuse existing persisted workflow with the same filename + // Try to reuse an existing loaded workflow with the same filename + // that is not stored in the workflows directory if (path && workflowData) { const existingWorkflow = workflows.value.find( - (w) => w.fullFilename == path + (w) => w.fullFilename === path )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/platform/workflow/core/services/workflowService.ts(1 hunks)src/platform/workflow/management/stores/workflowStore.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (14)
**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{vue,ts,tsx}: Leverage VueUse functions for performance-enhancing utilities
Use vue-i18n in Composition API for any string literals and place new translation entries in src/locales/en/main.json
Files:
src/platform/workflow/management/stores/workflowStore.tssrc/platform/workflow/core/services/workflowService.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursorrules)
Use es-toolkit for utility functions
Files:
src/platform/workflow/management/stores/workflowStore.tssrc/platform/workflow/core/services/workflowService.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Use TypeScript for type safety
**/*.{ts,tsx}: Never useanytype - use proper TypeScript types
Never useas anytype assertions - fix the underlying type issue
Files:
src/platform/workflow/management/stores/workflowStore.tssrc/platform/workflow/core/services/workflowService.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (.cursorrules)
Implement proper error handling in components and services
**/*.{ts,tsx,js,vue}: Use 2-space indentation, single quotes, no semicolons, and maintain 80-character line width as configured in.prettierrc
Organize imports by sorting and grouping by plugin, and runpnpm formatbefore committing
Files:
src/platform/workflow/management/stores/workflowStore.tssrc/platform/workflow/core/services/workflowService.ts
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/platform/workflow/management/stores/workflowStore.tssrc/platform/workflow/core/services/workflowService.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/platform/workflow/management/stores/workflowStore.tssrc/platform/workflow/core/services/workflowService.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use camelCase for variable and setting names in TypeScript/Vue files
Files:
src/platform/workflow/management/stores/workflowStore.tssrc/platform/workflow/core/services/workflowService.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,vue}: Useconst settingStore = useSettingStore()andsettingStore.get('Comfy.SomeSetting')to retrieve settings in TypeScript/Vue files
Useawait settingStore.set('Comfy.SomeSetting', newValue)to update settings in TypeScript/Vue files
Check server capabilities usingapi.serverSupportsFeature('feature_name')before using enhanced features
Useapi.getServerFeature('config_name', defaultValue)to retrieve server feature configurationEnforce ESLint rules for Vue + TypeScript including: no floating promises, no unused imports, and i18n raw text restrictions in templates
Files:
src/platform/workflow/management/stores/workflowStore.tssrc/platform/workflow/core/services/workflowService.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Define dynamic setting defaults using runtime context with functions in settings configuration
UsedefaultsByInstallVersionproperty for gradual feature rollout based on version in settings configuration
Files:
src/platform/workflow/management/stores/workflowStore.tssrc/platform/workflow/core/services/workflowService.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/platform/workflow/management/stores/workflowStore.tssrc/platform/workflow/core/services/workflowService.ts
src/**/stores/**/*.{ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/stores/**/*.{ts,tsx}: Maintain clear public interfaces and restrict extension access in stores
Use TypeScript for type safety in state management stores
Files:
src/platform/workflow/management/stores/workflowStore.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/platform/workflow/management/stores/workflowStore.tssrc/platform/workflow/core/services/workflowService.ts
**/stores/*Store.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name Pinia stores with the
*Store.tssuffix
Files:
src/platform/workflow/management/stores/workflowStore.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/platform/workflow/core/services/workflowService.ts
🧬 Code graph analysis (1)
src/platform/workflow/management/stores/workflowStore.ts (3)
src/platform/workflow/validation/schemas/workflowSchema.ts (1)
ComfyWorkflowJSON(456-458)src/stores/queueStore.ts (1)
workflow(314-316)src/stores/userFileStore.ts (1)
createTemporary(64-66)
⏰ 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). (2)
- GitHub Check: setup
- GitHub Check: test
🔇 Additional comments (4)
src/platform/workflow/management/stores/workflowStore.ts (3)
175-178: LGTM!The
createNewTemporarymethod signature is correctly added to theWorkflowStoreinterface, matching the implementation signature.
372-391: LGTM!Good refactoring to centralize workflow creation logic into a dedicated helper function. This reduces code duplication between
createTemporaryandcreateNewTemporary.
420-431: LGTM!The
createNewTemporarymethod correctly implements the PR objective by always creating a new workflow with a unique path suffix, without attempting to reuse existing workflows. This is the right approach for the import use case.src/platform/workflow/core/services/workflowService.ts (1)
317-341: LGTM! Clear logic for distinguishing restoration vs re-import cases.The implementation correctly handles the two scenarios:
- Restoration case (persisted but not loaded): Reuses the existing workflow and restores its change tracker state
- Re-import case (already loaded or not persisted): Creates a new temporary workflow with a unique suffix via
createNewTemporaryThis aligns well with the PR objective of allowing workflows with the same name to be opened as new tabs.
…ory and use strict equality
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/platform/workflow/management/stores/workflowStore.ts (1)
372-432: Temporary workflow creation/reuse logic meets requirements; consider tightening reuse predicateThe refactor into
createNewWorkflowplusgetUnconflictedPathnicely centralizes temporary workflow instantiation and guarantees unique paths for conflicting names, which aligns with the “(2), (3), …” tab requirement.In
createTemporary, the reuse branch:
- looks up by
fullFilename,- requires a
changeTracker,- and excludes items whose
directorystarts with theworkflowsbase.This will happily reuse any loaded
ComfyWorkflowwhose directory does not start with'workflows', which is fine if and only if those are truly temporary workflows you want to overwrite (and not, say, persisted subgraph files that also useComfyWorkflow). If that’s not guaranteed, it would be safer to additionally require the candidate to be temporary and/or explicitly scoped under the expected base path.Also, note that reuse only occurs when both
pathandworkflowDataare provided; any caller passing justpathwill now always get a new workflow. Please double‑check that all intended reuse call sites do passworkflowData.If you want to tighten the reuse guard and avoid unnecessary
getUnconflictedPathwork, something like this keeps the same behavior but narrows the match:- const createTemporary = (path?: string, workflowData?: ComfyWorkflowJSON) => { - const fullPath = getUnconflictedPath( - ComfyWorkflow.basePath + (path ?? 'Unsaved Workflow.json') - ) - - // Try to reuse an existing loaded workflow with the same filename - // that is not stored in the workflows directory - if (path && workflowData) { + const createTemporary = (path?: string, workflowData?: ComfyWorkflowJSON) => { + // Try to reuse an existing loaded temporary workflow with the same filename + if (path && workflowData) { const existingWorkflow = workflows.value.find( (w) => w.fullFilename === path ) if ( existingWorkflow?.changeTracker && + existingWorkflow.isTemporary && !existingWorkflow.directory.startsWith( ComfyWorkflow.basePath.slice(0, -1) ) ) { existingWorkflow.changeTracker.reset(workflowData) return existingWorkflow } } - return createNewWorkflow(fullPath, workflowData) + const fullPath = getUnconflictedPath( + ComfyWorkflow.basePath + (path ?? 'Unsaved Workflow.json') + ) + return createNewWorkflow(fullPath, workflowData) }This keeps the new behavior but ensures you only ever reuse loaded temporary workflows and avoids computing an unconflicted path when you’re going to reuse an existing instance anyway.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/platform/workflow/management/stores/workflowStore.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (13)
**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{vue,ts,tsx}: Leverage VueUse functions for performance-enhancing utilities
Use vue-i18n in Composition API for any string literals and place new translation entries in src/locales/en/main.json
Files:
src/platform/workflow/management/stores/workflowStore.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursorrules)
Use es-toolkit for utility functions
Files:
src/platform/workflow/management/stores/workflowStore.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Use TypeScript for type safety
**/*.{ts,tsx}: Never useanytype - use proper TypeScript types
Never useas anytype assertions - fix the underlying type issue
Files:
src/platform/workflow/management/stores/workflowStore.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (.cursorrules)
Implement proper error handling in components and services
**/*.{ts,tsx,js,vue}: Use 2-space indentation, single quotes, no semicolons, and maintain 80-character line width as configured in.prettierrc
Organize imports by sorting and grouping by plugin, and runpnpm formatbefore committing
Files:
src/platform/workflow/management/stores/workflowStore.ts
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/platform/workflow/management/stores/workflowStore.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/platform/workflow/management/stores/workflowStore.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use camelCase for variable and setting names in TypeScript/Vue files
Files:
src/platform/workflow/management/stores/workflowStore.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,vue}: Useconst settingStore = useSettingStore()andsettingStore.get('Comfy.SomeSetting')to retrieve settings in TypeScript/Vue files
Useawait settingStore.set('Comfy.SomeSetting', newValue)to update settings in TypeScript/Vue files
Check server capabilities usingapi.serverSupportsFeature('feature_name')before using enhanced features
Useapi.getServerFeature('config_name', defaultValue)to retrieve server feature configurationEnforce ESLint rules for Vue + TypeScript including: no floating promises, no unused imports, and i18n raw text restrictions in templates
Files:
src/platform/workflow/management/stores/workflowStore.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Define dynamic setting defaults using runtime context with functions in settings configuration
UsedefaultsByInstallVersionproperty for gradual feature rollout based on version in settings configuration
Files:
src/platform/workflow/management/stores/workflowStore.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/platform/workflow/management/stores/workflowStore.ts
src/**/stores/**/*.{ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/stores/**/*.{ts,tsx}: Maintain clear public interfaces and restrict extension access in stores
Use TypeScript for type safety in state management stores
Files:
src/platform/workflow/management/stores/workflowStore.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/platform/workflow/management/stores/workflowStore.ts
**/stores/*Store.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name Pinia stores with the
*Store.tssuffix
Files:
src/platform/workflow/management/stores/workflowStore.ts
🧬 Code graph analysis (1)
src/platform/workflow/management/stores/workflowStore.ts (1)
src/stores/userFileStore.ts (1)
createTemporary(64-66)
⏰ 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). (5)
- GitHub Check: deploy-and-comment
- GitHub Check: test
- GitHub Check: lint-and-format
- GitHub Check: collect
- GitHub Check: setup
🔇 Additional comments (2)
src/platform/workflow/management/stores/workflowStore.ts (2)
171-178: NewcreateNewTemporaryinterface entry is consistentThe
WorkflowStoreinterface now explicitly advertisescreateNewTemporarywith the same signature as the implementation and returned store, which cleanly separates "maybe reuse" (createTemporary) from "always new" usage. No issues here.
802-816:createNewTemporaryis correctly exposed and already being used appropriatelyThe distinction between
createTemporary(with reuse logic for same-filename workflows not in the workflows directory) andcreateNewTemporary(always creates new) is intentional and well-documented. Current call sites are using the correct function:workflowService.tsusescreateNewTemporaryfor imports/file-picker flows where always-new semantics are required, whileworkflowActionsService.tsanduseJobMenu.tsusecreateTemporarywhere session-level reuse is appropriate. No migration is needed—the separation of concerns is properly implemented.
When importing a workflow file that has the same name as an existing open workflow, create a new tab with a unique suffix (2), (3), etc. instead of silently failing or replacing the existing workflow.
Screen.Recording.2025-12-02.at.22.03.48.mov
┆Issue is synchronized with this Notion page by Unito