Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
c5dd38f
refactor(assets): extract getAssetStoredFilename helper, add mockFeat…
dante01yoon May 15, 2026
4888326
[automated] Apply ESLint and Oxfmt fixes
actions-user May 15, 2026
e927025
refactor: unify image editor upload contract
mattmillerai May 18, 2026
45402c6
fix: throw on malformed upload response instead of silent stale ref
mattmillerai May 18, 2026
29a8650
Merge branch 'main' into matt/fe-750-unify-image-editor-upload-contract
mattmillerai May 18, 2026
5809f55
test: drop dead /upload/mask interceptors from maskEditor e2e
mattmillerai May 18, 2026
7f9804e
refactor: use UploadImageResponse type from @comfyorg/ingest-types
mattmillerai May 19, 2026
7f6d354
refactor(assets): delete isAssetAPIEnabled and Comfy.Assets.UseAssetAPI
dante01yoon May 19, 2026
fcdc440
[automated] Apply ESLint and Oxfmt fixes
actions-user May 19, 2026
b340265
refactor(assets): remove isAssetPreviewSupported wrapper and simplify…
dante01yoon May 19, 2026
09943f8
[automated] Apply ESLint and Oxfmt fixes
actions-user May 19, 2026
c3cde8d
refactor(asset-card): read image dimensions from typed metadata field
mattmillerai May 19, 2026
2a5de94
fix(asset-card): validate metadata dimensions and clarify fallback na…
mattmillerai May 19, 2026
5b1446a
refactor(asset-card): extract metadata dimension helper to utils
mattmillerai May 19, 2026
e48dcd1
ci(temp): enable --enable-assets on Playwright ComfyUI server
dante01yoon May 19, 2026
ec0711d
Merge branch 'main' into jaewon/fe-729-delete-is-asset-api-enabled
dante01yoon May 19, 2026
ffe8d0f
test(assets): drop OSS legacy sidebar tree tests (FE-729)
dante01yoon May 19, 2026
b38fab4
refactor(assets): remove isCloud forks in assetsStore + delete legacy…
dante01yoon May 19, 2026
1d71c93
[automated] Apply ESLint and Oxfmt fixes
actions-user May 20, 2026
0797b7a
test(builder): make selectInputWidget work in both grid and asset modes
dante01yoon May 20, 2026
c579c88
[automated] Apply ESLint and Oxfmt fixes
actions-user May 20, 2026
9b9b0f2
refactor(assets): remove isCloud guards in widget composables (FE-731)
dante01yoon May 20, 2026
c9d9ee1
[automated] Apply ESLint and Oxfmt fixes
actions-user May 20, 2026
55b0329
test(builder): use exclusive fallback instead of .or() in selectInput…
dante01yoon May 21, 2026
9864230
refactor(missingMedia): adopt file_path as primary detection key (FE-…
dante01yoon May 21, 2026
2184d25
refactor(assets): consume unified display_name across helpers (FE-747…
dante01yoon May 21, 2026
e35bb25
test(missingMedia): cover getLibraryOptions integration with getMedia…
dante01yoon May 21, 2026
2fd6725
docs(assets): tighten getMediaDisplayName and getAssetDisplayFilename…
dante01yoon May 21, 2026
77514a4
merge: FE-733 (PR #12287) into m1-fe-integration
dante01yoon May 22, 2026
3051c00
merge: FE-729 (PR #12322) into m1-fe-integration
dante01yoon May 22, 2026
4837307
merge: FE-730 (PR #12335) into m1-fe-integration
dante01yoon May 22, 2026
c244b53
merge: FE-731 (PR #12375) into m1-fe-integration
dante01yoon May 22, 2026
117d011
merge: FE-746 (PR #12398) into m1-fe-integration
dante01yoon May 22, 2026
4b0f6dc
merge: FE-749 (PR #12328) into m1-fe-integration
dante01yoon May 22, 2026
d45e641
merge: FE-750 (PR #12318) into m1-fe-integration
dante01yoon May 22, 2026
bec9e37
Merge branch 'main' into jaewon/fe-746-l3-fe-migrate-missingmediascan…
dante01yoon May 22, 2026
37f9a14
refactor(missingMedia): union file_path with legacy detection keys (C…
dante01yoon May 22, 2026
076cba9
refactor(missingMedia): soft-degrade independent asset oracles (Promi…
dante01yoon May 22, 2026
1510884
test(missingMedia): cover file_path union + soft-degrade in browser t…
dante01yoon May 22, 2026
d842269
merge: FE-746 (PR #12398) Case B + soft-degrade + e2e onto m1-fe-inte…
dante01yoon May 22, 2026
b50510f
merge: FE-747 (PR #12399) into m1-fe-integration
dante01yoon May 22, 2026
0a8f936
refactor(assets): remove isCloud guards on L1 UI surfaces (FE-732)
dante01yoon May 22, 2026
519aefd
test(assets): tighten FE-732 coverage on context menu + filter bar
dante01yoon May 22, 2026
752452b
merge: FE-732 (PR #12417) into m1-fe-integration
dante01yoon May 22, 2026
02b8cc6
test(assets): cover input-tab bulk Delete footer on OSS build (FE-732)
dante01yoon May 22, 2026
4acd50c
fix(widgets): refresh model assets and skip output refresh on asset-m…
dante01yoon May 26, 2026
4991802
merge: FE-732 update (PR #12465 fix + #12417 follow-up) into m1-fe-in…
dante01yoon May 26, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .github/actions/start-comfyui-server/action.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,8 @@ runs:
run: |
set -euo pipefail
cp -r ./tools/devtools/* /ComfyUI/custom_nodes/ComfyUI_devtools/
cd /ComfyUI && python3 main.py --cpu --multi-user --front-end-root "${{ inputs.front_end_root }}" &
# TODO(FE-729): remove --enable-assets once BE-786 lands in the CI ComfyUI image
# (BE-786 removes the gate so /api/assets is always on). Until then, FE-729
# routes modelStore through assetService, which 503s without this flag.
cd /ComfyUI && python3 main.py --cpu --multi-user --enable-assets --front-end-root "${{ inputs.front_end_root }}" &
wait-for-it --service 127.0.0.1:8188 -t ${{ inputs.timeout }}
42 changes: 42 additions & 0 deletions browser_tests/assets/missing/fe746_load_image_bare_filename.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
{
"last_node_id": 10,
"last_link_id": 0,
"nodes": [
{
"id": 10,
"type": "LoadImage",
"pos": [50, 200],
"size": [315, 314],
"flags": {},
"order": 0,
"mode": 0,
"inputs": [],
"outputs": [
{
"name": "IMAGE",
"type": "IMAGE",
"links": null
},
{
"name": "MASK",
"type": "MASK",
"links": null
}
],
"properties": {
"Node name for S&R": "LoadImage"
},
"widgets_values": ["fe746_photo.png", "image"]
}
],
"links": [],
"groups": [],
"config": {},
"extra": {
"ds": {
"offset": [0, 0],
"scale": 1
}
},
"version": 0.4
}
19 changes: 16 additions & 3 deletions browser_tests/fixtures/helpers/BuilderSelectHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,22 @@ export class BuilderSelectHelper {
)[0]
if (!nodeRef) throw new Error(`Node ${nodeTitle} not found`)
await nodeRef.centerOnNode()
const widgetLocator = this.comfyPage.vueNodes
.getNodeLocator(String(nodeRef.id))
.getByLabel(widgetName, { exact: true })
const node = this.comfyPage.vueNodes.getNodeLocator(String(nodeRef.id))
// Grid-mode widgets (WidgetSelectDefault) and number widgets expose
// aria-label on a wrapper/input. Asset-mode widgets (WidgetSelectDropdown)
// do not — the widget name lives in a sibling
// [data-testid="widget-layout-field-label"] div, so fall back to clicking
// the dropdown trigger button in the same row.
const byAriaLabel = node.getByLabel(widgetName, { exact: true })
const widgetLocator =
(await byAriaLabel.count()) > 0
? byAriaLabel
: node
.getByTestId('widget-layout-field-label')
.filter({ hasText: widgetName })
.locator('..')
.locator('button')
.first()
Comment on lines +128 to +137
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="browser_tests/fixtures/helpers/BuilderSelectHelper.ts"
if [ ! -f "$FILE" ]; then
  echo "Missing file: $FILE" >&2
  exit 1
fi

echo "== Lines 90-170 =="
nl -ba "$FILE" | sed -n '90,170p'
echo
echo "== Extract function selectInputWidget (best-effort) =="
# Try to locate the function body without dumping huge output
rg -n "selectInputWidget" "$FILE" || true
START=$(rg -n "selectInputWidget" "$FILE" | head -n1 | cut -d: -f1 || true)
if [ -n "${START:-}" ]; then
  # Print from start to next closing brace-ish region; keep bounded
  END=$((START+140))
  nl -ba "$FILE" | sed -n "${START},${END}p"
fi

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 151


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="browser_tests/fixtures/helpers/BuilderSelectHelper.ts"
if [ ! -f "$FILE" ]; then
  echo "Missing file: $FILE" >&2
  exit 1
fi

echo "== Showing lines 110-160 =="
awk 'NR>=110 && NR<=160 {printf "%5d\t%s\n", NR, $0}' "$FILE"
echo

echo "== Finding selectInputWidget and widgetLocator usage =="
rg -n "selectInputWidget|widgetLocator" "$FILE" || true
echo

echo "== Finding any widgetLocator.click() usage =="
rg -n "widgetLocator\.click\(" "$FILE" || true
echo

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 2840


Fix strict-mode/ambiguity risks in selectInputWidget locator selection (and avoid extra comment)

  • When byAriaLabel.count() > 0, widgetLocator is set to byAriaLabel without narrowing to a single element; if the label matches multiple nodes, await widgetLocator.click({ force: true }) can throw strict-mode errors.
  • The fallback filter({ hasText: widgetName }) can match the wrong “field-label” when widget names overlap; use exact text matching for the intended row/button.
  • Remove or refactor the explanatory comment block (123–127) to keep the code self-documenting per repo guidelines.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@browser_tests/fixtures/helpers/BuilderSelectHelper.ts` around lines 128 -
137, In selectInputWidget, tighten ambiguous locators: change byAriaLabel to use
.first() (e.g., const byAriaLabel = node.getByLabel(widgetName, { exact: true
}).first()) so it always returns a single locator, and make the fallback filter
use exact text matching for the row/button (e.g., locate the field-label row by
matching the label/button text exactly via getByText(..., { exact: true }) or a
anchored RegExp like ^…$ then .locator('..').locator('button').first()). Remove
the explanatory comment block above this logic so the code remains
self-documenting; update references to byAriaLabel and widgetLocator in the
click call accordingly.

// oxlint-disable-next-line playwright/no-force-option -- Node container has conditional pointer-events:none that blocks actionability
await widgetLocator.click({ force: true })
await this.comfyPage.nextFrame()
Expand Down
171 changes: 90 additions & 81 deletions browser_tests/tests/assetDeleteClearsLoadImage.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@
* FE-230: Deleting an asset must clear the Load Image node preview, widget
* value, and mark the workflow dirty.
*
* Local run (requires cloud build of the frontend):
* pnpm build:cloud
* pnpm exec playwright test --project=cloud \
* browser_tests/tests/assetDeleteClearsLoadImage.spec.ts --reporter=list
* FE-732: Input-asset deletion is no longer gated on `isCloud`; the same
* teardown flow now applies to both Cloud and OSS builds. Cloud and OSS
* variants below cover both Playwright projects against the shared mock.
*
* The cloud project is required because input-asset deletion is gated on
* `isCloud === true` (see `useMediaAssetActions.deleteAssetApi`).
* Local run examples:
* pnpm build:cloud && pnpm exec playwright test --project=cloud \
* browser_tests/tests/assetDeleteClearsLoadImage.spec.ts --reporter=list
* pnpm build && pnpm exec playwright test --project=chromium \
* browser_tests/tests/assetDeleteClearsLoadImage.spec.ts --reporter=list
*/
import type { Page, Route } from '@playwright/test'
import { expect } from '@playwright/test'
Expand Down Expand Up @@ -119,81 +121,88 @@ const baseTest = comfyPageFixture.extend<{ assetMock: AssetMockApi }>({
}
})

function registerDeleteFlowTest() {
baseTest(
'deleting an input asset clears widget value, preview cache, and marks workflow modified',
async ({ comfyPage, assetMock }) => {
await comfyPage.workflow.loadWorkflow('widgets/load_image_widget')

// Drive the production drag-and-drop flow to point the Load Image
// widget at the asset we are about to delete and populate the preview
// cache. FE-230 is asserting that the deletion tears these down.
const loadImageNode = (
await comfyPage.nodeOps.getNodeRefsByType('LoadImage')
)[0]
const { x, y } = await loadImageNode.getPosition()
await comfyPage.dragDrop.dragAndDropFile(DROPPED_FILE, {
dropPosition: { x, y },
waitForUpload: true
})
const imageWidget = await loadImageNode.getWidget(0)
await expect.poll(() => imageWidget.getValue()).toBe(DROPPED_FILE)

// Re-baseline the change tracker so the deletion-side mutation is the
// only thing that can flip `isModified` later.
await comfyPage.page.evaluate(() => {
const tracker =
window.app?.extensionManager?.workflow?.activeWorkflow?.changeTracker
tracker?.reset?.()
})
await expect
.poll(() => comfyPage.workflow.isCurrentWorkflowModified())
.toBe(false)

// Drive the real production flow: assets sidebar → Imported tab →
// right-click asset card → Delete → confirm dialog.
const sidebar = comfyPage.menu.assetsTab
// The default `open()` waits for assets on the Generated tab; we seed
// only an input asset, so skip that wait and let `waitForAssets(1)`
// gate on the Imported tab instead.
await sidebar.open({ waitForAssets: false })
await sidebar.switchToImported()
await sidebar.waitForAssets(1)
await sidebar.rightClickAsset(TARGET_CARD_TEXT)

const deleteMenuItem = sidebar.contextMenuItem('Delete')
await expect(deleteMenuItem).toBeVisible()
await deleteMenuItem.click()

await comfyPage.confirmDialog.click('delete')

// Mocked DELETE was issued.
await expect
.poll(() => assetMock.deleteCalls.includes(TARGET_ASSET.id))
.toBe(true)

// Widget value was cleared.
await expect.poll(() => imageWidget.getValue()).toBe('')

// Preview cache was cleared.
await expect
.poll(() =>
comfyPage.page.evaluate((nodeId) => {
const node = window.app!.graph.getNodeById(nodeId)
return node?.imgs?.length ?? 0
}, loadImageNode.id)
)
.toBe(0)

// Workflow was marked dirty by changeTracker.captureCanvasState().
await expect
.poll(() => comfyPage.workflow.isCurrentWorkflowModified())
.toBe(true)
}
)
}

baseTest.describe(
'FE-230 asset delete clears Load Image preview',
'FE-230 asset delete clears Load Image preview (cloud)',
{ tag: '@cloud' },
() => {
baseTest(
'deleting an input asset clears widget value, preview cache, and marks workflow modified',
async ({ comfyPage, assetMock }) => {
await comfyPage.workflow.loadWorkflow('widgets/load_image_widget')

// Drive the production drag-and-drop flow to point the Load Image
// widget at the asset we are about to delete and populate the preview
// cache. FE-230 is asserting that the deletion tears these down.
const loadImageNode = (
await comfyPage.nodeOps.getNodeRefsByType('LoadImage')
)[0]
const { x, y } = await loadImageNode.getPosition()
await comfyPage.dragDrop.dragAndDropFile(DROPPED_FILE, {
dropPosition: { x, y },
waitForUpload: true
})
const imageWidget = await loadImageNode.getWidget(0)
await expect.poll(() => imageWidget.getValue()).toBe(DROPPED_FILE)

// Re-baseline the change tracker so the deletion-side mutation is the
// only thing that can flip `isModified` later.
await comfyPage.page.evaluate(() => {
const tracker =
window.app?.extensionManager?.workflow?.activeWorkflow
?.changeTracker
tracker?.reset?.()
})
await expect
.poll(() => comfyPage.workflow.isCurrentWorkflowModified())
.toBe(false)

// Drive the real production flow: assets sidebar → Imported tab →
// right-click asset card → Delete → confirm dialog.
const sidebar = comfyPage.menu.assetsTab
// The default `open()` waits for assets on the Generated tab; we seed
// only an input asset, so skip that wait and let `waitForAssets(1)`
// gate on the Imported tab instead.
await sidebar.open({ waitForAssets: false })
await sidebar.switchToImported()
await sidebar.waitForAssets(1)
await sidebar.rightClickAsset(TARGET_CARD_TEXT)

const deleteMenuItem = sidebar.contextMenuItem('Delete')
await expect(deleteMenuItem).toBeVisible()
await deleteMenuItem.click()

await comfyPage.confirmDialog.click('delete')

// Mocked DELETE was issued.
await expect
.poll(() => assetMock.deleteCalls.includes(TARGET_ASSET.id))
.toBe(true)

// Widget value was cleared.
await expect.poll(() => imageWidget.getValue()).toBe('')

// Preview cache was cleared.
await expect
.poll(() =>
comfyPage.page.evaluate((nodeId) => {
const node = window.app!.graph.getNodeById(nodeId)
return node?.imgs?.length ?? 0
}, loadImageNode.id)
)
.toBe(0)

// Workflow was marked dirty by changeTracker.captureCanvasState().
await expect
.poll(() => comfyPage.workflow.isCurrentWorkflowModified())
.toBe(true)
}
)
}
registerDeleteFlowTest
)

baseTest.describe(
'FE-230 asset delete clears Load Image preview (oss)',
{ tag: '@oss' },
registerDeleteFlowTest
)
1 change: 0 additions & 1 deletion browser_tests/tests/defaultKeybindings.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ test.describe('Default Keybindings', { tag: '@keyboard' }, () => {
const sidebarTabs = [
{ key: 'KeyW', tabId: 'workflows', label: 'workflows' },
{ key: 'KeyN', tabId: 'node-library', label: 'node library' },
{ key: 'KeyM', tabId: 'model-library', label: 'model library' },
{ key: 'KeyA', tabId: 'assets', label: 'assets' }
] as const

Expand Down
26 changes: 5 additions & 21 deletions browser_tests/tests/maskEditor.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand All @@ -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 })
)
Expand Down
Loading
Loading