Compare commits

...

6 Commits

Author SHA1 Message Date
bymyself
29ddb04870 test: cover changeTracker.reset id migration on workflow-object reload
Add coverage for the `afterLoadNewGraph` branch where `value` is a
ComfyWorkflow instance: a slug id on the payload is migrated to the
existing UUID, and both-legacy-slug ids produce a fresh UUID.

Addresses review feedback:
https://github.com/Comfy-Org/ComfyUI_frontend/pull/11631#discussion_r3262848924
2026-06-29 20:34:34 -07:00
bymyself
b6cddae098 test: use fromPartial for ComfyWorkflowJSON migration test data
Replace `as unknown as ComfyWorkflowJSON` casts with
`fromPartial<ComfyWorkflowJSON>` in the createTemporary id-migration tests.

Addresses review feedback:
https://github.com/Comfy-Org/ComfyUI_frontend/pull/11631#discussion_r3262859971
https://github.com/Comfy-Org/ComfyUI_frontend/pull/11631#discussion_r3262859979
https://github.com/Comfy-Org/ComfyUI_frontend/pull/11631#discussion_r3262859985
https://github.com/Comfy-Org/ComfyUI_frontend/pull/11631#discussion_r3262859989
2026-06-29 20:34:16 -07:00
bymyself
f6636298a2 test: harden reset-arg extraction with explicit call assertion
Add expect(reset).toHaveBeenCalled() before reading the mock's first
call and use optional chaining on calls[0]?.[0] so a missing call
produces a clear assertion error rather than a TypeError on undefined
indexing. Addresses CodeRabbit nitpick on PR #11631.
2026-06-29 20:30:35 -07:00
Glary-Bot
2b81a37725 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.
2026-06-29 20:30:35 -07:00
Glary-Bot
3473650fc4 fix: normalize non-UUID ids in same-path workflow reuse check
After ensureWorkflowId starts rewriting slug-shaped workflow ids to fresh
UUIDs, a second load of the same file (which still carries the original
slug id on disk) would no longer match the stored UUID in the active
tab, and afterLoadNewGraph would open a duplicate temporary tab.

Collapse non-UUID ids to undefined before the reuse equality check so
the first-load-rewrites, second-load-reuses flow works. Update existing
same-path-reuse tests to use real UUIDs and add two regression tests:
one covering the slug-to-UUID rewrite case, and one for legacy
workflows that still carry non-UUID ids on both sides.
2026-06-29 20:30:35 -07:00
Glary-Bot
c4526cbbc8 fix: lazily migrate non-UUID workflow ids to a fresh UUID
Extend ensureWorkflowId to regenerate the top-level workflow id when it
is present but not a canonical UUID. Previously the guard only replaced
missing ids, so slug ids carried in from workflow templates (e.g.
'video-point-prompt-example' from an older SAM3 example) would survive
round-trip save/publish and later be rejected by the cloud share loader's
zod validation.

Adds an isValidUuid helper and exhaustive unit coverage for valid UUIDs
(lowercase/uppercase/mixed-case, any version/variant, fresh generateUUID
output), for rejection of slugs, numbers, malformed UUID-like strings,
and for the new createTemporary migration branch.
2026-06-29 20:30:35 -07:00
6 changed files with 278 additions and 13 deletions

View File

