Merge remote-tracking branch 'origin/main' into fix/coderabbit-issue-9226

## Summary

Add E2E Playwright tests for the asset delete confirmation dialog flow.

## Changes

- **What**: New `Assets sidebar - delete confirmation` describe block in
`assets.spec.ts` covering right-click delete showing confirmation
dialog, confirming delete removes asset with success toast, and
cancelling delete preserves asset. Added `mockDeleteHistory()` to
`AssetsHelper` to intercept POST `/api/history` delete payloads and
update mock state.

## Review Focus

Tests use existing `ConfirmDialog` page object and `AssetsHelper` mock
infrastructure. The `mockDeleteHistory` handler removes jobs from the
in-memory mock array so subsequent `/api/jobs` fetches reflect the
deletion.

Fixes #10781

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10785-test-assets-add-E2E-tests-for-delete-confirmation-flow-3356d73d365081fb90c8e2a69de3a666)
by [Unito](https://www.unito.io)

browser_tests/fixtures/ComfyPage.ts

@@ -14,9 +14,12 @@ import { VueNodeHelpers } from '@e2e/fixtures/VueNodeHelpers'
 import { BottomPanel } from '@e2e/fixtures/components/BottomPanel'
 import { ComfyNodeSearchBox } from '@e2e/fixtures/components/ComfyNodeSearchBox'
 import { ComfyNodeSearchBoxV2 } from '@e2e/fixtures/components/ComfyNodeSearchBoxV2'
+import { ConfirmDialog } from '@e2e/fixtures/components/ConfirmDialog'
 import { ContextMenu } from '@e2e/fixtures/components/ContextMenu'
+import { MediaLightbox } from '@e2e/fixtures/components/MediaLightbox'
 import { QueuePanel } from '@e2e/fixtures/components/QueuePanel'
 import { SettingDialog } from '@e2e/fixtures/components/SettingDialog'
+import { TemplatesDialog } from '@e2e/fixtures/components/TemplatesDialog'
 import {
   AssetsSidebarTab,
   ModelLibrarySidebarTab,
@@ -131,48 +134,6 @@ class ComfyMenu {
   }
 }
 
-type KeysOfType<T, Match> = {
-  [K in keyof T]: T[K] extends Match ? K : never
-}[keyof T]
-
-class ConfirmDialog {
-  public readonly root: Locator
-  public readonly delete: Locator
-  public readonly overwrite: Locator
-  public readonly reject: Locator
-  public readonly confirm: Locator
-
-  constructor(public readonly page: Page) {
-    this.root = page.getByRole('dialog')
-    this.delete = this.root.getByRole('button', { name: 'Delete' })
-    this.overwrite = this.root.getByRole('button', { name: 'Overwrite' })
-    this.reject = this.root.getByRole('button', { name: 'Cancel' })
-    this.confirm = this.root.getByRole('button', { name: 'Confirm' })
-  }
-
-  async click(locator: KeysOfType<ConfirmDialog, Locator>) {
-    const loc = this[locator]
-    await loc.waitFor({ state: 'visible' })
-    await loc.click()
-
-    // Wait for the dialog mask to disappear after confirming
-    const mask = this.page.locator('.p-dialog-mask')
-    const count = await mask.count()
-    if (count > 0) {
-      await mask.first().waitFor({ state: 'hidden', timeout: 3000 })
-    }
-
-    // Wait for workflow service to finish if it's busy
-    await this.page.waitForFunction(
-      () =>
-        (window.app?.extensionManager as WorkspaceStore | undefined)?.workflow
-          ?.isBusy === false,
-      undefined,
-      { timeout: 3000 }
-    )
-  }
-}
-
 export class ComfyPage {
   public readonly url: string
   // All canvas position operations are based on default view of canvas.
@@ -196,6 +157,8 @@ export class ComfyPage {
   public readonly templates: ComfyTemplates
   public readonly settingDialog: SettingDialog
   public readonly confirmDialog: ConfirmDialog
+  public readonly templatesDialog: TemplatesDialog
+  public readonly mediaLightbox: MediaLightbox
   public readonly vueNodes: VueNodeHelpers
   public readonly appMode: AppModeHelper
   public readonly subgraph: SubgraphHelper
@@ -244,6 +207,8 @@ export class ComfyPage {
     this.templates = new ComfyTemplates(page)
     this.settingDialog = new SettingDialog(page, this)
     this.confirmDialog = new ConfirmDialog(page)
+    this.templatesDialog = new TemplatesDialog(page)
+    this.mediaLightbox = new MediaLightbox(page)
     this.vueNodes = new VueNodeHelpers(page)
     this.appMode = new AppModeHelper(this)
     this.subgraph = new SubgraphHelper(this)

browser_tests/fixtures/components/ConfirmDialog.ts

@@ -0,0 +1,44 @@
+import type { Locator, Page } from '@playwright/test'
+
+import type { WorkspaceStore } from '../../types/globals'
+
+type KeysOfType<T, Match> = {
+  [K in keyof T]: T[K] extends Match ? K : never
+}[keyof T]
+
+export class ConfirmDialog {
+  public readonly root: Locator
+  public readonly delete: Locator
+  public readonly overwrite: Locator
+  public readonly reject: Locator
+  public readonly confirm: Locator
+  public readonly save: Locator
+
+  constructor(public readonly page: Page) {
+    this.root = page.getByRole('dialog')
+    this.delete = this.root.getByRole('button', { name: 'Delete' })
+    this.overwrite = this.root.getByRole('button', { name: 'Overwrite' })
+    this.reject = this.root.getByRole('button', { name: 'Cancel' })
+    this.confirm = this.root.getByRole('button', { name: 'Confirm' })
+    this.save = this.root.getByRole('button', { name: 'Save' })
+  }
+
+  async click(locator: KeysOfType<ConfirmDialog, Locator>) {
+    const loc = this[locator]
+    await loc.waitFor({ state: 'visible' })
+    await loc.click()
+
+    // Wait for this confirm dialog to close (not all dialogs — another
+    // dialog like save-as may open immediately after).
+    await this.root.waitFor({ state: 'hidden', timeout: 5000 }).catch(() => {})
+
+    // Wait for workflow service to finish if it's busy
+    await this.page.waitForFunction(
+      () =>
+        (window.app?.extensionManager as WorkspaceStore | undefined)?.workflow
+          ?.isBusy === false,
+      undefined,
+      { timeout: 3000 }
+    )
+  }
+}

browser_tests/fixtures/components/MediaLightbox.ts

@@ -0,0 +1,11 @@
+import type { Locator, Page } from '@playwright/test'
+
+export class MediaLightbox {
+  public readonly root: Locator
+  public readonly closeButton: Locator
+
+  constructor(public readonly page: Page) {
+    this.root = page.getByRole('dialog')
+    this.closeButton = this.root.getByLabel('Close')
+  }
+}

browser_tests/fixtures/components/TemplatesDialog.ts

@@ -0,0 +1,19 @@
+import type { Locator, Page } from '@playwright/test'
+
+export class TemplatesDialog {
+  public readonly root: Locator
+
+  constructor(public readonly page: Page) {
+    this.root = page.getByRole('dialog')
+  }
+
+  filterByHeading(name: string): Locator {
+    return this.root.filter({
+      has: this.page.getByRole('heading', { name, exact: true })
+    })
+  }
+
+  getCombobox(name: RegExp | string): Locator {
+    return this.root.getByRole('combobox', { name })
+  }
+}

browser_tests/fixtures/helpers/AssetsHelper.ts

@@ -5,6 +5,7 @@ import type { RawJobListItem } from '../../../src/platform/remote/comfyui/jobs/j
 
 const jobsListRoutePattern = /\/api\/jobs(?:\?.*)?$/
 const inputFilesRoutePattern = /\/internal\/files\/input(?:\?.*)?$/
+const historyRoutePattern = /\/api\/history$/
 
 /** Factory to create a mock completed job with preview output. */
 export function createMockJob(
@@ -89,6 +90,8 @@ export class AssetsHelper {
   private jobsRouteHandler: ((route: Route) => Promise<void>) | null = null
   private inputFilesRouteHandler: ((route: Route) => Promise<void>) | null =
     null
+  private deleteHistoryRouteHandler: ((route: Route) => Promise<void>) | null =
+    null
   private generatedJobs: RawJobListItem[] = []
   private importedFiles: string[] = []
 
@@ -185,6 +188,36 @@ export class AssetsHelper {
     await this.page.route(inputFilesRoutePattern, this.inputFilesRouteHandler)
   }
 
+  /**
+   * Mock the POST /api/history endpoint used for deleting history items.
+   * On receiving a `{ delete: [id] }` payload, removes matching jobs from
+   * the in-memory mock state so subsequent /api/jobs fetches reflect the
+   * deletion.
+   */
+  async mockDeleteHistory(): Promise<void> {
+    if (this.deleteHistoryRouteHandler) return
+
+    this.deleteHistoryRouteHandler = async (route: Route) => {
+      const request = route.request()
+      if (request.method() !== 'POST') {
+        await route.continue()
+        return
+      }
+
+      const body = request.postDataJSON() as { delete?: string[] }
+      if (body.delete) {
+        const idsToRemove = new Set(body.delete)
+        this.generatedJobs = this.generatedJobs.filter(
+          (job) => !idsToRemove.has(job.id)
+        )
+      }
+
+      await route.fulfill({ status: 200, body: '{}' })
+    }
+
+    await this.page.route(historyRoutePattern, this.deleteHistoryRouteHandler)
+  }
+
   async mockEmptyState(): Promise<void> {
     await this.mockOutputHistory([])
     await this.mockInputFiles([])
@@ -206,5 +239,13 @@ export class AssetsHelper {
       )
       this.inputFilesRouteHandler = null
     }
+
+    if (this.deleteHistoryRouteHandler) {
+      await this.page.unroute(
+        historyRoutePattern,
+        this.deleteHistoryRouteHandler
+      )
+      this.deleteHistoryRouteHandler = null
+    }
   }
 }

browser_tests/tests/confirmDialogTextWrap.spec.ts

@@ -18,15 +18,13 @@ test.describe('Confirm dialog text wrapping', { tag: ['@mobile'] }, () => {
         .catch(() => {})
     }, longFilename)
 
-    const dialog = comfyPage.page.getByRole('dialog')
-    await expect(dialog).toBeVisible()
+    const { root, confirm, reject } = comfyPage.confirmDialog
+    await expect(root).toBeVisible()
 
-    const confirmButton = dialog.getByRole('button', { name: 'Confirm' })
-    await expect(confirmButton).toBeVisible()
-    await expect(confirmButton).toBeInViewport()
+    await expect(confirm).toBeVisible()
+    await expect(confirm).toBeInViewport()
 
-    const cancelButton = dialog.getByRole('button', { name: 'Cancel' })
-    await expect(cancelButton).toBeVisible()
-    await expect(cancelButton).toBeInViewport()
+    await expect(reject).toBeVisible()
+    await expect(reject).toBeInViewport()
   })
 })

