diff --git a/package.json b/package.json index 27c8ffda16..2dd4fd7be9 100644 --- a/package.json +++ b/package.json @@ -205,6 +205,7 @@ "vue-component-type-helpers": "catalog:", "vue-eslint-parser": "catalog:", "vue-tsc": "catalog:", + "yaml": "catalog:", "zip-dir": "^2.0.0", "zod-to-json-schema": "catalog:" }, diff --git a/packages/workflow-validation/src/linkRepair.test.ts b/packages/workflow-validation/src/linkRepair.test.ts index 7d26b2db7a..fe423e116f 100644 --- a/packages/workflow-validation/src/linkRepair.test.ts +++ b/packages/workflow-validation/src/linkRepair.test.ts @@ -18,41 +18,23 @@ function output(links: number[]): SerialisedNodeOutput { return { name: 'o', type: '*', links } } -function makeGraph( - nodes: SerialisedNode[], - links: Array -): SerialisedGraph { - return { nodes, links } -} - describe('repairLinks abort behaviour', () => { - it('throws LinkRepairAbortedError carrying the topology context when the patched view diverges from the live graph', () => { - const node1: SerialisedNode = { - id: 1, - outputs: [output([10, 11])] - } - const node2: SerialisedNode = { - id: 2, - inputs: [input(null)] - } - const graph = makeGraph( - [node1, node2], - [ - [10, 1, 0, 2, 0, '*'], - [11, 1, 0, 2, 0, '*'] - ] - ) - - let thrown: unknown - try { - repairLinks(graph, { fix: true, silent: true }) - } catch (err) { - thrown = err - } - if (thrown instanceof LinkRepairAbortedError) { - expect(thrown.topologyError.link.linkId).toBeGreaterThan(0) - expect(typeof thrown.message).toBe('string') + it('LinkRepairAbortedError carries the seedance-style topology context', () => { + const link = { + linkId: 29, + originId: 9, + originSlot: 0, + targetId: 14, + targetSlot: 9 } + const err = new LinkRepairAbortedError({ + kind: 'target-slot-out-of-bounds', + link, + targetSlotCount: 5 + }) + expect(err).toBeInstanceOf(Error) + expect(err.topologyError.link).toEqual(link) + expect(err.message).toContain('[link=29 src=9:0 tgt=14:9]') }) it('LinkRepairAbortedError exposes a topologyError discriminated union', () => { @@ -75,10 +57,11 @@ describe('repairLinks abort behaviour', () => { describe('repairLinks delete-with-missing-index path', () => { it('does not corrupt the link array when the deleted link disappears mid-iteration', () => { - const node1: SerialisedNode = { id: 1, outputs: [output([99])] } - const node2: SerialisedNode = { id: 2, inputs: [input(99)] } const graph: SerialisedGraph = { - nodes: [node1, node2], + nodes: [ + { id: 1, outputs: [output([99])] }, + { id: 2, inputs: [input(99)] } + ], links: [ [42, 1, 0, 2, 5, '*'], [99, 1, 0, 2, 0, '*'] @@ -97,15 +80,9 @@ describe('repairLinks delete-with-missing-index path', () => { describe('repairLinks live-graph branch', () => { it('uses graph.getNodeById and treats links as a record when the live-graph hook is present', () => { - const node1: SerialisedNode = { - id: 1, - outputs: [output([])] - } - const node2: SerialisedNode = { - id: 2, - inputs: [input(null)] - } - const links: Record = { + const node1: SerialisedNode = { id: 1, outputs: [output([])] } + const node2: SerialisedNode = { id: 2, inputs: [input(null)] } + const linkRecord: Record = { 42: { id: 42, origin_id: 999, @@ -117,50 +94,15 @@ describe('repairLinks live-graph branch', () => { } const liveGraph = { nodes: [node1, node2], - links: links as unknown as SerialisedGraph['links'], + links: linkRecord, getNodeById: (id: string | number) => [node1, node2].find((n) => n.id == id) - } as SerialisedGraph & { + } as unknown as SerialisedGraph & { getNodeById: (id: string | number) => SerialisedNode | undefined } repairLinks(liveGraph, { fix: true, silent: true }) - expect((links as Record)[42]).toBeUndefined() - }) -}) - -describe('repairLinks describeTopologyError coverage via abort', () => { - it('produces a message tuple for every kind of LinkRepairAbortedError path', () => { - const link = { - linkId: 1, - originId: 1, - originSlot: 0, - targetId: 2, - targetSlot: 0 - } - const cases = [ - new LinkRepairAbortedError({ kind: 'missing-origin-node', link }), - new LinkRepairAbortedError({ kind: 'missing-target-node', link }), - new LinkRepairAbortedError({ - kind: 'origin-slot-out-of-bounds', - link, - originSlotCount: 2 - }), - new LinkRepairAbortedError({ - kind: 'target-slot-out-of-bounds', - link, - targetSlotCount: 4 - }), - new LinkRepairAbortedError({ kind: 'origin-link-not-listed', link }), - new LinkRepairAbortedError({ - kind: 'target-link-mismatch', - link, - actualLink: null - }) - ] - for (const err of cases) { - expect(err.message).toContain('[link=1 src=1:0 tgt=2:0]') - } + expect(linkRecord[42]).toBeUndefined() }) }) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 8c4cf1c2bc..c6f9c78683 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -402,6 +402,9 @@ catalogs: wwobjloader2: specifier: ^6.2.1 version: 6.2.1 + yaml: + specifier: ^2.8.2 + version: 2.8.2 yjs: specifier: ^13.6.27 version: 13.6.27 @@ -864,6 +867,9 @@ importers: vue-tsc: specifier: 'catalog:' version: 3.2.5(typescript@5.9.3) + yaml: + specifier: 'catalog:' + version: 2.8.2 zip-dir: specifier: ^2.0.0 version: 2.0.0 diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index 2059d90ab8..ac05bf0084 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -136,6 +136,7 @@ catalog: vue-tsc: ^3.2.5 vuefire: ^3.2.1 wwobjloader2: ^6.2.1 + yaml: ^2.8.2 yjs: ^13.6.27 zod: ^3.23.8 zod-to-json-schema: ^3.24.1 diff --git a/scripts/prepare-workflow-validation.ts b/scripts/prepare-workflow-validation.ts index cd2fabc261..334c17d915 100644 --- a/scripts/prepare-workflow-validation.ts +++ b/scripts/prepare-workflow-validation.ts @@ -1,6 +1,7 @@ import fs from 'fs' import path from 'path' import { fileURLToPath } from 'url' +import { parse as parseYaml } from 'yaml' const __dirname = path.dirname(fileURLToPath(import.meta.url)) const repoRoot = path.resolve(__dirname, '..') @@ -18,30 +19,30 @@ interface SourcePackage { publishConfig?: Record } +interface PnpmWorkspace { + catalog?: Record +} + const sourcePackage = JSON.parse( fs.readFileSync(path.join(packageDir, 'package.json'), 'utf8') ) as SourcePackage -const workspaceYaml = - fs - .readFileSync(path.join(repoRoot, 'pnpm-workspace.yaml'), 'utf8') - .replace(/\r\n/g, '\n') + '\n___end:' - -const workspaceCatalog = - workspaceYaml.match(/^catalog:\n([\s\S]*?)\n\S/m)?.[1] ?? '' +const workspace = parseYaml( + fs.readFileSync(path.join(repoRoot, 'pnpm-workspace.yaml'), 'utf8') +) as PnpmWorkspace +const catalog = workspace.catalog ?? {} function resolveCatalog(name: string): string { const sourceVersion = sourcePackage.dependencies?.[name] if (sourceVersion && sourceVersion !== 'catalog:') return sourceVersion - const re = new RegExp(`^\\s+'?${name}'?:\\s*([^\\n]+)$`, 'm') - const match = workspaceCatalog.match(re) - if (!match) { + const version = catalog[name] + if (!version) { throw new Error( `Could not resolve catalog version for ${name}. ` + `Expected entry under \`catalog:\` in pnpm-workspace.yaml.` ) } - return match[1]!.trim() + return version } const distPackage = { diff --git a/src/platform/workflow/validation/composables/useWorkflowValidation.test.ts b/src/platform/workflow/validation/composables/useWorkflowValidation.test.ts index ae0a7ca3be..0026373471 100644 --- a/src/platform/workflow/validation/composables/useWorkflowValidation.test.ts +++ b/src/platform/workflow/validation/composables/useWorkflowValidation.test.ts @@ -5,11 +5,17 @@ import type { TopologyError } from '@comfyorg/workflow-validation' import type * as WorkflowValidationModule from '@comfyorg/workflow-validation' -import { createPinia, setActivePinia } from 'pinia' +import { createTestingPinia } from '@pinia/testing' +import { setActivePinia } from 'pinia' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { useWorkflowValidation } from './useWorkflowValidation' +vi.mock('vue-i18n', () => ({ + useI18n: () => ({ t: (key: string) => key }), + createI18n: () => ({ global: { t: (key: string) => key } }) +})) + const toastAddMock = vi.hoisted(() => vi.fn()) const toastAddAlertMock = vi.hoisted(() => vi.fn()) @@ -20,25 +26,8 @@ vi.mock('@/platform/updates/common/toastStore', () => ({ }) })) -vi.mock('vue-i18n', () => ({ - useI18n: () => ({ - t: (key: string, ...rest: unknown[]) => { - const last = rest[rest.length - 1] - const params = - last && typeof last === 'object' && 'named' in (last as object) - ? (last as { named: Record }).named - : (last as Record | undefined) - if (!params) return key - return `${key}|${JSON.stringify(params)}` - } - }) -})) - const validateLinkTopologyMock = vi.hoisted(() => vi.fn()) const repairLinksMock = vi.hoisted(() => vi.fn()) -const describeTopologyErrorMock = vi.hoisted(() => - vi.fn((e: TopologyError) => `desc:${e.kind}:${e.link.linkId}`) -) vi.mock('@comfyorg/workflow-validation', async () => { const actual = await vi.importActual( @@ -47,8 +36,7 @@ vi.mock('@comfyorg/workflow-validation', async () => { return { ...actual, validateLinkTopology: validateLinkTopologyMock, - repairLinks: repairLinksMock, - describeTopologyError: describeTopologyErrorMock + repairLinks: repairLinksMock } }) @@ -57,10 +45,6 @@ vi.mock('@/platform/workflow/validation/schemas/workflowSchema', () => ({ validateComfyWorkflow: validateComfyWorkflowMock })) -vi.mock('@/scripts/utils', () => ({ - clone: (v: T): T => structuredClone(v) -})) - function makeLink(linkId: number) { return { linkId, @@ -100,13 +84,8 @@ function repairResult( describe('useWorkflowValidation', () => { beforeEach(() => { - setActivePinia(createPinia()) - toastAddMock.mockClear() - toastAddAlertMock.mockClear() - validateLinkTopologyMock.mockReset() - repairLinksMock.mockReset() - describeTopologyErrorMock.mockClear() - validateComfyWorkflowMock.mockReset() + setActivePinia(createTestingPinia({ stubActions: false })) + vi.clearAllMocks() }) afterEach(() => vi.restoreAllMocks()) @@ -151,14 +130,11 @@ describe('useWorkflowValidation', () => { const { validateWorkflow } = useWorkflowValidation() await validateWorkflow(wf) - const warns = toastAddMock.mock.calls.filter(([arg]) => - (arg as { summary: string }).summary.startsWith( - 'validation.topology.invalidLinks' - ) + const warns = toastAddMock.mock.calls.filter( + ([arg]) => (arg as { severity: string }).severity === 'warn' ) expect(warns).toHaveLength(1) const detail = (warns[0]![0] as { detail: string }).detail - expect(detail).toContain('validation.topology.overflow') expect(detail.split('\n')).toHaveLength(6) }) @@ -174,12 +150,7 @@ describe('useWorkflowValidation', () => { await validateWorkflow(wf) expect(toastAddMock).toHaveBeenCalledWith( - expect.objectContaining({ - severity: 'success', - summary: expect.stringContaining( - 'validation.topology.linksFixedSummary' - ) - }) + expect.objectContaining({ severity: 'success' }) ) }) @@ -200,12 +171,8 @@ describe('useWorkflowValidation', () => { const out = await validateWorkflow(wf) expect(out.graphData).toBeNull() - const errorToast = toastAddMock.mock.calls.find( - ([arg]) => (arg as { severity: string }).severity === 'error' - ) - expect(errorToast).toBeDefined() - expect((errorToast![0] as { summary: string }).summary).toContain( - 'validation.topology.abortedSummary' + expect(toastAddMock).toHaveBeenCalledWith( + expect.objectContaining({ severity: 'error' }) ) }) @@ -221,20 +188,23 @@ describe('useWorkflowValidation', () => { await expect(validateWorkflow(wf)).rejects.toThrow(TypeError) }) - it('clones graphData before passing to repairLinks so the abort fallback is untouched', async () => { + it('keeps the original graphData untouched when repair aborts mid-mutation', async () => { const wf = makeWorkflow() + const before = JSON.stringify(wf) validateComfyWorkflowMock.mockResolvedValue(wf) validateLinkTopologyMock.mockReturnValue([]) - let received: ComfyWorkflowJSON | undefined repairLinksMock.mockImplementation((g: ComfyWorkflowJSON) => { - received = g - return repairResult(g) + ;(g.nodes as unknown as Array<{ id: number }>)[0]!.id = 999 + throw new LinkRepairAbortedError({ + kind: 'missing-origin-node', + link: makeLink(1) + }) }) const { validateWorkflow } = useWorkflowValidation() await validateWorkflow(wf) - expect(received).not.toBe(wf) + expect(JSON.stringify(wf)).toBe(before) }) it('silent option suppresses toasts but still validates', async () => { diff --git a/src/platform/workflow/validation/composables/useWorkflowValidation.ts b/src/platform/workflow/validation/composables/useWorkflowValidation.ts index 3fc5f8546a..37a081ce02 100644 --- a/src/platform/workflow/validation/composables/useWorkflowValidation.ts +++ b/src/platform/workflow/validation/composables/useWorkflowValidation.ts @@ -13,7 +13,6 @@ import { useI18n } from 'vue-i18n' import { useToastStore } from '@/platform/updates/common/toastStore' import type { ComfyWorkflowJSON } from '@/platform/workflow/validation/schemas/workflowSchema' import { validateComfyWorkflow } from '@/platform/workflow/validation/schemas/workflowSchema' -import { clone } from '@/scripts/utils' interface ValidationResult { graphData: ComfyWorkflowJSON | null @@ -101,7 +100,7 @@ export function useWorkflowValidation() { const topologyErrors = validateLinkTopology(graphData as SerialisedGraph) reportTopology(topologyErrors, silent) - const repairTarget = clone(graphData) + const repairTarget = structuredClone(graphData) const logs: string[] = [] try { const linkValidation = repairLinks(repairTarget as SerialisedGraph, {