From 448ad73fae30838de9b6b94e3ed27d73c42a24ba Mon Sep 17 00:00:00 2001 From: Dante Date: Tue, 19 May 2026 09:05:40 +0900 Subject: [PATCH 1/4] refactor(assets): collapse useMediaAssets factory wrapper (FE-727) (#12283) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary https://linear.app/comfyorg/issue/FE-727/refactor-usemediaasset `useMediaAssets` was a factory wrapper that branched on `isCloud` and returned one of two near-identical implementations (`useAssetsApi` / `useInternalFilesApi`). Both implementations delegated to the same `assetsStore` actions (`updateInputs` / `updateHistory` / `loadMoreHistory`) — the real cloud/local fork lives inside `assetsStore.fetchInputFiles` (line 121), not at the composable layer. Collapse the wrapper: 1. Delete `useInternalFilesApi` (identical to `useAssetsApi`). 2. Delete `useMediaAssets` (now a pass-through). 3. Switch the four callers to `useAssetsApi` directly. ## Changes - **What**: - Delete `useInternalFilesApi.ts` and `useMediaAssets.ts`. - `AssetsSidebarTab.vue`, `WidgetSelectDropdown.vue`, `useOutputHistory.ts`: call `useAssetsApi` directly. - `useWidgetSelectItems.ts`: narrow `outputMediaAssets` prop type to `IAssetsProvider` (the interface `useOutputHistory` already implements directly). - Test mocks repointed from `useMediaAssets` → `useAssetsApi`. - Two unrelated tailwind class-order lint errors auto-fixed by `pnpm lint:fix` to keep CI green. - **Breaking**: None — no behavior change. ## Review Focus The real cloud/local fork remains at `assetsStore.ts:121` (`fetchInputFiles = isCloud ? fetchInputFilesFromCloud : fetchInputFilesFromAPI`). That branch is M1-strict and clears once BE-933 lands. This PR only collapses the dead wrapper layer above it. `IAssetsProvider` is intentionally kept — `useOutputHistory.ts:83` directly implements it for a different use case, so the interface still has more than one consumer. Surfaced while working on the FE-678 cloud/local asset branching survey. ## Screenshots (if applicable) N/A — no UI change. --------- Co-authored-by: GitHub Action Co-authored-by: Alexander Brown --- .../sidebar/tabs/AssetsSidebarTab.vue | 6 +- .../composables/media/useInternalFilesApi.ts | 63 ------------------- .../composables/media/useMediaAssets.ts | 15 ----- .../linearMode/useOutputHistory.test.ts | 4 +- .../extensions/linearMode/useOutputHistory.ts | 4 +- .../components/WidgetSelectDropdown.test.ts | 4 +- .../components/WidgetSelectDropdown.vue | 4 +- .../composables/useWidgetSelectItems.test.ts | 4 +- .../composables/useWidgetSelectItems.ts | 4 +- 9 files changed, 15 insertions(+), 93 deletions(-) delete mode 100644 src/platform/assets/composables/media/useInternalFilesApi.ts delete mode 100644 src/platform/assets/composables/media/useMediaAssets.ts diff --git a/src/components/sidebar/tabs/AssetsSidebarTab.vue b/src/components/sidebar/tabs/AssetsSidebarTab.vue index ce661eef00..20d4814c97 100644 --- a/src/components/sidebar/tabs/AssetsSidebarTab.vue +++ b/src/components/sidebar/tabs/AssetsSidebarTab.vue @@ -237,7 +237,7 @@ import Button from '@/components/ui/button/Button.vue' import MediaAssetContextMenu from '@/platform/assets/components/MediaAssetContextMenu.vue' import MediaAssetFilterBar from '@/platform/assets/components/MediaAssetFilterBar.vue' import { getAssetType } from '@/platform/assets/composables/media/assetMappers' -import { useMediaAssets } from '@/platform/assets/composables/media/useMediaAssets' +import { useAssetsApi } from '@/platform/assets/composables/media/useAssetsApi' import { useAssetSelection } from '@/platform/assets/composables/useAssetSelection' import { useMediaAssetActions } from '@/platform/assets/composables/useMediaAssetActions' import { useMediaAssetFiltering } from '@/platform/assets/composables/useMediaAssetFiltering' @@ -309,8 +309,8 @@ const formattedExecutionTime = computed(() => { const toast = useToast() -const inputAssets = useMediaAssets('input') -const outputAssets = useMediaAssets('output') +const inputAssets = useAssetsApi('input') +const outputAssets = useAssetsApi('output') // Asset selection const { diff --git a/src/platform/assets/composables/media/useInternalFilesApi.ts b/src/platform/assets/composables/media/useInternalFilesApi.ts deleted file mode 100644 index acba1ee8de..0000000000 --- a/src/platform/assets/composables/media/useInternalFilesApi.ts +++ /dev/null @@ -1,63 +0,0 @@ -import { computed } from 'vue' - -import type { AssetItem } from '@/platform/assets/schemas/assetSchema' -import { useAssetsStore } from '@/stores/assetsStore' - -/** - * Composable for fetching media assets from local environment - * Uses AssetsStore for centralized state management - */ -export function useInternalFilesApi(directory: 'input' | 'output') { - const assetsStore = useAssetsStore() - - const media = computed(() => - directory === 'input' ? assetsStore.inputAssets : assetsStore.historyAssets - ) - - const loading = computed(() => - directory === 'input' - ? assetsStore.inputLoading - : assetsStore.historyLoading - ) - - const error = computed(() => - directory === 'input' ? assetsStore.inputError : assetsStore.historyError - ) - - const fetchMediaList = async (): Promise => { - if (directory === 'input') { - await assetsStore.updateInputs() - return assetsStore.inputAssets - } else { - await assetsStore.updateHistory() - return assetsStore.historyAssets - } - } - - const refresh = () => fetchMediaList() - - const loadMore = async (): Promise => { - if (directory === 'output') { - await assetsStore.loadMoreHistory() - } - } - - const hasMore = computed(() => { - return directory === 'output' ? assetsStore.hasMoreHistory : false - }) - - const isLoadingMore = computed(() => { - return directory === 'output' ? assetsStore.isLoadingMore : false - }) - - return { - media, - loading, - error, - fetchMediaList, - refresh, - loadMore, - hasMore, - isLoadingMore - } -} diff --git a/src/platform/assets/composables/media/useMediaAssets.ts b/src/platform/assets/composables/media/useMediaAssets.ts deleted file mode 100644 index 46bb6111be..0000000000 --- a/src/platform/assets/composables/media/useMediaAssets.ts +++ /dev/null @@ -1,15 +0,0 @@ -import { isCloud } from '@/platform/distribution/types' - -import type { IAssetsProvider } from './IAssetsProvider' -import { useAssetsApi } from './useAssetsApi' -import { useInternalFilesApi } from './useInternalFilesApi' - -/** - * Factory function that returns the appropriate media assets implementation - * based on the current distribution (cloud vs internal) - * @param directory - The directory to fetch assets from - * @returns IAssetsProvider implementation - */ -export function useMediaAssets(directory: 'input' | 'output'): IAssetsProvider { - return isCloud ? useAssetsApi(directory) : useInternalFilesApi(directory) -} diff --git a/src/renderer/extensions/linearMode/useOutputHistory.test.ts b/src/renderer/extensions/linearMode/useOutputHistory.test.ts index 15826965f0..85371b156a 100644 --- a/src/renderer/extensions/linearMode/useOutputHistory.test.ts +++ b/src/renderer/extensions/linearMode/useOutputHistory.test.ts @@ -23,8 +23,8 @@ const selectAsLatestFn = vi.fn() const resolveIfReadyFn = vi.fn() const resolvedOutputsCacheRef = new Map() -vi.mock('@/platform/assets/composables/media/useMediaAssets', () => ({ - useMediaAssets: () => ({ +vi.mock('@/platform/assets/composables/media/useAssetsApi', () => ({ + useAssetsApi: () => ({ media: mediaRef, loading: ref(false), error: ref(null), diff --git a/src/renderer/extensions/linearMode/useOutputHistory.ts b/src/renderer/extensions/linearMode/useOutputHistory.ts index c723c977f7..317c99de5e 100644 --- a/src/renderer/extensions/linearMode/useOutputHistory.ts +++ b/src/renderer/extensions/linearMode/useOutputHistory.ts @@ -3,7 +3,7 @@ import type { ComputedRef } from 'vue' import { computed, ref, watchEffect } from 'vue' import type { IAssetsProvider } from '@/platform/assets/composables/media/IAssetsProvider' -import { useMediaAssets } from '@/platform/assets/composables/media/useMediaAssets' +import { useAssetsApi } from '@/platform/assets/composables/media/useAssetsApi' import { getOutputAssetMetadata } from '@/platform/assets/schemas/assetMetadataSchema' import type { AssetItem } from '@/platform/assets/schemas/assetSchema' import { useWorkflowStore } from '@/platform/workflow/management/stores/workflowStore' @@ -25,7 +25,7 @@ export function useOutputHistory(): { isWorkflowActive: ComputedRef cancelActiveWorkflowJobs: () => Promise } { - const backingOutputs = useMediaAssets('output') + const backingOutputs = useAssetsApi('output') void backingOutputs.fetchMediaList() const linearStore = useLinearOutputStore() const workflowStore = useWorkflowStore() diff --git a/src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDropdown.test.ts b/src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDropdown.test.ts index deab218513..fc485a4091 100644 --- a/src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDropdown.test.ts +++ b/src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDropdown.test.ts @@ -69,8 +69,8 @@ const { mockMediaAssets } = vi.hoisted(() => { } }) -vi.mock('@/platform/assets/composables/media/useMediaAssets', () => ({ - useMediaAssets: () => mockMediaAssets +vi.mock('@/platform/assets/composables/media/useAssetsApi', () => ({ + useAssetsApi: () => mockMediaAssets })) vi.mock('@/platform/assets/utils/outputAssetUtil', () => ({ diff --git a/src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDropdown.vue b/src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDropdown.vue index 970124615e..541ac04a3b 100644 --- a/src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDropdown.vue +++ b/src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDropdown.vue @@ -4,7 +4,7 @@ import { useI18n } from 'vue-i18n' import { useTransformCompatOverlayProps } from '@/composables/useTransformCompatOverlayProps' import { SUPPORTED_EXTENSIONS_ACCEPT } from '@/extensions/core/load3d/constants' -import { useMediaAssets } from '@/platform/assets/composables/media/useMediaAssets' +import { useAssetsApi } from '@/platform/assets/composables/media/useAssetsApi' import FormDropdown from '@/renderer/extensions/vueNodes/widgets/components/form/dropdown/FormDropdown.vue' import { AssetKindKey } from '@/renderer/extensions/vueNodes/widgets/components/form/dropdown/types' import type { LayoutMode } from '@/renderer/extensions/vueNodes/widgets/components/form/dropdown/types' @@ -47,7 +47,7 @@ const modelValue = defineModel({ const { t } = useI18n() -const outputMediaAssets = useMediaAssets('output') +const outputMediaAssets = useAssetsApi('output') const transformCompatProps = useTransformCompatOverlayProps() diff --git a/src/renderer/extensions/vueNodes/widgets/composables/useWidgetSelectItems.test.ts b/src/renderer/extensions/vueNodes/widgets/composables/useWidgetSelectItems.test.ts index 5b6bff0145..4c688642dd 100644 --- a/src/renderer/extensions/vueNodes/widgets/composables/useWidgetSelectItems.test.ts +++ b/src/renderer/extensions/vueNodes/widgets/composables/useWidgetSelectItems.test.ts @@ -37,8 +37,8 @@ function createMockMediaAssets() { let mockMediaAssets = createMockMediaAssets() -vi.mock('@/platform/assets/composables/media/useMediaAssets', () => ({ - useMediaAssets: () => mockMediaAssets +vi.mock('@/platform/assets/composables/media/useAssetsApi', () => ({ + useAssetsApi: () => mockMediaAssets })) vi.mock('@/platform/assets/composables/useAssetFilterOptions', () => ({ diff --git a/src/renderer/extensions/vueNodes/widgets/composables/useWidgetSelectItems.ts b/src/renderer/extensions/vueNodes/widgets/composables/useWidgetSelectItems.ts index 8450bb0f46..12c1584d79 100644 --- a/src/renderer/extensions/vueNodes/widgets/composables/useWidgetSelectItems.ts +++ b/src/renderer/extensions/vueNodes/widgets/composables/useWidgetSelectItems.ts @@ -25,7 +25,7 @@ import type { useAssetWidgetData } from '@/renderer/extensions/vueNodes/widgets/ import { getOutputAssetMetadata } from '@/platform/assets/schemas/assetMetadataSchema' import type { AssetItem } from '@/platform/assets/schemas/assetSchema' import { resolveOutputAssetItems } from '@/platform/assets/utils/outputAssetUtil' -import type { useMediaAssets } from '@/platform/assets/composables/media/useMediaAssets' +import type { IAssetsProvider } from '@/platform/assets/composables/media/IAssetsProvider' import type { AssetKind } from '@/types/widgetTypes' import { getMediaTypeFromFilename } from '@/utils/formatUtil' @@ -65,7 +65,7 @@ interface UseWidgetSelectItemsOptions { > modelValue: Ref assetKind: MaybeRefOrGetter - outputMediaAssets: ReturnType + outputMediaAssets: IAssetsProvider assetData: ReturnType | null isAssetMode: MaybeRefOrGetter } From a97f46b4975b87d5e9d967fea8eefa993356858a Mon Sep 17 00:00:00 2001 From: Benjamin Lu Date: Mon, 18 May 2026 18:34:56 -0700 Subject: [PATCH 2/4] test: cover job history sidebar with typed route mocks (#12272) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Add the first product-area browser coverage on top of the merged typed route mock foundation: the docked job history sidebar. ## Changes - **What**: Adds `browser_tests/tests/sidebar/jobHistory.spec.ts` using `jobsRouteFixture`. - **What**: Covers direct sidebar entry, docked QPO history entry, terminal history jobs, active queue jobs, tab filtering, search, clear queue, and clear history. - **What**: Adds typed `POST /api/queue` and `POST /api/history` route helpers that validate request bodies with generated zod schemas. - **What**: Adds stable test ids for the job history sidebar and queue progress overlay so tests avoid structural CSS selectors. - **Dependencies**: Builds on the typed route mock foundation merged in #12267. ## Review Focus Review the product assertions and whether this is the right first coverage slice on top of the typed route mock foundation. This PR intentionally avoids asset sidebar and floating QPO lifecycle coverage; those should remain follow-up PRs. ## Screenshots (if applicable) Not applicable. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12272-test-cover-job-history-sidebar-with-typed-route-mocks-3606d73d3650817481d5f9fac4bfc93c) by [Unito](https://www.unito.io) --- .../fixtures/components/QueuePanel.ts | 4 +- browser_tests/fixtures/jobsRouteFixture.ts | 64 +++- browser_tests/fixtures/selectors.ts | 3 + .../tests/sidebar/jobHistory.spec.ts | 280 ++++++++++++++++++ src/components/queue/QueueProgressOverlay.vue | 1 + .../sidebar/tabs/JobHistorySidebarTab.vue | 5 +- 6 files changed, 345 insertions(+), 12 deletions(-) create mode 100644 browser_tests/tests/sidebar/jobHistory.spec.ts diff --git a/browser_tests/fixtures/components/QueuePanel.ts b/browser_tests/fixtures/components/QueuePanel.ts index 525af60ee0..819a68fa0a 100644 --- a/browser_tests/fixtures/components/QueuePanel.ts +++ b/browser_tests/fixtures/components/QueuePanel.ts @@ -5,11 +5,13 @@ import { TestIds } from '@e2e/fixtures/selectors' export class QueuePanel { readonly overlayToggle: Locator + readonly overlay: Locator readonly moreOptionsButton: Locator constructor(readonly page: Page) { this.overlayToggle = page.getByTestId(TestIds.queue.overlayToggle) - this.moreOptionsButton = page.getByLabel(/More options/i).first() + this.overlay = page.getByTestId(TestIds.queue.progressOverlay) + this.moreOptionsButton = this.overlay.getByLabel(/More options/i) } async openClearHistoryDialog() { diff --git a/browser_tests/fixtures/jobsRouteFixture.ts b/browser_tests/fixtures/jobsRouteFixture.ts index 5128a2f397..11e9b4e4e7 100644 --- a/browser_tests/fixtures/jobsRouteFixture.ts +++ b/browser_tests/fixtures/jobsRouteFixture.ts @@ -1,6 +1,11 @@ import { test as base } from '@playwright/test' import type { Page } from '@playwright/test' import type { z } from 'zod' +import { + zHistoryManageRequest, + zQueueManageRequest, + zQueueManageResponse +} from '@comfyorg/ingest-types/zod' import type { JobStatus, @@ -9,6 +14,8 @@ import type { } from '@/platform/remote/comfyui/jobs/jobTypes' type JobsListResponse = z.infer +type HistoryManageRequest = z.infer +type QueueManageRequest = z.infer const terminalJobStatuses = [ 'completed', @@ -22,7 +29,8 @@ const activeJobStatuses = [ const defaultJobsListLimit = 200 const defaultScenarioHistoryLimit = 64 const defaultJobsListOffset = 0 -const defaultRouteMockJobTimestamp = Date.UTC(2026, 0, 1, 12) + +export const routeMockJobTimestamp = Date.UTC(2026, 0, 1, 12) interface JobsListRoute { statuses: readonly JobStatus[] @@ -65,11 +73,9 @@ function hasJobsListPageParams( ) } -function isJobsListRequest(url: URL, route: JobsListRoute): boolean { +function matchesJobsListRoute(url: URL, route: JobsListRoute): boolean { return ( - url.pathname.endsWith('/api/jobs') && - hasExactStatuses(url, route.statuses) && - hasJobsListPageParams(url, route) + hasExactStatuses(url, route.statuses) && hasJobsListPageParams(url, route) ) } @@ -99,9 +105,9 @@ export function createRouteMockJob({ return { id, status: 'completed', - create_time: defaultRouteMockJobTimestamp, - execution_start_time: defaultRouteMockJobTimestamp, - execution_end_time: defaultRouteMockJobTimestamp + 5_000, + create_time: routeMockJobTimestamp, + execution_start_time: routeMockJobTimestamp, + execution_end_time: routeMockJobTimestamp + 5_000, preview_output: { filename: `output_${id}.png`, subfolder: '', @@ -150,7 +156,8 @@ export class JobsRouteMocker { const response = createJobsListResponse(route) await this.page.route( - (url) => isJobsListRequest(url, route), + (url) => + url.pathname.endsWith('/api/jobs') && matchesJobsListRoute(url, route), async (requestRoute) => { if (requestRoute.request().method().toUpperCase() !== 'GET') { await requestRoute.fallback() @@ -161,6 +168,44 @@ export class JobsRouteMocker { } ) } + + async mockClearQueue(): Promise { + const response = zQueueManageResponse.parse({ cleared: true }) + return await this.mockPostManageRoute( + 'queue', + zQueueManageRequest, + response + ) + } + + async mockClearHistory(): Promise { + return await this.mockPostManageRoute('history', zHistoryManageRequest, {}) + } + + private async mockPostManageRoute( + type: 'queue' | 'history', + requestSchema: z.ZodType, + response: unknown + ): Promise { + const requests: TRequest[] = [] + + await this.page.route( + (url) => url.pathname.endsWith(`/api/${type}`), + async (requestRoute) => { + if (requestRoute.request().method().toUpperCase() !== 'POST') { + await requestRoute.fallback() + return + } + + requests.push( + requestSchema.parse(requestRoute.request().postDataJSON()) + ) + await requestRoute.fulfill({ json: response }) + } + ) + + return requests + } } export const jobsRouteFixture = base.extend<{ @@ -168,6 +213,5 @@ export const jobsRouteFixture = base.extend<{ }>({ jobsRoutes: async ({ page }, use) => { await use(new JobsRouteMocker(page)) - await page.unrouteAll({ behavior: 'wait' }) } }) diff --git a/browser_tests/fixtures/selectors.ts b/browser_tests/fixtures/selectors.ts index 2745eff0b0..ef1f9e09d8 100644 --- a/browser_tests/fixtures/selectors.ts +++ b/browser_tests/fixtures/selectors.ts @@ -226,7 +226,10 @@ export const TestIds = { currentUserIndicator: 'current-user-indicator' }, queue: { + jobHistorySidebar: 'job-history-sidebar', + progressOverlay: 'queue-progress-overlay', overlayToggle: 'queue-overlay-toggle', + dockedJobHistoryAction: 'docked-job-history-action', jobDetailsPopover: 'queue-job-details-popover', clearHistoryAction: 'clear-history-action', jobAssetsList: 'job-assets-list', diff --git a/browser_tests/tests/sidebar/jobHistory.spec.ts b/browser_tests/tests/sidebar/jobHistory.spec.ts new file mode 100644 index 0000000000..40154f9490 --- /dev/null +++ b/browser_tests/tests/sidebar/jobHistory.spec.ts @@ -0,0 +1,280 @@ +import { expect, mergeTests } from '@playwright/test' + +import { comfyPageFixture } from '@e2e/fixtures/ComfyPage' +import type { ComfyPage } from '@e2e/fixtures/ComfyPage' +import { + createRouteMockJob, + jobsRouteFixture, + routeMockJobTimestamp +} from '@e2e/fixtures/jobsRouteFixture' +import type { JobsRouteMocker } from '@e2e/fixtures/jobsRouteFixture' +import { TestIds } from '@e2e/fixtures/selectors' +import type { RawJobListItem } from '@/platform/remote/comfyui/jobs/jobTypes' + +const test = mergeTests(comfyPageFixture, jobsRouteFixture) + +const historyJobs: RawJobListItem[] = [ + createRouteMockJob({ + id: 'history-completed', + status: 'completed', + create_time: routeMockJobTimestamp - 60_000, + execution_start_time: routeMockJobTimestamp - 60_000, + execution_end_time: routeMockJobTimestamp - 55_000, + preview_output: { + filename: 'completed-output.png', + subfolder: '', + type: 'output', + nodeId: '1', + mediaType: 'images' + } + }), + createRouteMockJob({ + id: 'history-failed', + status: 'failed', + create_time: routeMockJobTimestamp - 120_000, + execution_start_time: routeMockJobTimestamp - 120_000, + execution_end_time: routeMockJobTimestamp - 118_000, + outputs_count: 0, + execution_error: { + node_id: '1', + node_type: 'SaveImage', + exception_message: 'Intentional fixture failure', + exception_type: 'Error', + traceback: [], + current_inputs: {}, + current_outputs: {} + } + }), + createRouteMockJob({ + id: 'history-cancelled', + status: 'cancelled', + create_time: routeMockJobTimestamp - 180_000, + execution_start_time: routeMockJobTimestamp - 180_000, + execution_end_time: routeMockJobTimestamp - 179_000, + outputs_count: 0 + }) +] + +const activeJobs: RawJobListItem[] = [ + createRouteMockJob({ + id: 'queue-running', + status: 'in_progress', + create_time: routeMockJobTimestamp - 10_000, + execution_start_time: routeMockJobTimestamp - 9_000, + execution_end_time: null, + outputs_count: 0 + }), + createRouteMockJob({ + id: 'queue-pending', + status: 'pending', + create_time: routeMockJobTimestamp - 5_000, + execution_start_time: null, + execution_end_time: null, + outputs_count: 0 + }) +] +const runningOnlyJobs = activeJobs.filter((job) => job.status !== 'pending') + +async function setupJobHistorySidebar( + comfyPage: ComfyPage, + jobsRoutes: JobsRouteMocker, + { + history = historyJobs, + queue = activeJobs + }: { + history?: readonly RawJobListItem[] + queue?: readonly RawJobListItem[] + } = {} +) { + await jobsRoutes.mockJobsScenario({ history, queue }) + await comfyPage.settings.setSetting('Comfy.Queue.QPOV2', true) + await comfyPage.setup() + + await comfyPage.page + .getByTestId(TestIds.sidebar.toolbar) + .getByRole('button', { name: 'Job History', exact: true }) + .click() + await expect(jobHistorySidebar(comfyPage)).toBeVisible() +} + +function jobRow(comfyPage: ComfyPage) { + const list = comfyPage.page.getByTestId(TestIds.queue.jobAssetsList) + + return (jobId: string) => list.locator(`[data-job-id="${jobId}"]`) +} + +function jobHistorySidebar(comfyPage: ComfyPage) { + return comfyPage.page.getByTestId(TestIds.queue.jobHistorySidebar) +} + +function clearQueueButton(comfyPage: ComfyPage) { + return jobHistorySidebar(comfyPage).getByRole('button', { + name: 'Clear queue', + exact: true + }) +} + +async function openSidebarClearHistoryDialog(comfyPage: ComfyPage) { + await jobHistorySidebar(comfyPage) + .getByLabel(/More options/i) + .click() + await comfyPage.page.getByTestId(TestIds.queue.clearHistoryAction).click() +} + +test.describe('Job history sidebar', { tag: '@ui' }, () => { + test('opens from the queue overlay docked history action', async ({ + comfyPage, + jobsRoutes + }) => { + await jobsRoutes.mockJobsScenario({ + history: historyJobs, + queue: activeJobs + }) + await comfyPage.settings.setSetting('Comfy.Queue.QPOV2', false) + await comfyPage.setup() + + await comfyPage.page.getByTestId(TestIds.queue.overlayToggle).click() + await comfyPage.queuePanel.moreOptionsButton.click() + await comfyPage.page + .getByTestId(TestIds.queue.dockedJobHistoryAction) + .click() + + await expect(jobHistorySidebar(comfyPage)).toBeVisible() + await expect(jobRow(comfyPage)('history-completed')).toBeVisible() + await expect(jobRow(comfyPage)('queue-pending')).toBeVisible() + }) + + test('shows terminal and active job states', async ({ + comfyPage, + jobsRoutes + }) => { + await setupJobHistorySidebar(comfyPage, jobsRoutes) + + const row = jobRow(comfyPage) + await expect(row('queue-pending')).toBeVisible() + await expect(row('queue-running')).toBeVisible() + await expect(row('history-completed')).toBeVisible() + await expect(row('history-failed')).toBeVisible() + await expect(row('history-cancelled')).toBeVisible() + + await expect(clearQueueButton(comfyPage)).toBeEnabled() + }) + + test('filters completed and failed history jobs', async ({ + comfyPage, + jobsRoutes + }) => { + await setupJobHistorySidebar(comfyPage, jobsRoutes) + + await comfyPage.page + .getByRole('button', { name: 'Completed', exact: true }) + .click() + + const row = jobRow(comfyPage) + await expect(row('history-completed')).toBeVisible() + await expect(row('history-failed')).toBeHidden() + await expect(row('queue-running')).toBeHidden() + + await comfyPage.page + .getByRole('button', { name: 'Failed', exact: true }) + .click() + + await expect(row('history-failed')).toBeVisible() + await expect(row('history-cancelled')).toBeVisible() + await expect(row('history-completed')).toBeHidden() + }) + + test('searches by job id and output filename', async ({ + comfyPage, + jobsRoutes + }) => { + await setupJobHistorySidebar(comfyPage, jobsRoutes) + + const row = jobRow(comfyPage) + const searchInput = comfyPage.page.getByPlaceholder('Search...') + + await searchInput.fill('history-failed') + await expect(row('history-failed')).toBeVisible() + await expect(row('history-completed')).toBeHidden() + await expect(row('queue-running')).toBeHidden() + + await searchInput.fill('completed-output') + await expect(row('history-completed')).toBeVisible() + await expect(row('history-failed')).toBeHidden() + + await searchInput.clear() + await expect(row('history-completed')).toBeVisible() + await expect(row('queue-running')).toBeVisible() + }) + + test('disables clear queue when there are no pending jobs', async ({ + comfyPage, + jobsRoutes + }) => { + await setupJobHistorySidebar(comfyPage, jobsRoutes, { + queue: runningOnlyJobs + }) + + await expect(clearQueueButton(comfyPage)).toBeDisabled() + }) + + test('clears pending queue jobs and leaves running/history jobs', async ({ + comfyPage, + jobsRoutes + }) => { + await setupJobHistorySidebar(comfyPage, jobsRoutes) + + const row = jobRow(comfyPage) + await expect(row('queue-pending')).toBeVisible() + + const clearQueueRequests = await jobsRoutes.mockClearQueue() + const clearHistoryRequests = await jobsRoutes.mockClearHistory() + await jobsRoutes.mockJobsScenario({ + history: historyJobs, + queue: runningOnlyJobs + }) + + await clearQueueButton(comfyPage).click() + + await expect.poll(() => clearQueueRequests.length).toBe(1) + expect(clearQueueRequests).toContainEqual({ clear: true }) + await expect(row('queue-pending')).toBeHidden() + await expect(row('queue-running')).toBeVisible() + await expect(row('history-completed')).toBeVisible() + await expect(clearQueueButton(comfyPage)).toBeDisabled() + expect(clearHistoryRequests).toHaveLength(0) + }) + + test('clears history from the sidebar menu and keeps active jobs', async ({ + comfyPage, + jobsRoutes + }) => { + await setupJobHistorySidebar(comfyPage, jobsRoutes) + + const row = jobRow(comfyPage) + await expect(row('history-completed')).toBeVisible() + + const clearHistoryRequests = await jobsRoutes.mockClearHistory() + const clearQueueRequests = await jobsRoutes.mockClearQueue() + await jobsRoutes.mockJobsScenario({ + history: [], + queue: activeJobs + }) + + await openSidebarClearHistoryDialog(comfyPage) + await expect( + comfyPage.page.getByText('Clear your job queue history?') + ).toBeVisible() + await comfyPage.page + .getByRole('button', { name: 'Clear', exact: true }) + .click() + + await expect.poll(() => clearHistoryRequests.length).toBe(1) + expect(clearHistoryRequests).toContainEqual({ clear: true }) + await expect(row('history-completed')).toBeHidden() + await expect(row('history-failed')).toBeHidden() + await expect(row('queue-running')).toBeVisible() + await expect(row('queue-pending')).toBeVisible() + expect(clearQueueRequests).toHaveLength(0) + }) +}) diff --git a/src/components/queue/QueueProgressOverlay.vue b/src/components/queue/QueueProgressOverlay.vue index 9ba1dea9f9..38cfae3146 100644 --- a/src/components/queue/QueueProgressOverlay.vue +++ b/src/components/queue/QueueProgressOverlay.vue @@ -4,6 +4,7 @@ :class="['flex', 'justify-end', 'w-full', 'pointer-events-none']" >
- + diff --git a/src/composables/bottomPanelTabs/useLogsTerminal.ts b/src/composables/bottomPanelTabs/useLogsTerminal.ts new file mode 100644 index 0000000000..25f413b25f --- /dev/null +++ b/src/composables/bottomPanelTabs/useLogsTerminal.ts @@ -0,0 +1,123 @@ +import { until, useEventListener } from '@vueuse/core' +import { storeToRefs } from 'pinia' +import type { Ref } from 'vue' +import { onMounted, onScopeDispose, ref } from 'vue' +import { useI18n } from 'vue-i18n' + +import type { LogEntry, LogsWsMessage } from '@/schemas/apiSchema' +import { api } from '@/scripts/api' +import { useExecutionStore } from '@/stores/executionStore' + +type TerminalLike = { + write: (data: string) => void + reset: () => void + scrollToBottom: () => void +} + +/** + * Drives the built-in logs terminal: initial load, live `logs` stream, and + * full resync when the backend WebSocket reconnects (e.g., after a reboot). + * + * Listeners are registered synchronously so we cannot miss a `reconnected` + * event during the mount-time fetch/subscribe awaits. In-flight fetches are + * tied to AbortControllers so that: + * - rapid double-reconnects don't interleave writes / double-subscribe + * - unmount mid-fetch never writes to a disposed terminal + */ +export function useLogsTerminal( + terminal: Readonly> +) { + const { t } = useI18n() + const errorMessage = ref('') + const loading = ref(true) + + let mountController: AbortController | undefined + let resyncController: AbortController | undefined + + const writeEntries = (entries: LogEntry[]) => { + terminal.value?.write(entries.map((e) => e.m).join('')) + } + + const resyncLogs = async () => { + // Cancel both the in-flight mount fetch and any prior resync so a late + // mount response can't write a stale snapshot on top of a freshly-reset + // terminal after we've already written the post-reconnect view. + mountController?.abort() + resyncController?.abort() + const controller = new AbortController() + resyncController = controller + const { signal } = controller + + try { + const logs = await api.getRawLogs() + if (signal.aborted || !terminal.value) return + terminal.value.reset() + writeEntries(logs.entries) + terminal.value.scrollToBottom() + // Backend lost the per-client log subscription across the restart; + // re-subscribe so new runtime logs stream over the fresh WebSocket. + await api.subscribeLogs(true) + if (signal.aborted) return + errorMessage.value = '' + loading.value = false + } catch (err) { + if (signal.aborted) return + console.error('Error resyncing logs after reconnect', err) + errorMessage.value = t('logsTerminal.resyncError') + } + } + + // Register listeners synchronously, before any awaits, so a reconnect + // fired during mount cannot be missed. useEventListener handles cleanup + // on scope dispose. + useEventListener(api, 'logs', (e: CustomEvent) => { + writeEntries(e.detail.entries) + }) + useEventListener(api, 'reconnected', () => { + void resyncLogs() + }) + + onMounted(async () => { + if (!terminal.value) await until(terminal).toBeTruthy() + + const controller = new AbortController() + mountController = controller + const { signal } = controller + + try { + const logs = await api.getRawLogs() + if (signal.aborted || !terminal.value) return + writeEntries(logs.entries) + } catch (err) { + if (signal.aborted) return + console.error('Error loading logs', err) + errorMessage.value = t('logsTerminal.loadError') + loading.value = false + return + } + + const { clientId } = storeToRefs(useExecutionStore()) + if (!clientId.value) await until(clientId).not.toBeNull() + if (signal.aborted) return + + try { + await api.subscribeLogs(true) + } catch (err) { + if (signal.aborted) return + console.error('Error subscribing to logs', err) + } + + if (!signal.aborted) loading.value = false + }) + + onScopeDispose(() => { + mountController?.abort() + resyncController?.abort() + if (!api.clientId) return + api.subscribeLogs(false).catch((err) => { + console.error('Error unsubscribing from logs', err) + }) + }) + + return { errorMessage, loading } +} diff --git a/src/locales/en/main.json b/src/locales/en/main.json index 6450f09142..3d4cc8dd96 100644 --- a/src/locales/en/main.json +++ b/src/locales/en/main.json @@ -1160,6 +1160,10 @@ "saveAsTemplate": "Save as template", "enterName": "Enter name" }, + "logsTerminal": { + "loadError": "Unable to load logs, please ensure you have updated your ComfyUI backend.", + "resyncError": "Unable to resync logs after the backend reconnected. Reopen the console to retry." + }, "workflowService": { "exportWorkflow": "Export Workflow", "enterFilename": "Enter the filename", From 4931b0c4b26145b78ce9c5e6f93e63174af0ba2d Mon Sep 17 00:00:00 2001 From: Christian Byrne Date: Mon, 18 May 2026 19:24:16 -0700 Subject: [PATCH 4/4] fix(website): add dark-background favicon for legibility in search results (#12285) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit *PR Created by the Glary-Bot Agent* --- ## Summary The comfy.org favicon was reported as illegible in Google search results. The current `logomark.svg` is a transparent yellow "C" — when Google (or any client) composites it onto a white surface (search results, light-theme tab strips), the yellow disappears into the background. Fix: ship a dedicated `/favicon.svg` that wraps the existing yellow logomark in a solid black square, and point `` at it. The in-page nav logo, `Organization.logo` Schema.org URL, and any other consumer of `logomark.svg` are left untouched, so transparent-composite contexts (knowledge panels, dark nav) continue to render cleanly. ## Changes - `apps/website/public/favicon.svg` *(new)* — 48×48 SVG: black square + scaled-down original logomark path. Existing path geometry is reused verbatim inside a `` so the C glyph is byte-identical to the source. - `apps/website/src/layouts/BaseLayout.astro` — `` → ``. One-line change. ## Why a new file (vs editing `logomark.svg` in place) `logomark.svg` is also used by `SiteNav.vue` (in-page header on the dark `--color-primary-comfy-ink` background) and by the JSON-LD `Organization.logo` URL. Both consumers want the transparent version. Editing it in place would draw an ugly black square in the site's own header. ## User report > "just google searched comfyui and logo isnt legible. We should update.." ## Verification **Built site** - `pnpm typecheck` (astro check): 0 errors, 0 warnings - `pnpm build` (astro build): 280 pages built, exit 0 - Built `dist/index.html` contains exactly one `` and zero references to the old icon path in `` - `oxlint` on changed `.astro` file: 0 warnings, 0 errors **Visual (Playwright on local astro dev server)** - New favicon renders correctly at 16/32/64 px — yellow C centered on black square, no clipping. - In-page nav logo unchanged (yellow C floats cleanly on the dark `--color-primary-comfy-ink` nav background, no black wrapper visible). - Mock of Google search-result row shows the new favicon is high-contrast inside Google's white circular wrapper; the old one is nearly invisible. ## Screenshots ### Google-style search result simulation (before / after) ![Google search result simulation — before and after](google-result-before-after) ### Favicon at native sizes + Google circular wrapper ![Favicon comparison at 16/32/64 px and inside a Google-style white circle wrapper](favicon-comparison-preview) ### In-page nav header (unchanged after the fix) ![Site nav header showing the yellow Comfy logomark still sits cleanly on the dark nav background](homepage-header-after) ## Notes for reviewers - The change deliberately uses pure black `#000` (matching the user's literal request "make the white background, black") rather than `--color-primary-comfy-ink` (`#211927`). Either would work; happy to switch if brand preference is the ink color. - Search-engine cached favicons can take days/weeks to refresh on Google's side after the new file is deployed. ## Screenshots ![google-result-before-after](https://pub-1fd11710d4c8405b948c9edc4287a3f2.r2.dev/sessions/3cb3469452b36ec7b11a5cd6b88e31056ad2dfadfb1b4c3b99db2b91c8229d89/pr-images/1778827200860-80e3877f-e1af-4cf3-962e-a1bf25ea9815.png) ![favicon-comparison-preview](https://pub-1fd11710d4c8405b948c9edc4287a3f2.r2.dev/sessions/3cb3469452b36ec7b11a5cd6b88e31056ad2dfadfb1b4c3b99db2b91c8229d89/pr-images/1778827201188-582fe0fb-aa7c-4fa1-af5b-fbc2a72387b7.png) ![homepage-header-after](https://pub-1fd11710d4c8405b948c9edc4287a3f2.r2.dev/sessions/3cb3469452b36ec7b11a5cd6b88e31056ad2dfadfb1b4c3b99db2b91c8229d89/pr-images/1778827201630-3a88502e-dd59-4e52-82a6-55f6b9768e4d.png) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12285-fix-website-add-dark-background-favicon-for-legibility-in-search-results-3616d73d365081babbcbedf0b86d3d67) by [Unito](https://www.unito.io) --------- Co-authored-by: Glary-Bot --- apps/website/public/favicon.svg | 14 ++++++++++++++ apps/website/src/layouts/BaseLayout.astro | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-) create mode 100644 apps/website/public/favicon.svg diff --git a/apps/website/public/favicon.svg b/apps/website/public/favicon.svg new file mode 100644 index 0000000000..f41c8445d7 --- /dev/null +++ b/apps/website/public/favicon.svg @@ -0,0 +1,14 @@ + + + + + + + diff --git a/apps/website/src/layouts/BaseLayout.astro b/apps/website/src/layouts/BaseLayout.astro index d46e85c7b3..5397d96820 100644 --- a/apps/website/src/layouts/BaseLayout.astro +++ b/apps/website/src/layouts/BaseLayout.astro @@ -71,7 +71,7 @@ const websiteJsonLd = { {noindex && } {title} - +