-
Notifications
You must be signed in to change notification settings - Fork 490
[bugfix] Clear queue button now properly removes initializing jobs from UI #8203
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🎭 Playwright Tests:
|
📝 WalkthroughWalkthroughQueue cancellation and clear-queue flows now also remove associated initialization tracking in the ExecutionStore. For cloud deployments item deletion (api.deleteItem) is used for running/initializing jobs; for local deployments api.interrupt remains. Batch and single-item clear operations call ExecutionStore to remove prompt IDs from initializing state and then update queue state. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Queue UI / Sidebar
participant QueueStore
participant ExecutionStore
participant API
User->>UI: click cancel item / clear queue
UI->>QueueStore: read pendingTasks / target id(s)
UI->>API: if cloud -> deleteItem('queue', id) else -> interrupt(id)
API-->>UI: success
UI->>ExecutionStore: clearInitializationByPromptId(s) (single or batch)
ExecutionStore-->>UI: cleared
UI->>QueueStore: update() / dispatch clear pending tasks
QueueStore-->>UI: updated state
UI-->>User: UI reflects cancelled/cleared queue
Possibly related PRs
Suggested reviewers
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 22.4 kB (baseline 22.4 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 1.02 MB (baseline 1.02 MB) • 🔴 +521 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 80.7 kB (baseline 80.7 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 9 added / 9 removed Panels & Settings — 430 kB (baseline 430 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 8 added / 8 removed User & Accounts — 3.94 kB (baseline 3.94 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 3 added / 3 removed Editors & Dialogs — 2.8 kB (baseline 2.8 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 2 added / 2 removed UI Components — 32.8 kB (baseline 32.8 kB) • ⚪ 0 BReusable component library chunks
Status: 5 added / 5 removed Data & Services — 3.04 MB (baseline 3.04 MB) • 🔴 +926 BStores, services, APIs, and repositories
Status: 7 added / 7 removed Utilities & Hooks — 18.8 kB (baseline 18.8 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 4 added / 4 removed Vendor & Third-Party — 10.4 MB (baseline 10.4 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 6.28 MB (baseline 6.28 MB) • ⚪ 0 BBundles that do not match a named category
Status: 25 added / 25 removed |
- Add isCloud mock for Cloud environment handling - Add ComfyWorkflow export to workflowStore mock - Add executionStore mock for clearInitializationByPromptId Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/composables/queue/useJobMenu.test.ts`:
- Around line 112-117: The tests for the cancel behavior currently mock
executionStore.clearInitializationByPromptId but never assert it was invoked;
update the cancel-related tests to assert that
executionStoreMock.clearInitializationByPromptId was called with the correct
promptId when canceling both running and initializing jobs. Locate the cancel
test cases (the tests exercising the cancel action/path) and add expectations
like
expect(executionStoreMock.clearInitializationByPromptId).toHaveBeenCalledWith(expectedPromptId)
and corresponding toHaveBeenCalled checks for each scenario to prevent
regressions; ensure you reference the same promptId used in the test setup so
the assertion verifies the intended removal behavior.
- Around line 8-10: Add tests covering the Cloud-specific cancel path by mocking
isCloud true per-test (use vi.hoisted to expose an isCloudMock with a getter
like in the suggestion), then in a new test for cancelJob (use mountJobMenu,
setCurrentItem with createJobItem state 'running' or 'initializing') set
isCloudMock.value = true, call cancelJob, and assert that deleteItemMock
(api.deleteItem) was called with the expected args and interruptMock
(api.interrupt) was not called; also add the inverse check for isCloudMock.value
= false to ensure existing behavior remains unchanged.
| vi.mock('@/platform/distribution/types', () => ({ | ||
| isCloud: false | ||
| })) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Missing test coverage for Cloud environment cancel behavior.
The mock sets isCloud: false, which tests local behavior. Per the PR objectives, Cloud environments require api.deleteItem() instead of api.interrupt() for canceling running/initializing jobs. Consider adding test cases that mock isCloud: true and verify the Cloud-specific cancel path.
Suggested approach using vi.hoisted for per-test isCloud control
const { isCloudMock } = vi.hoisted(() => ({
isCloudMock: { value: false }
}))
vi.mock('@/platform/distribution/types', () => ({
get isCloud() {
return isCloudMock.value
}
}))
// Then in test:
it('cancels running job via deleteItem in Cloud environment', async () => {
isCloudMock.value = true
const { cancelJob } = mountJobMenu()
setCurrentItem(createJobItem({ state: 'running' }))
await cancelJob()
expect(deleteItemMock).toHaveBeenCalledWith('queue', 'job-1')
expect(interruptMock).not.toHaveBeenCalled()
})🤖 Prompt for AI Agents
In `@src/composables/queue/useJobMenu.test.ts` around lines 8 - 10, Add tests
covering the Cloud-specific cancel path by mocking isCloud true per-test (use
vi.hoisted to expose an isCloudMock with a getter like in the suggestion), then
in a new test for cancelJob (use mountJobMenu, setCurrentItem with createJobItem
state 'running' or 'initializing') set isCloudMock.value = true, call cancelJob,
and assert that deleteItemMock (api.deleteItem) was called with the expected
args and interruptMock (api.interrupt) was not called; also add the inverse
check for isCloudMock.value = false to ensure existing behavior remains
unchanged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is true
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@benceruleanlu, thanks for confirming! Would you like me to add the test coverage for the Cloud environment cancel behavior? I can implement the suggestion using vi.hoisted() to allow per-test control of the isCloud mock value, which would verify both the Cloud path (using api.deleteItem()) and the local path (using api.interrupt()) for running and initializing jobs.
Just let me know if you'd like me to proceed with adding those tests! 🐰
🧠 Learnings used
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T21:33:34.380Z
Learning: Applies to +(tests-ui|src)/**/*.test.ts : For mocking in tests, leverage Vitest's utilities; keep module mocks contained without global mutable state
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T21:33:34.380Z
Learning: Applies to +(tests-ui|src)/**/*.test.ts : For component testing, use Vue Test Utils and make components easy to test
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T21:33:34.380Z
Learning: Applies to +(tests-ui|src)/**/*.test.ts : Use `vi.hoisted()` if necessary to allow per-test Arrange phase manipulation of mock state
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T21:33:34.380Z
Learning: Applies to +(tests-ui|src|browser_tests)/**/*.+(test.ts|spec.ts) : Do not mock what you don't own; be selective with external dependencies
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T21:33:34.380Z
Learning: Applies to +(tests-ui|src)/**/*.test.ts : Aim for behavioral coverage of critical and new features
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T21:33:34.380Z
Learning: Applies to +(tests-ui|src)/**/*.test.ts : Do not write tests that just test the mocks; ensure tests fail when code behaves unexpectedly
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T21:33:34.380Z
Learning: Applies to +(tests-ui|src|browser_tests)/**/*.+(test.ts|spec.ts) : Write tests for all changes, especially bug fixes to catch regressions
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7894
File: src/renderer/extensions/vueNodes/widgets/components/WidgetToggleSwitch.test.ts:11-14
Timestamp: 2026-01-08T02:40:22.621Z
Learning: In the Comfy-Org/ComfyUI_frontend repository test files: When testing components, import the real type definitions from the component files instead of duplicating interface definitions in the test files. This prevents type drift and maintains consistency.
Learnt from: Myestery
Repo: Comfy-Org/ComfyUI_frontend PR: 7422
File: .github/workflows/pr-update-playwright-expectations.yaml:131-135
Timestamp: 2025-12-12T23:02:37.473Z
Learning: In the `.github/workflows/pr-update-playwright-expectations.yaml` workflow in the Comfy-Org/ComfyUI_frontend repository, the snapshot update process is intentionally scoped to only add and update snapshot images. Deletions of snapshot files are handled explicitly outside this workflow and should not be suggested as part of this automation.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7649
File: tests-ui/tests/platform/cloud/subscription/components/SubscriptionPanel.test.ts:189-194
Timestamp: 2025-12-22T21:36:46.909Z
Learning: In the Comfy-Org/ComfyUI_frontend repository test files: Do not stub primitive UI components or customized primitive components (e.g., Button). Instead, import and register the real components in test setup. This ensures tests accurately reflect production behavior and component API usage.
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/AGENTS.md:0-0
Timestamp: 2026-01-16T21:33:39.388Z
Learning: Applies to browser_tests/**/*.spec.ts : Use premade JSON workflows in the `assets/` directory to load desired graph state in E2E tests
Learnt from: henrikvilhelmberglund
Repo: Comfy-Org/ComfyUI_frontend PR: 7617
File: src/components/actionbar/ComfyActionbar.vue:301-308
Timestamp: 2025-12-18T16:03:09.642Z
Learning: In the ComfyUI frontend queue system (src/stores/queueStore.ts), the `useQueuePendingTaskCountStore().count` includes the currently executing task. When count = 1, there is only the active/running task with no pending tasks. When count > 1, there is an active task plus pending tasks waiting in the queue. The "Clear Pending Tasks" button should only be enabled when count > 1 to avoid clearing the currently running task, which should be handled by the "Cancel current run" button instead.
| const executionStoreMock = { | ||
| clearInitializationByPromptId: vi.fn() | ||
| } | ||
| vi.mock('@/stores/executionStore', () => ({ | ||
| useExecutionStore: () => executionStoreMock | ||
| })) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing test assertions for clearInitializationByPromptId behavior.
The mock is added but the cancel tests (lines 255-288) don't verify that clearInitializationByPromptId is called when canceling running or initializing jobs. Since this PR fixes a bug where initializing jobs weren't removed from the UI on cancellation, tests should assert this behavior to prevent regressions.
Suggested test enhancement
it.each([
['running', interruptMock, deleteItemMock],
['initialization', interruptMock, deleteItemMock]
])('cancels %s job via interrupt', async (state) => {
const { cancelJob } = mountJobMenu()
setCurrentItem(createJobItem({ state: state as any }))
await cancelJob()
expect(interruptMock).toHaveBeenCalledWith('job-1')
expect(deleteItemMock).not.toHaveBeenCalled()
expect(queueStoreMock.update).toHaveBeenCalled()
+ expect(executionStoreMock.clearInitializationByPromptId).toHaveBeenCalledWith('job-1')
})
it('cancels pending job via deleteItem', async () => {
const { cancelJob } = mountJobMenu()
setCurrentItem(createJobItem({ state: 'pending' }))
await cancelJob()
expect(deleteItemMock).toHaveBeenCalledWith('queue', 'job-1')
expect(queueStoreMock.update).toHaveBeenCalled()
+ expect(executionStoreMock.clearInitializationByPromptId).toHaveBeenCalledWith('job-1')
})🤖 Prompt for AI Agents
In `@src/composables/queue/useJobMenu.test.ts` around lines 112 - 117, The tests
for the cancel behavior currently mock
executionStore.clearInitializationByPromptId but never assert it was invoked;
update the cancel-related tests to assert that
executionStoreMock.clearInitializationByPromptId was called with the correct
promptId when canceling both running and initializing jobs. Locate the cancel
test cases (the tests exercising the cancel action/path) and add expectations
like
expect(executionStoreMock.clearInitializationByPromptId).toHaveBeenCalledWith(expectedPromptId)
and corresponding toHaveBeenCalled checks for each scenario to prevent
regressions; ensure you reference the same promptId used in the test setup so
the assertion verifies the intended removal behavior.
benceruleanlu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, it really looks like we need to refactor this for readability later, out of scope for this PR though
…om UI (Comfy-Org#8203) ## Summary - Fix: Initializing jobs now properly disappear from UI when cancelled or cleared - Add `clearInitializationByPromptIds` batch function for optimized Set operations - Handle Cloud vs local environment correctly (use `api.deleteItem` for Cloud, `api.interrupt` for local) ## Problem When clicking 'Clear queue' button or X button on initializing jobs, the jobs remained visible in both AssetsSidebarListView and JobQueue components until page refresh. ## Root Cause 1. `initializingPromptIds` in `executionStore` was not being cleared when jobs were cancelled/deleted 2. Cloud environment requires `api.deleteItem()` instead of `api.interrupt()` for cancellation ## Changes - `src/stores/executionStore.ts`: Export `clearInitializationByPromptId` and add batch `clearInitializationByPromptIds` function - `src/composables/queue/useJobMenu.ts`: Add Cloud branch handling and initialization cleanup - `src/components/queue/QueueProgressOverlay.vue`: Fix `onCancelItem()`, `cancelQueuedWorkflows()`, `interruptAll()` - `src/components/sidebar/tabs/AssetsSidebarTab.vue`: Add initialization cleanup to `handleClearQueue()` [screen-capture.webm](https://github.com/user-attachments/assets/0bf911c2-d8f4-427c-96e0-4784e8fe0f08) 🤖 Generated with [Claude Code](https://claude.com/claude-code) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8203-bugfix-Clear-queue-button-now-properly-removes-initializing-jobs-from-UI-2ef6d73d36508162a55bd84ad39ab49c) by [Unito](https://www.unito.io) --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
|
1 similar comment
|

Summary
clearInitializationByPromptIdsbatch function for optimized Set operationsapi.deleteItemfor Cloud,api.interruptfor local)Problem
When clicking 'Clear queue' button or X button on initializing jobs, the jobs remained visible in both AssetsSidebarListView and JobQueue components until page refresh.
Root Cause
initializingPromptIdsinexecutionStorewas not being cleared when jobs were cancelled/deletedapi.deleteItem()instead ofapi.interrupt()for cancellationChanges
src/stores/executionStore.ts: ExportclearInitializationByPromptIdand add batchclearInitializationByPromptIdsfunctionsrc/composables/queue/useJobMenu.ts: Add Cloud branch handling and initialization cleanupsrc/components/queue/QueueProgressOverlay.vue: FixonCancelItem(),cancelQueuedWorkflows(),interruptAll()src/components/sidebar/tabs/AssetsSidebarTab.vue: Add initialization cleanup tohandleClearQueue()screen-capture.webm
🤖 Generated with Claude Code
┆Issue is synchronized with this Notion page by Unito