mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-05-23 06:10:32 +00:00
fix: prevent slug id from overwriting migrated UUID via changeTracker.reset
ChangeTracker.reset(state) clones the argument into activeState unconditionally, so passing the raw incoming workflowData on the same-path reuse path could overwrite a freshly-migrated UUID with the slug id that was on disk, negating the migration on the next save. Wrap the two reset call sites in a withMigratedId helper that preserves a valid incoming UUID and otherwise substitutes the existing UUID (or a fresh one) so reset can never write a non-UUID id back. Strengthen the slug-vs-UUID reuse test to assert the reset payload, and add a both- sides-legacy-slug test plus an explicit pass-through test for the common matching-UUID case.
This commit is contained in:
@@ -530,8 +530,8 @@ describe('useWorkflowService', () => {
|
||||
})
|
||||
|
||||
it('should reuse active workflow when the incoming id is a legacy slug and existing id is a fresh UUID', async () => {
|
||||
existingWorkflow.changeTracker.activeState.id =
|
||||
'9cea40bb-b0cf-4b40-a758-8935cfe8d52f'
|
||||
const existingUuid = '9cea40bb-b0cf-4b40-a758-8935cfe8d52f'
|
||||
existingWorkflow.changeTracker.activeState.id = existingUuid
|
||||
|
||||
await useWorkflowService().afterLoadNewGraph('repeat', {
|
||||
id: 'video-point-prompt-example',
|
||||
@@ -539,7 +539,9 @@ describe('useWorkflowService', () => {
|
||||
} as never)
|
||||
|
||||
expect(workflowStore.openWorkflow).toHaveBeenCalledWith(existingWorkflow)
|
||||
expect(existingWorkflow.changeTracker.reset).toHaveBeenCalled()
|
||||
expect(existingWorkflow.changeTracker.reset).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ id: existingUuid })
|
||||
)
|
||||
expect(existingWorkflow.changeTracker.restore).toHaveBeenCalled()
|
||||
expect(workflowStore.createNewTemporary).not.toHaveBeenCalled()
|
||||
})
|
||||
@@ -554,6 +556,26 @@ describe('useWorkflowService', () => {
|
||||
|
||||
expect(workflowStore.openWorkflow).toHaveBeenCalledWith(existingWorkflow)
|
||||
expect(workflowStore.createNewTemporary).not.toHaveBeenCalled()
|
||||
const resetArg = vi.mocked(existingWorkflow.changeTracker.reset).mock
|
||||
.calls[0][0]
|
||||
expect(resetArg?.id).toMatch(
|
||||
/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i
|
||||
)
|
||||
expect(resetArg?.id).not.toBe('different-legacy-name')
|
||||
})
|
||||
|
||||
it('should pass through the incoming UUID id to changeTracker.reset on same-path reuse', async () => {
|
||||
const sharedUuid = '9cea40bb-b0cf-4b40-a758-8935cfe8d52f'
|
||||
existingWorkflow.changeTracker.activeState.id = sharedUuid
|
||||
|
||||
await useWorkflowService().afterLoadNewGraph('repeat', {
|
||||
id: sharedUuid,
|
||||
nodes: [{ id: 1, type: 'TestNode', pos: [0, 0], size: [100, 100] }]
|
||||
} as never)
|
||||
|
||||
expect(existingWorkflow.changeTracker.reset).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ id: sharedUuid })
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@@ -51,6 +51,21 @@ function normalizeWorkflowIdForReuse(id: unknown): string | undefined {
|
||||
return isValidUuid(id) ? id : undefined
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the workflow payload with a UUID-shaped `id`, preserving the
|
||||
* existing one when valid and otherwise substituting `fallbackId` (or a fresh
|
||||
* UUID). Used before passing data to `ChangeTracker.reset` so a slug id on the
|
||||
* incoming payload cannot overwrite the migrated UUID already stored on the
|
||||
* reused workflow.
|
||||
*/
|
||||
function withMigratedId(
|
||||
workflowData: ComfyWorkflowJSON,
|
||||
fallbackId: string | undefined
|
||||
): ComfyWorkflowJSON {
|
||||
if (isValidUuid(workflowData.id)) return workflowData
|
||||
return { ...workflowData, id: fallbackId ?? generateUUID() }
|
||||
}
|
||||
|
||||
export const useWorkflowService = () => {
|
||||
const settingStore = useSettingStore()
|
||||
const workflowStore = useWorkflowStore()
|
||||
@@ -505,7 +520,9 @@ export const useWorkflowService = () => {
|
||||
) ?? freshLoadMode
|
||||
trackIfEnteringApp(loadedWorkflow)
|
||||
}
|
||||
loadedWorkflow.changeTracker.reset(workflowData)
|
||||
loadedWorkflow.changeTracker.reset(
|
||||
withMigratedId(workflowData, existingId)
|
||||
)
|
||||
loadedWorkflow.changeTracker.restore()
|
||||
return
|
||||
}
|
||||
@@ -526,7 +543,12 @@ export const useWorkflowService = () => {
|
||||
loadedWorkflow.initialMode = freshLoadMode
|
||||
trackIfEnteringApp(loadedWorkflow)
|
||||
}
|
||||
loadedWorkflow.changeTracker.reset(workflowData)
|
||||
loadedWorkflow.changeTracker.reset(
|
||||
withMigratedId(
|
||||
workflowData,
|
||||
normalizeWorkflowIdForReuse(loadedWorkflow.activeState?.id)
|
||||
)
|
||||
)
|
||||
loadedWorkflow.changeTracker.restore()
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user