From e927025ee349febcc1024f305b5e90a95dacd365 Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Mon, 18 May 2026 15:23:23 -0700 Subject: [PATCH 1/4] refactor: unify image editor upload contract Collapse the OSS vs Cloud branching in the mask editor and painter so both runtimes use the same upload contract: POST to /upload/image with type "input" and no subfolder, then reference the result by filename only. - maskeditor: replace separate uploadMask + uploadImage helpers with a single uploadLayer; drop the /upload/mask call, the original_ref form field, and the clipspace subfolder - painter: drop the isCloud branch on type/subfolder/result string; always upload as type "input" with no subfolder - update painter test to match the unified result shape --- .../maskeditor/useMaskEditorSaver.ts | 81 +++---------------- src/composables/painter/usePainter.test.ts | 2 +- src/composables/painter/usePainter.ts | 8 +- 3 files changed, 14 insertions(+), 77 deletions(-) diff --git a/src/composables/maskeditor/useMaskEditorSaver.ts b/src/composables/maskeditor/useMaskEditorSaver.ts index 17b82f8c5f..91e4c1bc25 100644 --- a/src/composables/maskeditor/useMaskEditorSaver.ts +++ b/src/composables/maskeditor/useMaskEditorSaver.ts @@ -6,7 +6,6 @@ import type { EditorOutputLayer, ImageRef } from '@/stores/maskEditorDataStore' -import { isCloud } from '@/platform/distribution/types' import { api } from '@/scripts/api' import { app } from '@/scripts/app' import { createAnnotatedPath } from '@/utils/createAnnotatedPath' @@ -209,18 +208,11 @@ export function useMaskEditorSaver() { } async function uploadAllLayers(outputData: EditorOutputData): Promise { - const sourceRef = dataStore.inputData!.sourceRef - - const actualMaskedRef = await uploadMask(outputData.maskedImage, sourceRef) - const actualPaintRef = await uploadImage(outputData.paintLayer, sourceRef) - const actualPaintedRef = await uploadImage( - outputData.paintedImage, - sourceRef - ) - - const actualPaintedMaskedRef = await uploadMask( - outputData.paintedMaskedImage, - actualPaintedRef + const actualMaskedRef = await uploadLayer(outputData.maskedImage) + const actualPaintRef = await uploadLayer(outputData.paintLayer) + const actualPaintedRef = await uploadLayer(outputData.paintedImage) + const actualPaintedMaskedRef = await uploadLayer( + outputData.paintedMaskedImage ) outputData.maskedImage.ref = actualMaskedRef @@ -229,50 +221,10 @@ export function useMaskEditorSaver() { outputData.paintedMaskedImage.ref = actualPaintedMaskedRef } - async function uploadMask( - layer: EditorOutputLayer, - originalRef: ImageRef - ): Promise { + async function uploadLayer(layer: EditorOutputLayer): Promise { const formData = new FormData() formData.append('image', layer.blob, layer.ref.filename) - formData.append('original_ref', JSON.stringify(originalRef)) formData.append('type', 'input') - formData.append('subfolder', 'clipspace') - - const response = await api.fetchApi('/upload/mask', { - method: 'POST', - body: formData - }) - - if (!response.ok) { - throw new Error(`Failed to upload mask: ${layer.ref.filename}`) - } - - try { - const data = await response.json() - if (data?.name) { - return { - filename: data.name, - subfolder: data.subfolder || layer.ref.subfolder, - type: data.type || layer.ref.type - } - } - } catch (error) { - console.warn('[MaskEditorSaver] Failed to parse upload response:', error) - } - - return layer.ref - } - - async function uploadImage( - layer: EditorOutputLayer, - originalRef: ImageRef - ): Promise { - const formData = new FormData() - formData.append('image', layer.blob, layer.ref.filename) - formData.append('original_ref', JSON.stringify(originalRef)) - formData.append('type', 'input') - formData.append('subfolder', 'clipspace') const response = await api.fetchApi('/upload/image', { method: 'POST', @@ -280,7 +232,7 @@ export function useMaskEditorSaver() { }) if (!response.ok) { - throw new Error(`Failed to upload image: ${layer.ref.filename}`) + throw new Error(`Failed to upload: ${layer.ref.filename}`) } try { @@ -288,8 +240,8 @@ export function useMaskEditorSaver() { if (data?.name) { return { filename: data.name, - subfolder: data.subfolder || layer.ref.subfolder, - type: data.type || layer.ref.type + subfolder: data.subfolder || '', + type: data.type || 'input' } } } catch (error) { @@ -322,19 +274,8 @@ export function useMaskEditorSaver() { const imageWidget = node.widgets?.find((w) => w.name === 'image') if (imageWidget) { - // Widget value format differs between Cloud and OSS: - // - Cloud: JUST the filename (subfolder handled by backend) - // - OSS: subfolder/filename (traditional format) - let widgetValue: string - if (isCloud) { - widgetValue = - mainRef.filename + (mainRef.type ? ` [${mainRef.type}]` : '') - } else { - widgetValue = - (mainRef.subfolder ? mainRef.subfolder + '/' : '') + - mainRef.filename + - (mainRef.type ? ` [${mainRef.type}]` : '') - } + const widgetValue = + mainRef.filename + (mainRef.type ? ` [${mainRef.type}]` : '') imageWidget.value = widgetValue diff --git a/src/composables/painter/usePainter.test.ts b/src/composables/painter/usePainter.test.ts index ef70fc7410..1777149b8f 100644 --- a/src/composables/painter/usePainter.test.ts +++ b/src/composables/painter/usePainter.test.ts @@ -384,7 +384,7 @@ describe('usePainter', () => { '/upload/image', expect.objectContaining({ method: 'POST' }) ) - expect(result).toBe('painter/uploaded.png [temp]') + expect(result).toBe('uploaded.png [input]') }) it('returns existing modelValue when canvas element is unmounted at serialize time', async () => { diff --git a/src/composables/painter/usePainter.ts b/src/composables/painter/usePainter.ts index 4fa623c331..fbefaed628 100644 --- a/src/composables/painter/usePainter.ts +++ b/src/composables/painter/usePainter.ts @@ -12,7 +12,6 @@ import { hexToRgb } from '@/utils/colorUtil' import type { Point } from '@/extensions/core/maskeditor/types' import type { LGraphNode } from '@/lib/litegraph/src/LGraphNode' import type { IBaseWidget } from '@/lib/litegraph/src/types/widgets' -import { isCloud } from '@/platform/distribution/types' import { useToastStore } from '@/platform/updates/common/toastStore' import { api } from '@/scripts/api' import { app } from '@/scripts/app' @@ -631,8 +630,7 @@ export function usePainter(nodeId: string, options: UsePainterOptions) { const name = `painter-${nodeId}-${Date.now()}.png` const body = new FormData() body.append('image', blob, name) - if (!isCloud) body.append('subfolder', 'painter') - body.append('type', isCloud ? 'input' : 'temp') + body.append('type', 'input') let resp: Response try { @@ -670,9 +668,7 @@ export function usePainter(nodeId: string, options: UsePainterOptions) { throw new Error(err) } - const result = isCloud - ? `${data.name} [input]` - : `painter/${data.name} [temp]` + const result = `${data.name} [input]` modelValue.value = result isDirty.value = false return result From 45402c6b825a80d4ee0904c03e5a5950710e9446 Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Mon, 18 May 2026 15:35:26 -0700 Subject: [PATCH 2/4] fix: throw on malformed upload response instead of silent stale ref Both uploadLayer (maskeditor) and the painter's serialize path previously fell through to the pre-upload layer.ref when the server returned 200 with no name or with a non-JSON body, logging only a console.warn. The caller then assigned the stale ref onto the saved widget value as if the upload had succeeded, producing graphs that pointed at files that were never written. - maskeditor: rewrite uploadLayer to throw on JSON parse failure and on missing data.name (was: console.warn + return layer.ref) - painter: add a missing-name guard alongside the existing JSON parse guard, surfacing the same toast/error path the JSON failure already uses - painter test: assert the upload FormData carries type=input with no subfolder, and add coverage for the two new error branches --- .../maskeditor/useMaskEditorSaver.ts | 28 ++++++---- src/composables/painter/usePainter.test.ts | 56 +++++++++++++++++++ src/composables/painter/usePainter.ts | 11 +++- 3 files changed, 84 insertions(+), 11 deletions(-) diff --git a/src/composables/maskeditor/useMaskEditorSaver.ts b/src/composables/maskeditor/useMaskEditorSaver.ts index 91e4c1bc25..1cdc421b57 100644 --- a/src/composables/maskeditor/useMaskEditorSaver.ts +++ b/src/composables/maskeditor/useMaskEditorSaver.ts @@ -235,20 +235,28 @@ export function useMaskEditorSaver() { throw new Error(`Failed to upload: ${layer.ref.filename}`) } + let data: { name?: string; subfolder?: string; type?: string } try { - const data = await response.json() - if (data?.name) { - return { - filename: data.name, - subfolder: data.subfolder || '', - type: data.type || 'input' - } - } + data = await response.json() } catch (error) { - console.warn('[MaskEditorSaver] Failed to parse upload response:', error) + throw new Error( + `Invalid upload response for ${layer.ref.filename}: ${ + error instanceof Error ? error.message : String(error) + }` + ) } - return layer.ref + if (!data?.name) { + throw new Error( + `Upload response missing 'name' for ${layer.ref.filename}` + ) + } + + return { + filename: data.name, + subfolder: data.subfolder || '', + type: data.type || 'input' + } } async function updateNodePreview( diff --git a/src/composables/painter/usePainter.test.ts b/src/composables/painter/usePainter.test.ts index 1777149b8f..892665518f 100644 --- a/src/composables/painter/usePainter.test.ts +++ b/src/composables/painter/usePainter.test.ts @@ -385,6 +385,62 @@ describe('usePainter', () => { expect.objectContaining({ method: 'POST' }) ) expect(result).toBe('uploaded.png [input]') + + const [, init] = fetchApiMock.mock.calls[0] + const body = init?.body as FormData + expect(body).toBeInstanceOf(FormData) + expect(body.get('type')).toBe('input') + expect(body.get('subfolder')).toBeNull() + }) + + it('throws when the upload response is missing a name', async () => { + const maskWidget = makeWidget('mask', '') + mockWidgets.push(maskWidget) + + vi.mocked(api.fetchApi).mockResolvedValueOnce({ + status: 200, + json: async () => ({}) + } as Response) + + const fakeCanvas = { + width: 4, + height: 4, + toBlob: (cb: BlobCallback) => cb(new Blob(['x'])) + } as unknown as HTMLCanvasElement + + const { canvasEl } = mountPainter('test-node', '') + canvasEl.value = fakeCanvas + await nextTick() + + await expect( + maskWidget.serializeValue!({} as LGraphNode, 0) + ).rejects.toThrow(/painter\.uploadError/) + }) + + it('throws when the upload response body is not valid JSON', async () => { + const maskWidget = makeWidget('mask', '') + mockWidgets.push(maskWidget) + + vi.mocked(api.fetchApi).mockResolvedValueOnce({ + status: 200, + json: async () => { + throw new SyntaxError('Unexpected token') + } + } as unknown as Response) + + const fakeCanvas = { + width: 4, + height: 4, + toBlob: (cb: BlobCallback) => cb(new Blob(['x'])) + } as unknown as HTMLCanvasElement + + const { canvasEl } = mountPainter('test-node', '') + canvasEl.value = fakeCanvas + await nextTick() + + await expect( + maskWidget.serializeValue!({} as LGraphNode, 0) + ).rejects.toThrow(/painter\.uploadError/) }) it('returns existing modelValue when canvas element is unmounted at serialize time', async () => { diff --git a/src/composables/painter/usePainter.ts b/src/composables/painter/usePainter.ts index fbefaed628..31c43a7db7 100644 --- a/src/composables/painter/usePainter.ts +++ b/src/composables/painter/usePainter.ts @@ -656,7 +656,7 @@ export function usePainter(nodeId: string, options: UsePainterOptions) { throw new Error(err) } - let data: { name: string } + let data: { name?: string } try { data = await resp.json() } catch (e) { @@ -668,6 +668,15 @@ export function usePainter(nodeId: string, options: UsePainterOptions) { throw new Error(err) } + if (!data?.name) { + const err = t('painter.uploadError', { + status: resp.status, + statusText: "missing 'name' in response" + }) + toastStore.addAlert(err) + throw new Error(err) + } + const result = `${data.name} [input]` modelValue.value = result isDirty.value = false From 5809f551717933fd18ee3f1a9cd1ba1442023e43 Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Mon, 18 May 2026 15:47:20 -0700 Subject: [PATCH 3/4] test: drop dead /upload/mask interceptors from maskEditor e2e After the upload-contract unification, the mask editor saves all four layers through /upload/image. The Playwright route interceptors for /upload/mask in the save-success and save-failure tests no longer fire. - save-success: remove the mask interceptor, tighten the count assertion from "> 0" to "=== 4" (one upload per layer) - save-failure: drop the unused mask interceptor; the image-route 500 is sufficient to keep the dialog open --- browser_tests/tests/maskEditor.spec.ts | 26 +++++--------------------- 1 file changed, 5 insertions(+), 21 deletions(-) diff --git a/browser_tests/tests/maskEditor.spec.ts b/browser_tests/tests/maskEditor.spec.ts index badf78e621..e6c8d7a68a 100644 --- a/browser_tests/tests/maskEditor.spec.ts +++ b/browser_tests/tests/maskEditor.spec.ts @@ -218,21 +218,8 @@ test.describe('Mask Editor', { tag: '@vue-nodes' }, () => { }) => { const dialog = await maskEditor.openDialog() - let maskUploadCount = 0 let imageUploadCount = 0 - await comfyPage.page.route('**/upload/mask', (route) => { - maskUploadCount++ - return route.fulfill({ - status: 200, - contentType: 'application/json', - body: JSON.stringify({ - name: `test-mask-${maskUploadCount}.png`, - subfolder: 'clipspace', - type: 'input' - }) - }) - }) await comfyPage.page.route('**/upload/image', (route) => { imageUploadCount++ return route.fulfill({ @@ -252,20 +239,17 @@ test.describe('Mask Editor', { tag: '@vue-nodes' }, () => { await expect(dialog).toBeHidden() - // The save pipeline uploads multiple layers (mask + image variants) + // The save pipeline uploads four layers (masked, paint, painted, paintedMasked) + // through the unified /upload/image endpoint. expect( - maskUploadCount + imageUploadCount, - 'save should trigger upload calls' - ).toBeGreaterThan(0) + imageUploadCount, + 'save should upload all four layers via /upload/image' + ).toBe(4) }) test('save failure keeps dialog open', async ({ comfyPage, maskEditor }) => { const dialog = await maskEditor.openDialog() - // Fail all upload routes - await comfyPage.page.route('**/upload/mask', (route) => - route.fulfill({ status: 500 }) - ) await comfyPage.page.route('**/upload/image', (route) => route.fulfill({ status: 500 }) ) From 7f9804e2bdd003242a1c6fb25ce9a62b52f786c9 Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Mon, 18 May 2026 17:08:19 -0700 Subject: [PATCH 4/4] refactor: use UploadImageResponse type from @comfyorg/ingest-types Replace the hand-rolled { name?: string ... } shapes in the painter and mask editor with UploadImageResponse from the codegen'd ingest-types package. The runtime shape is unchanged - the type now flows from the spec via the existing codegen pipeline instead of being declared inline locally. The runtime check for data.name remains: the spec marks the 200 response properties as optional, so the imported type still has name?: string. The check enforces what the server actually returns in practice; tightening the spec is a separate follow-up. --- src/composables/maskeditor/useMaskEditorSaver.ts | 4 +++- src/composables/painter/usePainter.ts | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/composables/maskeditor/useMaskEditorSaver.ts b/src/composables/maskeditor/useMaskEditorSaver.ts index 1cdc421b57..6128996662 100644 --- a/src/composables/maskeditor/useMaskEditorSaver.ts +++ b/src/composables/maskeditor/useMaskEditorSaver.ts @@ -1,3 +1,5 @@ +import type { UploadImageResponse } from '@comfyorg/ingest-types' + import { useMaskEditorDataStore } from '@/stores/maskEditorDataStore' import { useMaskEditorStore } from '@/stores/maskEditorStore' import { useNodeOutputStore } from '@/stores/nodeOutputStore' @@ -235,7 +237,7 @@ export function useMaskEditorSaver() { throw new Error(`Failed to upload: ${layer.ref.filename}`) } - let data: { name?: string; subfolder?: string; type?: string } + let data: UploadImageResponse try { data = await response.json() } catch (error) { diff --git a/src/composables/painter/usePainter.ts b/src/composables/painter/usePainter.ts index 31c43a7db7..fcb5576e88 100644 --- a/src/composables/painter/usePainter.ts +++ b/src/composables/painter/usePainter.ts @@ -1,3 +1,4 @@ +import type { UploadImageResponse } from '@comfyorg/ingest-types' import type { Ref } from 'vue' import { computed, onMounted, onUnmounted, ref, watch } from 'vue' import { useElementSize } from '@vueuse/core' @@ -656,7 +657,7 @@ export function usePainter(nodeId: string, options: UsePainterOptions) { throw new Error(err) } - let data: { name?: string } + let data: UploadImageResponse try { data = await resp.json() } catch (e) {