browser_tests/tests/resultGallery.spec.ts

@@ -41,30 +41,28 @@ test.describe('MediaLightbox', { tag: ['@slow'] }, () => {
     await assetCard.hover()
     await assetCard.getByLabel('Zoom in').click()
 
-    const gallery = comfyPage.page.getByRole('dialog')
-    await expect(gallery).toBeVisible()
-
-    return { gallery }
+    const { root } = comfyPage.mediaLightbox
+    await expect(root).toBeVisible()
   }
 
   test('opens gallery and shows dialog with close button', async ({
     comfyPage
   }) => {
-    const { gallery } = await runAndOpenGallery(comfyPage)
-    await expect(gallery.getByLabel('Close')).toBeVisible()
+    await runAndOpenGallery(comfyPage)
+    await expect(comfyPage.mediaLightbox.closeButton).toBeVisible()
   })
 
   test('closes gallery on Escape key', async ({ comfyPage }) => {
     await runAndOpenGallery(comfyPage)
 
     await comfyPage.page.keyboard.press('Escape')
-    await expect(comfyPage.page.getByRole('dialog')).not.toBeVisible()
+    await expect(comfyPage.mediaLightbox.root).not.toBeVisible()
   })
 
   test('closes gallery when clicking close button', async ({ comfyPage }) => {
-    const { gallery } = await runAndOpenGallery(comfyPage)
+    await runAndOpenGallery(comfyPage)
 
-    await gallery.getByLabel('Close').click()
-    await expect(comfyPage.page.getByRole('dialog')).not.toBeVisible()
+    await comfyPage.mediaLightbox.closeButton.click()
+    await expect(comfyPage.mediaLightbox.root).not.toBeVisible()
   })
 })

