Compare commits

..

1 Commits

Author SHA1 Message Date
CodeRabbit Fixer
de6a476aba fix: Implement atomic workflow overwrite via persistence-level flag (#9355)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-03-06 16:26:27 +01:00
14 changed files with 205 additions and 217 deletions

View File

@@ -51,17 +51,6 @@ export function createMonotoneInterpolator(
}
}
const segCount = n - 1
const segDx = new Float64Array(segCount)
const segM0dx = new Float64Array(segCount)
const segM1dx = new Float64Array(segCount)
for (let i = 0; i < segCount; i++) {
const dx = xs[i + 1] - xs[i]
segDx[i] = dx
segM0dx[i] = slopes[i] * dx
segM1dx[i] = slopes[i + 1] * dx
}
return (x: number): number => {
if (x <= xs[0]) return ys[0]
if (x >= xs[n - 1]) return ys[n - 1]
@@ -74,7 +63,7 @@ export function createMonotoneInterpolator(
else hi = mid
}
const dx = segDx[lo]
const dx = xs[hi] - xs[lo]
if (dx === 0) return ys[lo]
const t = (x - xs[lo]) / dx
@@ -86,7 +75,12 @@ export function createMonotoneInterpolator(
const h01 = -2 * t3 + 3 * t2
const h11 = t3 - t2
return h00 * ys[lo] + h10 * segM0dx[lo] + h01 * ys[hi] + h11 * segM1dx[lo]
return (
h00 * ys[lo] +
h10 * dx * slopes[lo] +
h01 * ys[hi] +
h11 * dx * slopes[hi]
)
}
}

View File