@@ -5,6 +5,7 @@ import {
ensureWorkflowSuffix,
formatLocalizedMediumDate,
formatLocalizedNumber,
generateUUID,
getFilePathSeparatorVariants,
getFilenameDetails,
getMediaTypeFromFilename,
@@ -13,6 +14,7 @@ import {
isCivitaiModelUrl,
isCivitaiUrl,
isPreviewableMediaType,
isValidUuid,
joinFilePath,
truncateFilename
} from './formatUtil'
@@ -477,4 +479,57 @@ describe('formatUtil', () => {
expect(formatLocalizedMediumDate('not a date', 'en')).toBe('—')
})
})
describe('isValidUuid', () => {
it('accepts canonical lowercase UUIDs', () => {
expect(isValidUuid('9cea40bb-b0cf-4b40-a758-8935cfe8d52f')).toBe(true)
expect(isValidUuid('00000000-0000-0000-0000-000000000000')).toBe(true)
})
it('accepts canonical uppercase and mixed-case UUIDs', () => {
expect(isValidUuid('9CEA40BB-B0CF-4B40-A758-8935CFE8D52F')).toBe(true)
expect(isValidUuid('9cea40BB-b0CF-4b40-A758-8935cfe8D52F')).toBe(true)
})
it('accepts any version and variant (not just v4)', () => {
expect(isValidUuid('00000000-0000-1000-0000-000000000000')).toBe(true)
expect(isValidUuid('ffffffff-ffff-7fff-ffff-ffffffffffff')).toBe(true)
})
it('recognizes generateUUID output as valid', () => {
for (let i = 0; i < 10; i++) {
expect(isValidUuid(generateUUID())).toBe(true)
}
})
it('rejects slug strings from workflow templates', () => {
expect(isValidUuid('video-point-prompt-example')).toBe(false)
expect(isValidUuid('default')).toBe(false)
expect(isValidUuid('my-workflow-v2')).toBe(false)
})
it('rejects non-string inputs', () => {
expect(isValidUuid(undefined)).toBe(false)
expect(isValidUuid(null)).toBe(false)
expect(isValidUuid('')).toBe(false)
expect(isValidUuid(0)).toBe(false)
expect(isValidUuid(123)).toBe(false)
expect(isValidUuid({})).toBe(false)
expect(isValidUuid([])).toBe(false)
})
it('rejects malformed UUID-like strings', () => {
expect(isValidUuid('9cea40bb-b0cf-4b40-a758-8935cfe8d52')).toBe(false)
expect(isValidUuid('9cea40bb-b0cf-4b40-a758-8935cfe8d52fa')).toBe(false)
expect(isValidUuid('9cea40bbb0cf4b40a7588935cfe8d52f')).toBe(false)
expect(isValidUuid('9cea40bb_b0cf_4b40_a758_8935cfe8d52f')).toBe(false)
expect(isValidUuid(' 9cea40bb-b0cf-4b40-a758-8935cfe8d52f')).toBe(false)
expect(isValidUuid('9cea40bb-b0cf-4b40-a758-8935cfe8d52f ')).toBe(false)
})
it('rejects strings with non-hex characters in UUID slots', () => {
expect(isValidUuid('gcea40bb-b0cf-4b40-a758-8935cfe8d52f')).toBe(false)
expect(isValidUuid('9cea40bb-b0cf-4b40-a758-8935cfe8d52z')).toBe(false)
})
})
})

View File