browser_tests/tests/sidebar/assets.spec.ts

@@ -676,3 +676,83 @@ test.describe('Assets sidebar - settings menu', () => {
     await expect(tab.gridViewOption).toBeVisible()
   })
 })
+
+// ==========================================================================
+// 11. Delete confirmation
+// ==========================================================================
+
+test.describe('Assets sidebar - delete confirmation', () => {
+  test.beforeEach(async ({ comfyPage }) => {
+    await comfyPage.assets.mockOutputHistory(SAMPLE_JOBS)
+    await comfyPage.assets.mockDeleteHistory()
+    await comfyPage.assets.mockInputFiles([])
+    await comfyPage.setup()
+  })
+
+  test.afterEach(async ({ comfyPage }) => {
+    await comfyPage.assets.clearMocks()
+  })
+
+  test('Right-click delete shows confirmation dialog', async ({
+    comfyPage
+  }) => {
+    const tab = comfyPage.menu.assetsTab
+    await tab.open()
+    await tab.waitForAssets()
+
+    await tab.assetCards.first().click({ button: 'right' })
+    await tab.contextMenuItem('Delete').click()
+
+    const dialog = comfyPage.confirmDialog.root
+    await expect(dialog).toBeVisible()
+    await expect(dialog.getByText('Delete this asset?')).toBeVisible()
+    await expect(
+      dialog.getByText('This asset will be permanently removed.')
+    ).toBeVisible()
+  })
+
+  test('Confirming delete removes asset and shows success toast', async ({
+    comfyPage
+  }) => {
+    const tab = comfyPage.menu.assetsTab
+    await tab.open()
+    await tab.waitForAssets()
+
+    const initialCount = await tab.assetCards.count()
+
+    await tab.assetCards.first().click({ button: 'right' })
+    await tab.contextMenuItem('Delete').click()
+
+    const dialog = comfyPage.confirmDialog.root
+    await expect(dialog).toBeVisible()
+
+    await comfyPage.confirmDialog.delete.click()
+
+    await expect(dialog).not.toBeVisible()
+    await expect(tab.assetCards).toHaveCount(initialCount - 1, {
+      timeout: 5000
+    })
+
+    const successToast = comfyPage.page.locator('.p-toast-message-success')
+    await expect(successToast).toBeVisible({ timeout: 5000 })
+  })
+
+  test('Cancelling delete preserves asset', async ({ comfyPage }) => {
+    const tab = comfyPage.menu.assetsTab
+    await tab.open()
+    await tab.waitForAssets()
+
+    const initialCount = await tab.assetCards.count()
+
+    await tab.assetCards.first().click({ button: 'right' })
+    await tab.contextMenuItem('Delete').click()
+
+    const dialog = comfyPage.confirmDialog.root
+    await expect(dialog).toBeVisible()
+
+    await comfyPage.confirmDialog.reject.click()
+
+    await expect(dialog).not.toBeVisible()
+    await expect(tab.assetCards).toHaveCount(initialCount)
+  })
+})