@@ -905,14 +905,6 @@ export function useCoreCommands(): ComfyCommand[] {
app.canvas.pasteFromClipboard()
}
},
{
id: 'Comfy.Canvas.PasteFromClipboardWithConnect',
icon: 'icon-[lucide--clipboard-paste]',
label: () => t('Paste with Connect'),
function: () => {
app.canvas.pasteFromClipboard({ connectInputs: true })
}
},
{
id: 'Comfy.Canvas.SelectAll',
icon: 'icon-[lucide--lasso-select]',
@@ -927,12 +919,6 @@ export function useCoreCommands(): ComfyCommand[] {
label: 'Delete Selected Items',
versionAdded: '1.10.5',
function: () => {
if (app.canvas.selectedItems.size === 0) {
app.canvas.canvas.dispatchEvent(
new CustomEvent('litegraph:no-items-selected', { bubbles: true })
)
return
}
app.canvas.deleteSelected()
app.canvas.setDirty(true, true)
}

View File

@@ -189,10 +189,11 @@ export function useWorkflowActionsMenu(
addItem({
id: 'share',
label: t('breadcrumbsMenu.share'),
label: t('menuLabels.Share'),
icon: 'icon-[comfy--send]',
command: async () => {},
visible: false
disabled: true,
visible: isRoot
})
addItem({

View File

@@ -3791,6 +3791,13 @@ export class LGraphCanvas implements CustomEventDispatcher<LGraphCanvasEventMap>
return
}
private _noItemsSelected(): void {
const event = new CustomEvent('litegraph:no-items-selected', {
bubbles: true
})
this.canvas.dispatchEvent(event)
}
/**
* process a key event
*/
@@ -3835,6 +3842,31 @@ export class LGraphCanvas implements CustomEventDispatcher<LGraphCanvasEventMap>
this.node_panel?.close()
this.options_panel?.close()
if (this.node_panel || this.options_panel) block_default = true
} else if (e.keyCode === 65 && e.ctrlKey) {
// select all Control A
this.selectItems()
block_default = true
} else if (e.keyCode === 67 && (e.metaKey || e.ctrlKey) && !e.shiftKey) {
// copy
if (this.selected_nodes) {
this.copyToClipboard()
block_default = true
}
} else if (e.keyCode === 86 && (e.metaKey || e.ctrlKey)) {
// paste
this.pasteFromClipboard({ connectInputs: e.shiftKey })
} else if (e.key === 'Delete' || e.key === 'Backspace') {
// delete or backspace
// @ts-expect-error EventTarget.localName is not in standard types
if (e.target.localName != 'input' && e.target.localName != 'textarea') {
if (this.selectedItems.size === 0) {
this._noItemsSelected()
return
}
this.deleteSelected()
block_default = true
}
}
// TODO

View File

@@ -1262,7 +1262,6 @@
"Move Selected Nodes Right": "Move Selected Nodes Right",
"Move Selected Nodes Up": "Move Selected Nodes Up",
"Paste": "Paste",
"Paste with Connect": "Paste with Connect",
"Reset View": "Reset View",
"Resize Selected Nodes": "Resize Selected Nodes",
"Select All": "Select All",
@@ -2604,8 +2603,7 @@
"deleteWorkflow": "Delete Workflow",
"deleteBlueprint": "Delete Blueprint",
"enterNewName": "Enter new name",
"missingNodesWarning": "Workflow contains unsupported nodes (highlighted red).",
"share": "Share"
"missingNodesWarning": "Workflow contains unsupported nodes (highlighted red)."
},
"shortcuts": {
"shortcuts": "Shortcuts",

View File

@@ -208,52 +208,5 @@ export const CORE_KEYBINDINGS: Keybinding[] = [
key: 'Escape'
},
commandId: 'Comfy.Graph.ExitSubgraph'
},
{
combo: {
ctrl: true,
key: 'a'
},
commandId: 'Comfy.Canvas.SelectAll',
targetElementId: 'graph-canvas-container'
},
{
combo: {
ctrl: true,
key: 'c'
},
commandId: 'Comfy.Canvas.CopySelected',
targetElementId: 'graph-canvas-container'
},
{
combo: {
ctrl: true,
key: 'v'
},
commandId: 'Comfy.Canvas.PasteFromClipboard',
targetElementId: 'graph-canvas-container'
},
{
combo: {
ctrl: true,
shift: true,
key: 'v'
},
commandId: 'Comfy.Canvas.PasteFromClipboardWithConnect',
targetElementId: 'graph-canvas-container'
},
{
combo: {
key: 'Delete'
},
commandId: 'Comfy.Canvas.DeleteSelectedItems',
targetElementId: 'graph-canvas-container'
},
{
combo: {
key: 'Backspace'
},
commandId: 'Comfy.Canvas.DeleteSelectedItems',
targetElementId: 'graph-canvas-container'
}
]

View File

@@ -1,11 +1,22 @@
import { createTestingPinia } from '@pinia/testing'
import { setActivePinia } from 'pinia'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { useKeybindingService } from '@/platform/keybindings/keybindingService'
import { app } from '@/scripts/app'
import { useCommandStore } from '@/stores/commandStore'
import { useDialogStore } from '@/stores/dialogStore'
vi.mock('@/scripts/app', () => {
return {
app: {
canvas: {
processKey: vi.fn()
}
}
}
})
vi.mock('@/platform/settings/settingStore', () => ({
useSettingStore: vi.fn(() => ({
get: vi.fn(() => [])
@@ -25,15 +36,13 @@ function createTestKeyboardEvent(
ctrlKey?: boolean
altKey?: boolean
metaKey?: boolean
shiftKey?: boolean
} = {}
): KeyboardEvent {
const {
target = document.body,
ctrlKey = false,
altKey = false,
metaKey = false,
shiftKey = false
metaKey = false
} = options
const event = new KeyboardEvent('keydown', {
@@ -41,7 +50,6 @@ function createTestKeyboardEvent(
ctrlKey,
altKey,
metaKey,
shiftKey,
bubbles: true,
cancelable: true
})
@@ -52,10 +60,8 @@ function createTestKeyboardEvent(
return event
}
describe('keybindingService - Canvas Keybindings', () => {
describe('keybindingService - Event Forwarding', () => {
let keybindingService: ReturnType<typeof useKeybindingService>
let canvasContainer: HTMLDivElement
let canvasChild: HTMLCanvasElement
beforeEach(() => {
vi.clearAllMocks()
@@ -70,156 +76,94 @@ describe('keybindingService - Canvas Keybindings', () => {
typeof useDialogStore
>)
canvasContainer = document.createElement('div')
canvasContainer.id = 'graph-canvas-container'
canvasChild = document.createElement('canvas')
canvasContainer.appendChild(canvasChild)
document.body.appendChild(canvasContainer)
keybindingService = useKeybindingService()
keybindingService.registerCoreKeybindings()
})
afterEach(() => {
canvasContainer.remove()
})
it('should execute DeleteSelectedItems for Delete key on canvas', async () => {
const event = createTestKeyboardEvent('Delete', {
target: canvasChild
})
it('should forward Delete key to canvas when no keybinding exists', async () => {
const event = createTestKeyboardEvent('Delete')
await keybindingService.keybindHandler(event)
expect(vi.mocked(useCommandStore().execute)).toHaveBeenCalledWith(
'Comfy.Canvas.DeleteSelectedItems'
)
expect(vi.mocked(app.canvas.processKey)).toHaveBeenCalledWith(event)
expect(vi.mocked(useCommandStore().execute)).not.toHaveBeenCalled()
})
it('should execute DeleteSelectedItems for Backspace key on canvas', async () => {
const event = createTestKeyboardEvent('Backspace', {
target: canvasChild
})
it('should forward Backspace key to canvas when no keybinding exists', async () => {
const event = createTestKeyboardEvent('Backspace')
await keybindingService.keybindHandler(event)
expect(vi.mocked(useCommandStore().execute)).toHaveBeenCalledWith(
'Comfy.Canvas.DeleteSelectedItems'
)
expect(vi.mocked(app.canvas.processKey)).toHaveBeenCalledWith(event)
expect(vi.mocked(useCommandStore().execute)).not.toHaveBeenCalled()
})
it('should not execute DeleteSelectedItems when typing in input field', async () => {
it('should not forward Delete key when typing in input field', async () => {
const inputElement = document.createElement('input')
const event = createTestKeyboardEvent('Delete', { target: inputElement })
await keybindingService.keybindHandler(event)
expect(vi.mocked(app.canvas.processKey)).not.toHaveBeenCalled()
expect(vi.mocked(useCommandStore().execute)).not.toHaveBeenCalled()
})
it('should not execute DeleteSelectedItems when typing in textarea', async () => {
it('should not forward Delete key when typing in textarea', async () => {
const textareaElement = document.createElement('textarea')
const event = createTestKeyboardEvent('Delete', {
target: textareaElement
})
const event = createTestKeyboardEvent('Delete', { target: textareaElement })
await keybindingService.keybindHandler(event)
expect(vi.mocked(app.canvas.processKey)).not.toHaveBeenCalled()
expect(vi.mocked(useCommandStore().execute)).not.toHaveBeenCalled()
})
it('should execute SelectAll for Ctrl+A on canvas', async () => {
const event = createTestKeyboardEvent('a', {
ctrlKey: true,
target: canvasChild
})
it('should not forward Delete key when canvas processKey is not available', async () => {
// Temporarily replace processKey with undefined - testing edge case
const originalProcessKey = vi.mocked(app.canvas).processKey
vi.mocked(app.canvas).processKey = undefined!
await keybindingService.keybindHandler(event)
const event = createTestKeyboardEvent('Delete')
expect(vi.mocked(useCommandStore().execute)).toHaveBeenCalledWith(
'Comfy.Canvas.SelectAll'
)
try {
await keybindingService.keybindHandler(event)
expect(vi.mocked(useCommandStore().execute)).not.toHaveBeenCalled()
} finally {
// Restore processKey for other tests
vi.mocked(app.canvas).processKey = originalProcessKey
}
})
it('should execute CopySelected for Ctrl+C on canvas', async () => {
const event = createTestKeyboardEvent('c', {
ctrlKey: true,
target: canvasChild
})
it('should not forward Delete key when canvas is not available', async () => {
const originalCanvas = vi.mocked(app).canvas
vi.mocked(app).canvas = null!
await keybindingService.keybindHandler(event)
const event = createTestKeyboardEvent('Delete')
expect(vi.mocked(useCommandStore().execute)).toHaveBeenCalledWith(
'Comfy.Canvas.CopySelected'
)
try {
await keybindingService.keybindHandler(event)
expect(vi.mocked(useCommandStore().execute)).not.toHaveBeenCalled()
} finally {
// Restore canvas for other tests
vi.mocked(app).canvas = originalCanvas
}
})
it('should execute PasteFromClipboard for Ctrl+V on canvas', async () => {
const event = createTestKeyboardEvent('v', {
ctrlKey: true,
target: canvasChild
})
await keybindingService.keybindHandler(event)
expect(vi.mocked(useCommandStore().execute)).toHaveBeenCalledWith(
'Comfy.Canvas.PasteFromClipboard'
)
})
it('should execute PasteFromClipboardWithConnect for Ctrl+Shift+V on canvas', async () => {
const event = createTestKeyboardEvent('v', {
ctrlKey: true,
shiftKey: true,
target: canvasChild
})
await keybindingService.keybindHandler(event)
expect(vi.mocked(useCommandStore().execute)).toHaveBeenCalledWith(
'Comfy.Canvas.PasteFromClipboardWithConnect'
)
})
it('should execute graph-canvas bindings by normalizing to graph-canvas-container', async () => {
const event = createTestKeyboardEvent('=', {
altKey: true,
target: canvasChild
})
await keybindingService.keybindHandler(event)
expect(vi.mocked(useCommandStore().execute)).toHaveBeenCalledWith(
'Comfy.Canvas.ZoomIn'
)
})
it('should not execute graph-canvas bindings when target is outside canvas', async () => {
const outsideDiv = document.createElement('div')
document.body.appendChild(outsideDiv)
const event = createTestKeyboardEvent('=', {
altKey: true,
target: outsideDiv
})
it('should not forward non-canvas keys', async () => {
const event = createTestKeyboardEvent('Enter')
await keybindingService.keybindHandler(event)
expect(vi.mocked(app.canvas.processKey)).not.toHaveBeenCalled()
expect(vi.mocked(useCommandStore().execute)).not.toHaveBeenCalled()
outsideDiv.remove()
})
it('should not execute canvas commands when target is outside canvas container', async () => {
const outsideDiv = document.createElement('div')
document.body.appendChild(outsideDiv)
const event = createTestKeyboardEvent('Delete', {
target: outsideDiv
})
it('should not forward when modifier keys are pressed', async () => {
const event = createTestKeyboardEvent('Delete', { ctrlKey: true })
await keybindingService.keybindHandler(event)
expect(vi.mocked(app.canvas.processKey)).not.toHaveBeenCalled()
expect(vi.mocked(useCommandStore().execute)).not.toHaveBeenCalled()
outsideDiv.remove()
})
})

View File

@@ -1,5 +1,6 @@
import { isCloud } from '@/platform/distribution/types'
import { useSettingStore } from '@/platform/settings/settingStore'
import { app } from '@/scripts/app'
import { useCommandStore } from '@/stores/commandStore'
import { useDialogStore } from '@/stores/dialogStore'
@@ -14,6 +15,16 @@ export function useKeybindingService() {
const settingStore = useSettingStore()
const dialogStore = useDialogStore()
function shouldForwardToCanvas(event: KeyboardEvent): boolean {
if (event.ctrlKey || event.altKey || event.metaKey) {
return false
}
const canvasKeys = ['Delete', 'Backspace']
return canvasKeys.includes(event.key)
}
async function keybindHandler(event: KeyboardEvent) {
const keyCombo = KeyComboImpl.fromEvent(event)
if (keyCombo.isModifier) {
@@ -33,17 +44,7 @@ export function useKeybindingService() {
}
const keybinding = keybindingStore.getKeybinding(keyCombo)
if (keybinding) {
const targetElementId =
keybinding.targetElementId === 'graph-canvas'
? 'graph-canvas-container'
: keybinding.targetElementId
if (targetElementId) {
const container = document.getElementById(targetElementId)
if (!container?.contains(target)) {
return
}
}
if (keybinding && keybinding.targetElementId !== 'graph-canvas') {
if (
event.key === 'Escape' &&
!event.ctrlKey &&
@@ -73,6 +74,18 @@ export function useKeybindingService() {
return
}
if (!keybinding && shouldForwardToCanvas(event)) {
const canvas = app.canvas
if (
canvas &&
canvas.processKey &&
typeof canvas.processKey === 'function'
) {
canvas.processKey(event)
return
}
}
if (event.ctrlKey || event.altKey || event.metaKey) {
return
}

View File

@@ -99,7 +99,9 @@ vi.mock('@/renderer/core/canvas/canvasStore', () => ({
vi.mock('@/renderer/core/thumbnail/useWorkflowThumbnail', () => ({
useWorkflowThumbnail: () => ({
storeThumbnail: vi.fn(),
getThumbnail: vi.fn()
getThumbnail: vi.fn(),
clearThumbnail: vi.fn(),
moveWorkflowThumbnail: vi.fn()
})
}))
@@ -853,12 +855,13 @@ describe('useWorkflowService', () => {
const existing = createSaveableWorkflow('workflows/test.app.json')
vi.spyOn(workflowStore, 'getWorkflowByPath').mockReturnValue(existing)
vi.spyOn(workflowStore, 'deleteWorkflow').mockResolvedValue()
vi.spyOn(workflowStore, 'removeWorkflowEntry').mockResolvedValue()
mockConfirm.mockResolvedValue(true)
await service.saveWorkflow(workflow)
expect(mockConfirm).toHaveBeenCalled()
expect(workflowStore.removeWorkflowEntry).toHaveBeenCalledWith(existing)
expect(workflowStore.renameWorkflow).toHaveBeenCalledWith(
workflow,
'workflows/test.app.json'

View File

@@ -130,11 +130,20 @@ export const useWorkflowService = () => {
if (existingWorkflow && !existingWorkflow.isTemporary) {
if ((await confirmOverwrite(newPath)) !== true) return false
}
if (!isSelfOverwrite) {
const deleted = await deleteWorkflow(existingWorkflow, true)
if (!deleted) return false
const needsOverwrite =
!!existingWorkflow && !existingWorkflow.isTemporary && !isSelfOverwrite
// Close and remove the old workflow entry before saving the new content.
// The file on disk is intentionally kept so that a save failure does not
// cause data loss. The subsequent save with overwrite: true will
// atomically replace it.
if (needsOverwrite) {
if (workflowStore.isOpen(existingWorkflow)) {
await closeWorkflow(existingWorkflow, { warnIfUnsaved: false })
}
await workflowStore.removeWorkflowEntry(existingWorkflow)
}
workflow.changeTracker?.checkState()
@@ -143,11 +152,19 @@ export const useWorkflowService = () => {
await saveWorkflow(workflow)
} else if (workflow.isTemporary) {
await renameWorkflow(workflow, newPath)
await workflowStore.saveWorkflow(workflow)
if (needsOverwrite) {
await workflowStore.saveWorkflow(workflow, { overwrite: true })
} else {
await workflowStore.saveWorkflow(workflow)
}
} else {
const tempWorkflow = workflowStore.saveAs(workflow, newPath)
await openWorkflow(tempWorkflow)
await workflowStore.saveWorkflow(tempWorkflow)
if (needsOverwrite) {
await workflowStore.saveWorkflow(tempWorkflow, { overwrite: true })
} else {
await workflowStore.saveWorkflow(tempWorkflow)
}
}
return true
}
@@ -174,7 +191,12 @@ export const useWorkflowService = () => {
await workflowStore.saveWorkflow(workflow)
return
}
await deleteWorkflow(existing, true)
// Remove the old entry without deleting the file; the rename
// will atomically replace it, preventing data loss on failure.
if (workflowStore.isOpen(existing)) {
await closeWorkflow(existing, { warnIfUnsaved: false })
}
await workflowStore.removeWorkflowEntry(existing)
}
await renameWorkflow(workflow, expectedPath)
toastStore.add({

View File

@@ -151,14 +151,16 @@ export class ComfyWorkflow extends UserFile {
super.unload()
}
override async save() {
override async save({
overwrite
}: { force?: boolean; overwrite?: boolean } = {}) {
const { useWorkflowDraftStore } =
await import('@/platform/workflow/persistence/stores/workflowDraftStore')
const draftStore = useWorkflowDraftStore()
this.content = JSON.stringify(this.activeState)
// Force save to ensure the content is updated in remote storage incase
// the isModified state is screwed by changeTracker.
const ret = await super.save({ force: true })
const ret = await super.save({ force: true, overwrite })
this.changeTracker?.reset()
this.isModified = false
draftStore.removeDraft(this.path)

View File

@@ -63,7 +63,11 @@ interface WorkflowStore {
) => ComfyWorkflow
renameWorkflow: (workflow: ComfyWorkflow, newPath: string) => Promise<void>
deleteWorkflow: (workflow: ComfyWorkflow) => Promise<void>
saveWorkflow: (workflow: ComfyWorkflow) => Promise<void>
removeWorkflowEntry: (workflow: ComfyWorkflow) => Promise<void>
saveWorkflow: (
workflow: ComfyWorkflow,
options?: { overwrite?: boolean }
) => Promise<void>
workflows: ComfyWorkflow[]
bookmarkedWorkflows: ComfyWorkflow[]
@@ -539,14 +543,32 @@ export const useWorkflowStore = defineStore('workflow', () => {
}
}
/**
* Remove a workflow entry from the store without deleting the file on disk.
* Used during atomic overwrite to clear the old entry before saving the new
* content, so that the save can use overwrite: true to replace the file.
*/
const removeWorkflowEntry = async (workflow: ComfyWorkflow) => {
useWorkflowDraftStore().removeDraft(workflow.path)
if (bookmarkStore.isBookmarked(workflow.path)) {
await bookmarkStore.setBookmarked(workflow.path, false)
}
clearThumbnail(workflow.key)
delete workflowLookup.value[workflow.path]
}
/**
* Save a workflow.
* @param workflow The workflow to save.
* @param options.overwrite Force overwrite of existing file at the path.
*/
const saveWorkflow = async (workflow: ComfyWorkflow) => {
const saveWorkflow = async (
workflow: ComfyWorkflow,
options?: { overwrite?: boolean }
) => {
isBusy.value = true
try {
await workflow.save()
await workflow.save({ overwrite: options?.overwrite })
// Synchronously detach and re-attach to force refresh the tree objects
// without an async gap that would cause the tab to disappear.
const openIndex = detachWorkflow(workflow)
@@ -774,6 +796,7 @@ export const useWorkflowStore = defineStore('workflow', () => {
createNewTemporary,
renameWorkflow,
deleteWorkflow,
removeWorkflowEntry,
saveAs,
saveWorkflow,
reorderWorkflows,

View File

@@ -676,6 +676,20 @@ export class ComfyApp {
e.stopImmediatePropagation()
return
}
// Ctrl+C Copy
if (e.key === 'c' && (e.metaKey || e.ctrlKey)) {
return
}
// Ctrl+V Paste
if (
(e.key === 'v' || e.key == 'V') &&
(e.metaKey || e.ctrlKey) &&
!e.shiftKey
) {
return
}
}
// Fall through to Litegraph defaults

View File

@@ -140,11 +140,14 @@ export class UserFile {
* Saves the file to the remote storage.
* @param force Whether to force the save even if the file is not modified.
*/
async save({ force = false }: { force?: boolean } = {}): Promise<UserFile> {
async save({
force = false,
overwrite
}: { force?: boolean; overwrite?: boolean } = {}): Promise<UserFile> {
if (this.isPersisted && !this.isModified && !force) return this
const resp = await api.storeUserData(this.path, this.content, {
overwrite: this.isPersisted,
overwrite: overwrite ?? this.isPersisted,
throwOnError: true,
full_info: true
})