@@ -358,6 +358,24 @@ export const paramsToCacheKey = (params: unknown): string => {
return String(params)
}
/**
* Matches the RFC 4122 canonical 8-4-4-4-12 hex form (case-insensitive).
* Accepts any version/variant, since we only need to distinguish "looks like a
* UUID" from "does not look like a UUID" (e.g. a template slug).
*/
const UUID_PATTERN =
/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i
/**
* Checks whether a value is a RFC 4122 canonical-form UUID string.
*
* Intentionally permissive on version/variant bits: some historical workflows
* carry UUIDs generated by older code paths that did not enforce v4, and we
* only use this to guard against non-UUID slugs like `"video-point-prompt-example"`.
*/
export const isValidUuid = (value: unknown): value is string =>
typeof value === 'string' && UUID_PATTERN.test(value)
/**
* Generates a RFC4122 compliant UUID v4 using the native crypto API when available
* @returns A properly formatted UUID string

View File

@@ -564,7 +564,7 @@ describe('useWorkflowService', () => {
})
it('should reuse the active workflow when loading the same path repeatedly', async () => {
const workflowId = 'repeat-workflow-id'
const workflowId = '9cea40bb-b0cf-4b40-a758-8935cfe8d52f'
existingWorkflow.changeTracker.activeState.id = workflowId
await useWorkflowService().afterLoadNewGraph('repeat', {
@@ -596,7 +596,8 @@ describe('useWorkflowService', () => {
})
it('should reuse active workflow when only one side has an id', async () => {
existingWorkflow.changeTracker.activeState.id = 'existing-id'
existingWorkflow.changeTracker.activeState.id =
'9cea40bb-b0cf-4b40-a758-8935cfe8d52f'
await useWorkflowService().afterLoadNewGraph('repeat', {
nodes: [{ id: 1, type: 'TestNode', pos: [0, 0], size: [100, 100] }]
@@ -610,7 +611,7 @@ describe('useWorkflowService', () => {
it('should reuse active workflow when only workflowData has an id', async () => {
await useWorkflowService().afterLoadNewGraph('repeat', {
id: 'incoming-id',
id: '9cea40bb-b0cf-4b40-a758-8935cfe8d52f',
nodes: [{ id: 1, type: 'TestNode', pos: [0, 0], size: [100, 100] }]
} as never)
@@ -621,7 +622,8 @@ describe('useWorkflowService', () => {
})
it('should create new temporary when ids differ', async () => {
existingWorkflow.changeTracker.activeState.id = 'existing-id'
existingWorkflow.changeTracker.activeState.id =
'9cea40bb-b0cf-4b40-a758-8935cfe8d52f'
const tempWorkflow = createModeTestWorkflow({
path: 'workflows/repeat (2).json'
@@ -630,7 +632,7 @@ describe('useWorkflowService', () => {
vi.mocked(workflowStore.openWorkflow).mockResolvedValue(tempWorkflow)
await useWorkflowService().afterLoadNewGraph('repeat', {
id: 'different-id',
id: '11111111-2222-3333-4444-555555555555',
nodes: [{ id: 1, type: 'TestNode', pos: [0, 0], size: [100, 100] }]
} as never)
@@ -701,6 +703,89 @@ describe('useWorkflowService', () => {
expect(existingWorkflow.shareId).toBe('share-2')
})
it('should reuse active workflow when the incoming id is a legacy slug and existing id is a fresh UUID', async () => {
const existingUuid = '9cea40bb-b0cf-4b40-a758-8935cfe8d52f'
existingWorkflow.changeTracker.activeState.id = existingUuid
await useWorkflowService().afterLoadNewGraph('repeat', {
id: 'video-point-prompt-example',
nodes: [{ id: 1, type: 'TestNode', pos: [0, 0], size: [100, 100] }]
} as never)
expect(workflowStore.openWorkflow).toHaveBeenCalledWith(existingWorkflow)
expect(existingWorkflow.changeTracker.reset).toHaveBeenCalledWith(
expect.objectContaining({ id: existingUuid })
)
expect(existingWorkflow.changeTracker.restore).toHaveBeenCalled()
expect(workflowStore.createNewTemporary).not.toHaveBeenCalled()
})
it('should reuse active workflow when both sides carry malformed legacy ids', async () => {
existingWorkflow.changeTracker.activeState.id = 'legacy-workflow-name'
await useWorkflowService().afterLoadNewGraph('repeat', {
id: 'different-legacy-name',
nodes: [{ id: 1, type: 'TestNode', pos: [0, 0], size: [100, 100] }]
} as never)
expect(workflowStore.openWorkflow).toHaveBeenCalledWith(existingWorkflow)
expect(workflowStore.createNewTemporary).not.toHaveBeenCalled()
expect(existingWorkflow.changeTracker.reset).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 })
)
})
it('migrates a slug id to the existing UUID when reloading a workflow object', async () => {
const existingUuid = '9cea40bb-b0cf-4b40-a758-8935cfe8d52f'
existingWorkflow.changeTracker.activeState.id = existingUuid
await useWorkflowService().afterLoadNewGraph(existingWorkflow, {
id: 'video-point-prompt-example',
nodes: [{ id: 1, type: 'TestNode', pos: [0, 0], size: [100, 100] }]
} as never)
expect(workflowStore.openWorkflow).toHaveBeenCalledWith(existingWorkflow)
expect(existingWorkflow.changeTracker.reset).toHaveBeenCalledWith(
expect.objectContaining({ id: existingUuid })
)
expect(existingWorkflow.changeTracker.restore).toHaveBeenCalled()
})
it('generates a fresh UUID when reloading a workflow object whose ids are both legacy slugs', async () => {
existingWorkflow.changeTracker.activeState.id = 'legacy-workflow-name'
await useWorkflowService().afterLoadNewGraph(existingWorkflow, {
id: 'different-legacy-name',
nodes: [{ id: 1, type: 'TestNode', pos: [0, 0], size: [100, 100] }]
} as never)
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')
expect(resetArg?.id).not.toBe('legacy-workflow-name')
})
})
describe('per-workflow mode switching', () => {

View File

@@ -34,7 +34,8 @@ import { useWorkspaceStore } from '@/stores/workspaceStore'
import {
appendJsonExt,
appendWorkflowJsonExt,
generateUUID
generateUUID,
isValidUuid
} from '@/utils/formatUtil'
import type { AppMode } from '@/utils/appMode'
@@ -43,6 +44,32 @@ function linearModeToAppMode(linearMode: unknown): AppMode | null {
return linearMode ? 'app' : 'graph'
}
/**
* Normalize a workflow id for the same-path reuse equality check in
* `afterLoadNewGraph`. Non-UUID ids (empty, slug, or otherwise malformed) are
* collapsed to `undefined` so that subsequent loads of a file whose stored
* legacy id was rewritten by `ensureWorkflowId` still match the active tab
* instead of opening a duplicate.
*/
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()
@@ -477,12 +504,16 @@ export const useWorkflowService = () => {
//
// This prevents accidental duplicate tabs when startup/load flows
// invoke loadGraphData more than once for the same workflow name.
const existingId = normalizeWorkflowIdForReuse(
existingWorkflow?.activeState?.id
)
const incomingId = normalizeWorkflowIdForReuse(workflowData.id)
const isSameActiveWorkflowLoad =
!!existingWorkflow &&
workflowStore.isActive(existingWorkflow) &&
(existingWorkflow.activeState?.id === undefined ||
workflowData.id === undefined ||
existingWorkflow.activeState.id === workflowData.id)
(existingId === undefined ||
incomingId === undefined ||
existingId === incomingId)
if (
existingWorkflow &&
@@ -503,7 +534,9 @@ export const useWorkflowService = () => {
if (shareId) {
loadedWorkflow.shareId = shareId
}
loadedWorkflow.changeTracker.reset(workflowData)
loadedWorkflow.changeTracker.reset(
withMigratedId(workflowData, existingId)
)
loadedWorkflow.changeTracker.restore()
return
}
@@ -530,7 +563,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()
}

View File

@@ -10,6 +10,7 @@ import type {
ComfyWorkflow,
LoadedComfyWorkflow
} from '@/platform/workflow/management/stores/workflowStore'
import type { ComfyWorkflowJSON } from '@/platform/workflow/validation/schemas/workflowSchema'
import {
useWorkflowBookmarkStore,
useWorkflowStore
@@ -246,6 +247,74 @@ describe('useWorkflowStore', () => {
expect(state.id.length).toBeGreaterThan(0)
expect(workflowDataWithoutId.id).toBeUndefined()
})
it('should replace a non-UUID workflow id with a fresh UUID', () => {
const workflowDataWithSlugId = fromPartial<ComfyWorkflowJSON>({
...defaultGraph,
id: 'video-point-prompt-example'
})
const workflow = store.createTemporary(
'slug-id.json',
workflowDataWithSlugId
)
const state = JSON.parse(workflow.content!)
expect(state.id).not.toBe('video-point-prompt-example')
expect(state.id).toMatch(
/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i
)
expect(workflowDataWithSlugId.id).toBe('video-point-prompt-example')
})
it('should replace an empty-string id with a fresh UUID', () => {
const workflowDataWithEmptyId = fromPartial<ComfyWorkflowJSON>({
...defaultGraph,
id: ''
})
const workflow = store.createTemporary(
'empty-id.json',
workflowDataWithEmptyId
)
const state = JSON.parse(workflow.content!)
expect(state.id).toMatch(
/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i
)
})
it('should preserve a valid existing UUID', () => {
const existingUuid = '9cea40bb-b0cf-4b40-a758-8935cfe8d52f'
const workflowDataWithValidId = fromPartial<ComfyWorkflowJSON>({
...defaultGraph,
id: existingUuid
})
const workflow = store.createTemporary(
'valid-uuid.json',
workflowDataWithValidId
)
const state = JSON.parse(workflow.content!)
expect(state.id).toBe(existingUuid)
})
it('should preserve a valid uppercase UUID', () => {
const existingUuid = '9CEA40BB-B0CF-4B40-A758-8935CFE8D52F'
const workflowDataWithUpperId = fromPartial<ComfyWorkflowJSON>({
...defaultGraph,
id: existingUuid
})
const workflow = store.createTemporary(
'upper-uuid.json',
workflowDataWithUpperId
)
const state = JSON.parse(workflow.content!)
expect(state.id).toBe(existingUuid)
})
})
describe('openWorkflow', () => {

View File

@@ -25,7 +25,7 @@ import {
} from '@/types/nodeIdentification'
import { parseNodeId } from '@/types/nodeId'
import type { NodeId } from '@/types/nodeId'
import { generateUUID, getPathDetails } from '@/utils/formatUtil'
import { generateUUID, getPathDetails, isValidUuid } from '@/utils/formatUtil'
import { syncEntities } from '@/utils/syncUtil'
import { isSubgraph } from '@/utils/typeGuardUtil'
import { ComfyWorkflow } from './comfyWorkflow'
@@ -260,7 +260,7 @@ export const useWorkflowStore = defineStore('workflow', () => {
? (JSON.parse(JSON.stringify(workflowData)) as ComfyWorkflowJSON)
: (JSON.parse(defaultGraphJSON) as ComfyWorkflowJSON)
if (!base.id) {
if (!isValidUuid(base.id)) {
base.id = generateUUID()
}