browser_tests/tests/templates.spec.ts

@@ -116,9 +116,7 @@ test.describe('Templates', { tag: ['@slow', '@workflow'] }, () => {
 
     await comfyPage.command.executeCommand('Comfy.BrowseTemplates')
 
-    const dialog = comfyPage.page.getByRole('dialog').filter({
-      has: comfyPage.page.getByRole('heading', { name: 'Modèles', exact: true })
-    })
+    const dialog = comfyPage.templatesDialog.filterByHeading('Modèles')
     await expect(dialog).toBeVisible()
 
     // Validate that French-localized strings from the templates index are rendered
@@ -220,8 +218,7 @@ test.describe('Templates', { tag: ['@slow', '@workflow'] }, () => {
       await expect(comfyPage.templates.content).toBeVisible()
 
       // Wait for filter bar select components to render
-      const dialog = comfyPage.page.getByRole('dialog')
-      const sortBySelect = dialog.getByRole('combobox', { name: /Sort/ })
+      const sortBySelect = comfyPage.templatesDialog.getCombobox(/Sort/)
       await expect(sortBySelect).toBeVisible()
 
       // Screenshot the filter bar containing MultiSelect and SingleSelect

browser_tests/tests/workflowPersistence.spec.ts

@@ -460,11 +460,8 @@ test.describe('Workflow Persistence', () => {
       .getWorkflowTab('Unsaved Workflow')
       .click({ button: 'middle' })
 
-    // Click "Save" in the dirty close dialog (scoped to dialog)
-    const dialog = comfyPage.page.getByRole('dialog')
-    const saveButton = dialog.getByRole('button', { name: 'Save' })
-    await saveButton.waitFor({ state: 'visible' })
-    await saveButton.click()
+    // Click "Save" in the dirty close dialog
+    await comfyPage.confirmDialog.click('save')
 
     // Fill in the filename dialog
     const saveDialog = comfyPage.menu.topbar.getSaveDialog()

src/composables/node/useNodeImageUpload.ts

@@ -9,6 +9,7 @@ import { api } from '@/scripts/api'
 import { useAssetsStore } from '@/stores/assetsStore'
 
 const PASTED_IMAGE_EXPIRY_MS = 2000
+const UPLOAD_TIMEOUT_MS = 120_000
 
 interface ImageUploadFormFields {
   /**
@@ -30,7 +31,8 @@ const uploadFile = async (
 
   const resp = await api.fetchApi('/upload/image', {
     method: 'POST',
-    body
+    body,
+    signal: AbortSignal.timeout(UPLOAD_TIMEOUT_MS)
   })
 
   if (resp.status !== 200) {
@@ -88,7 +90,11 @@ export const useNodeImageUpload = (
       if (!path) return
       return path
     } catch (error) {
-      useToastStore().addAlert(String(error))
+      if (error instanceof DOMException && error.name === 'TimeoutError') {
+        useToastStore().addAlert(t('g.uploadTimedOut'))
+      } else {
+        useToastStore().addAlert(String(error))
+      }
     }
   }
 

src/locales/en/main.json

@@ -199,6 +199,7 @@
     "control_before_generate": "control before generate",
     "choose_file_to_upload": "choose file to upload",
     "uploadAlreadyInProgress": "Upload already in progress",
+    "uploadTimedOut": "Upload timed out. Please try again.",
     "capture": "capture",
     "nodes": "Nodes",
     "nodesCount": "{count} node | {count} nodes",