fix: Implement atomic workflow overwrite via persistence-level flag (#9355)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

src/locales/en/main.json

@@ -178,7 +178,7 @@
     "uploadAlreadyInProgress": "Upload already in progress",
     "capture": "capture",
     "nodes": "Nodes",
-    "nodesCount": "{count} nodes | {count} node | {count} nodes",
+    "nodesCount": "{count} node | {count} nodes",
     "addNode": "Add a node...",
     "filterBy": "Filter by:",
     "filterByType": "Filter by {type}...",
@@ -222,7 +222,7 @@
     "failed": "Failed",
     "cancelled": "Cancelled",
     "job": "Job",
-    "asset": "{count} assets | {count} asset | {count} assets",
+    "asset": "{count} asset | {count} assets",
     "untitled": "Untitled",
     "emDash": "—",
     "enabling": "Enabling {id}",
@@ -3347,7 +3347,7 @@
     }
   },
   "errorOverlay": {
-    "errorCount": "{count} ERRORS | {count} ERROR | {count} ERRORS",
+    "errorCount": "{count} ERROR | {count} ERRORS",
     "seeErrors": "See Errors"
   },
   "help": {
@@ -3357,7 +3357,7 @@
   "progressToast": {
     "importingModels": "Importing Models",
     "downloadingModel": "Downloading model...",
-    "downloadsFailed": "{count} downloads failed | {count} download failed | {count} downloads failed",
+    "downloadsFailed": "{count} download failed | {count} downloads failed",
     "allDownloadsCompleted": "All downloads completed",
     "noImportsInQueue": "No {filter} in queue",
     "failed": "Failed",
@@ -3374,7 +3374,7 @@
     "exportingAssets": "Exporting Assets",
     "preparingExport": "Preparing export...",
     "exportError": "Export failed",
-    "exportFailed": "{count} export failed | {count} export failed | {count} exports failed",
+    "exportFailed": "{count} export failed | {count} exports failed",
     "allExportsCompleted": "All exports completed",
     "noExportsInQueue": "No {filter} exports in queue",
     "exportStarted": "Preparing ZIP download...",

src/platform/workflow/core/services/workflowService.test.ts

@@ -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'

src/platform/workflow/core/services/workflowService.ts

@@ -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({

src/platform/workflow/management/stores/comfyWorkflow.ts

@@ -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)

src/platform/workflow/management/stores/workflowStore.ts

@@ -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,

src/platform/workflow/sharing/components/publish/ComfyHubPublishDialog.test.ts

@@ -1,79 +1,59 @@
 import { flushPromises, mount } from '@vue/test-utils'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
+import { ref } from 'vue'
 
 import ComfyHubPublishDialog from '@/platform/workflow/sharing/components/publish/ComfyHubPublishDialog.vue'
-import type { ComfyHubProfile } from '@/schemas/apiSchema'
 
-const mockFetchApi = vi.hoisted(() => vi.fn())
-const mockToastErrorHandler = vi.hoisted(() => vi.fn())
-const mockResolvedUserInfo = vi.hoisted(() => ({
-  value: { id: 'user-a' }
-}))
+const mockFetchProfile = vi.hoisted(() => vi.fn())
+const mockGoToStep = vi.hoisted(() => vi.fn())
+const mockGoNext = vi.hoisted(() => vi.fn())
+const mockGoBack = vi.hoisted(() => vi.fn())
+const mockOpenProfileCreationStep = vi.hoisted(() => vi.fn())
+const mockCloseProfileCreationStep = vi.hoisted(() => vi.fn())
 
-vi.mock('@/scripts/api', () => ({
-  api: {
-    fetchApi: mockFetchApi
-  }
-}))
-
-vi.mock('@/composables/auth/useCurrentUser', () => ({
-  useCurrentUser: () => ({
-    resolvedUserInfo: mockResolvedUserInfo
+vi.mock(
+  '@/platform/workflow/sharing/composables/useComfyHubProfileGate',
+  () => ({
+    useComfyHubProfileGate: () => ({
+      fetchProfile: mockFetchProfile
+    })
   })
-}))
+)
 
-vi.mock('@/composables/useErrorHandling', () => ({
-  useErrorHandling: () => ({
-    toastErrorHandler: mockToastErrorHandler
+vi.mock(
+  '@/platform/workflow/sharing/composables/useComfyHubPublishWizard',
+  () => ({
+    useComfyHubPublishWizard: () => ({
+      currentStep: ref('finish'),
+      formData: ref({
+        name: '',
+        description: '',
+        workflowType: '',
+        tags: [],
+        thumbnailType: 'image',
+        thumbnailFile: null,
+        comparisonBeforeFile: null,
+        comparisonAfterFile: null,
+        exampleImages: [],
+        selectedExampleIds: []
+      }),
+      isFirstStep: ref(false),
+      isLastStep: ref(true),
+      goToStep: mockGoToStep,
+      goNext: mockGoNext,
+      goBack: mockGoBack,
+      openProfileCreationStep: mockOpenProfileCreationStep,
+      closeProfileCreationStep: mockCloseProfileCreationStep
+    })
   })
-}))
-
-vi.mock('@/platform/workflow/management/stores/workflowStore', () => ({
-  useWorkflowStore: () => ({
-    activeWorkflow: { filename: 'test-workflow' }
-  })
-}))
-
-const mockProfile: ComfyHubProfile = {
-  username: 'testuser',
-  name: 'Test User',
-  description: 'A test profile'
-}
-
-function mockSuccessResponse(data?: unknown) {
-  return {
-    ok: true,
-    json: async () => data ?? mockProfile
-  } as Response
-}
-
-function mockErrorResponse(status = 404) {
-  return {
-    ok: false,
-    status,
-    json: async () => ({ message: 'Not found' })
-  } as Response
-}
-
-// Reset module-level singleton state in useComfyHubProfileGate between tests
-async function resetProfileGateSingleton() {
-  const { useComfyHubProfileGate } =
-    await import('@/platform/workflow/sharing/composables/useComfyHubProfileGate')
-  const gate = useComfyHubProfileGate()
-  gate.hasProfile.value = null
-  gate.profile.value = null
-  gate.isCheckingProfile.value = false
-  gate.isFetchingProfile.value = false
-}
+)
 
 describe('ComfyHubPublishDialog', () => {
   const onClose = vi.fn()
 
-  beforeEach(async () => {
+  beforeEach(() => {
     vi.clearAllMocks()
-    mockResolvedUserInfo.value = { id: 'user-a' }
-    mockFetchApi.mockResolvedValue(mockErrorResponse())
-    await resetProfileGateSingleton()
+    mockFetchProfile.mockResolvedValue(null)
   })
 
   function createWrapper() {
@@ -98,7 +78,7 @@ describe('ComfyHubPublishDialog', () => {
           },
           ComfyHubPublishWizardContent: {
             template:
-              '<div><button data-testid="require-profile" @click="$props.onRequireProfile()" /><button data-testid="gate-complete" @click="$props.onGateComplete()" /><button data-testid="gate-close" @click="$props.onGateClose()" /><span data-testid="current-step">{{ $props.currentStep }}</span></div>',
+              '<div><button data-testid="require-profile" @click="$props.onRequireProfile()" /><button data-testid="gate-complete" @click="$props.onGateComplete()" /><button data-testid="gate-close" @click="$props.onGateClose()" /></div>',
             props: [
               'currentStep',
               'formData',
@@ -108,9 +88,7 @@ describe('ComfyHubPublishDialog', () => {
               'onGoBack',
               'onRequireProfile',
               'onGateComplete',
-              'onGateClose',
-              'onUpdateFormData',
-              'onPublish'
+              'onGateClose'
             ]
           }
         }
@@ -118,62 +96,44 @@ describe('ComfyHubPublishDialog', () => {
     })
   }
 
-  it('prefetches profile on mount via real composable', async () => {
+  it('starts in publish wizard mode and prefetches profile asynchronously', async () => {
     createWrapper()
     await flushPromises()
 
-    expect(mockFetchApi).toHaveBeenCalledWith('/hub/profile')
+    expect(mockFetchProfile).toHaveBeenCalledWith()
   })
 
-  it('starts on the describe step with real wizard composable', async () => {
-    const wrapper = createWrapper()
-    await flushPromises()
-
-    expect(wrapper.find('[data-testid="current-step"]').text()).toBe('describe')
-  })
-
-  it('switches to profileCreation step when require-profile is triggered', async () => {
+  it('switches to profile creation step when final-step publish requires profile', async () => {
     const wrapper = createWrapper()
     await flushPromises()
 
     await wrapper.find('[data-testid="require-profile"]').trigger('click')
 
-    expect(wrapper.find('[data-testid="current-step"]').text()).toBe(
-      'profileCreation'
-    )
+    expect(mockOpenProfileCreationStep).toHaveBeenCalledOnce()
   })
 
-  it('returns to finish step and re-fetches profile after gate complete', async () => {
-    mockFetchApi.mockResolvedValue(mockSuccessResponse())
+  it('returns to finish state after gate complete and does not auto-close', async () => {
     const wrapper = createWrapper()
     await flushPromises()
 
     await wrapper.find('[data-testid="require-profile"]').trigger('click')
-    expect(wrapper.find('[data-testid="current-step"]').text()).toBe(
-      'profileCreation'
-    )
-
     await wrapper.find('[data-testid="gate-complete"]').trigger('click')
-    await flushPromises()
 
-    expect(wrapper.find('[data-testid="current-step"]').text()).toBe('finish')
-    // Initial prefetch + force re-fetch after gate complete
-    expect(mockFetchApi).toHaveBeenCalledTimes(2)
+    expect(mockOpenProfileCreationStep).toHaveBeenCalledOnce()
+    expect(mockCloseProfileCreationStep).toHaveBeenCalledOnce()
+    expect(mockFetchProfile).toHaveBeenCalledWith({ force: true })
     expect(onClose).not.toHaveBeenCalled()
   })
 
-  it('returns to finish step when profile gate is closed', async () => {
+  it('returns to finish state when profile gate is closed', async () => {
     const wrapper = createWrapper()
     await flushPromises()
 
     await wrapper.find('[data-testid="require-profile"]').trigger('click')
-    expect(wrapper.find('[data-testid="current-step"]').text()).toBe(
-      'profileCreation'
-    )
-
     await wrapper.find('[data-testid="gate-close"]').trigger('click')
 
-    expect(wrapper.find('[data-testid="current-step"]').text()).toBe('finish')
+    expect(mockOpenProfileCreationStep).toHaveBeenCalledOnce()
+    expect(mockCloseProfileCreationStep).toHaveBeenCalledOnce()
     expect(onClose).not.toHaveBeenCalled()
   })
 })

src/stores/userFileStore.ts

@@ -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
     })