Skip to content

Commit 4111782

Browse files
DrJKLampagentactions-user
authored
fix: re-check tab fallback after deferred error scans (#15012)
## Summary Pasted and duplicated nodes register widgets before their deferred missing-model and missing-media scans finish. The right-side panel could therefore recalculate its tabs, see no selected-node error yet, and switch away from Errors permanently. This change tracks graph-scoped added-node scans from node addition through asynchronous verification. An already-active Errors tab remains available while its selected node is being scanned, then either stays active when an error surfaces or falls back when the scan settles cleanly. Scan lifecycle handling now: - reference-counts overlapping scans by root graph and execution ID - releases the tab hold without aborting valid verification during subgraph navigation - aborts verification when the scanned node is removed - suppresses late results after workflow/node replacement, widget changes, or ownership changes - waits for already-started verification when a later scan stage fails ## Testing - Added focused component coverage for pending, error, clean fallback, unmount, and subgraph-interior selection behavior. - Added lifecycle coverage for overlapping scans, graph isolation, disposal, workflow replacement, model/media cancellation, changed values, and scan-stage failures. - Added a Playwright flow covering paste, delayed cloud verification, retained Errors selection, surfaced errors, and clean fallback after resolution. - `pnpm test:unit src/components/rightSidePanel/RightSidePanel.test.ts src/composables/graph/useErrorClearingHooks.test.ts src/stores/executionErrorStore.test.ts` - `pnpm typecheck` - ESLint and Oxfmt on changed files --------- Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: GitHub Action <action@github.com>
1 parent 6b56659 commit 4111782

7 files changed

Lines changed: 944 additions & 48 deletions

File tree

browser_tests/tests/propertiesPanel/errorsTabCloudMissingModels.spec.ts

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,18 @@ const CLOUD_UNKNOWN_MODEL_NAME = 'cloud_unknown_model.safetensors'
3939
const CLOUD_IMPORTED_CANONICAL_MODEL_NAME =
4040
'models/checkpoints/cloud_importable_model.safetensors'
4141

42+
const FAKE_MODEL_ASSET: Asset = {
43+
id: 'test-fake-checkpoint',
44+
name: FAKE_MODEL_NAME,
45+
size: 1_024,
46+
mime_type: 'application/octet-stream',
47+
tags: ['models', 'checkpoints'],
48+
created_at: '2026-05-05T00:00:00Z',
49+
updated_at: '2026-05-05T00:00:00Z',
50+
last_access_time: '2026-05-05T00:00:00Z',
51+
user_metadata: { filename: FAKE_MODEL_NAME }
52+
}
53+
4254
const LOTUS_DIFFUSION_MODEL: Asset & { hash?: string } = {
4355
id: 'test-lotus-depth-d-v1-1',
4456
name: LOTUS_MODEL_NAME,
@@ -183,6 +195,64 @@ test.describe(
183195
await expect(errorsTab).toBeHidden()
184196
})
185197

198+
test('keeps Errors active through pasted-node verification and falls back when resolved', async ({
199+
comfyPage
200+
}) => {
201+
await loadWorkflowAndOpenErrorsTab(comfyPage, 'missing/missing_models')
202+
const panel = new PropertiesPanelHelper(comfyPage.page)
203+
const missingModelsGroup = comfyPage.page.getByTestId(
204+
TestIds.dialogs.missingModelsGroup
205+
)
206+
let visibleAssets: Asset[] = []
207+
let markVerificationStarted: () => void = () => undefined
208+
const verificationStarted = new Promise<void>((resolve) => {
209+
markVerificationStarted = resolve
210+
})
211+
let releaseVerification: () => void = () => undefined
212+
const verificationGate = new Promise<void>((resolve) => {
213+
releaseVerification = resolve
214+
})
215+
await comfyPage.page.route(/\/api\/assets(?:\?.*)?$/, async (route) => {
216+
markVerificationStarted()
217+
await verificationGate
218+
const response: ListAssetsResponse = {
219+
assets: visibleAssets,
220+
total: visibleAssets.length,
221+
has_more: false
222+
}
223+
await route.fulfill({ json: response })
224+
})
225+
226+
const source = await comfyPage.nodeOps.getNodeRefById('1')
227+
await source.click('title')
228+
await comfyPage.clipboard.copy()
229+
await comfyPage.clipboard.paste()
230+
await verificationStarted
231+
232+
await expect(panel.errorsTab).toHaveAttribute('aria-selected', 'true')
233+
releaseVerification()
234+
await expect.poll(() => comfyPage.nodeOps.getNodeCount()).toBe(2)
235+
await expect(
236+
missingModelsGroup.getByTestId(
237+
TestIds.dialogs.missingModelReferenceCount
238+
)
239+
).toHaveText('2')
240+
await expect(panel.errorsTab).toHaveAttribute('aria-selected', 'true')
241+
242+
visibleAssets = [FAKE_MODEL_ASSET]
243+
await source.click('title')
244+
await comfyPage.clipboard.copy()
245+
await comfyPage.clipboard.paste()
246+
247+
await expect.poll(() => comfyPage.nodeOps.getNodeCount()).toBe(3)
248+
await expect(missingModelsGroup).toBeHidden()
249+
await expect(panel.errorsTab).toBeHidden()
250+
await expect(panel.getTab('Parameters')).toHaveAttribute(
251+
'aria-selected',
252+
'true'
253+
)
254+
})
255+
186256
test('separates importable cloud models from unsupported rows', async ({
187257
comfyPage
188258
}) => {
Lines changed: 196 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,196 @@
1+
import { createTestingPinia } from '@pinia/testing'
2+
import { render, screen } from '@testing-library/vue'
3+
import { setActivePinia } from 'pinia'
4+
import { beforeEach, describe, expect, it, vi } from 'vitest'
5+
import { markRaw, nextTick } from 'vue'
6+
import { createI18n } from 'vue-i18n'
7+
8+
import RightSidePanel from '@/components/rightSidePanel/RightSidePanel.vue'
9+
import { LGraph, LGraphNode } from '@/lib/litegraph/src/litegraph'
10+
import {
11+
createTestSubgraph,
12+
createTestSubgraphNode
13+
} from '@/lib/litegraph/src/subgraph/__fixtures__/subgraphHelpers'
14+
import enMessages from '@/locales/en/main.json' with { type: 'json' }
15+
import { useMissingModelStore } from '@/platform/missingModel/missingModelStore'
16+
import { useCanvasStore } from '@/renderer/core/canvas/canvasStore'
17+
import { useExecutionErrorStore } from '@/stores/executionErrorStore'
18+
import { useRightSidePanelStore } from '@/stores/workspace/rightSidePanelStore'
19+
import { getExecutionIdByNode } from '@/utils/graphTraversalUtil'
20+
import { toNodeId } from '@/types/nodeId'
21+
22+
const mockApp = vi.hoisted(() => ({
23+
isGraphReady: true,
24+
rootGraph: null as LGraph | null
25+
}))
26+
27+
vi.mock('@/scripts/app', () => ({ app: mockApp }))
28+
29+
vi.mock('@/composables/graph/useGraphHierarchy', () => ({
30+
useGraphHierarchy: () => ({ findParentGroup: vi.fn(() => null) })
31+
}))
32+
33+
vi.mock('@/platform/telemetry', () => ({ useTelemetry: () => undefined }))
34+
35+
vi.mock('@/platform/settings/settingStore', () => ({
36+
useSettingStore: () => ({
37+
get: (key: string) => {
38+
if (key === 'Comfy.RightSidePanel.ShowErrorsTab') return true
39+
if (key === 'Comfy.Sidebar.Location') return 'left'
40+
if (key === 'Comfy.UseNewMenu') return 'Top'
41+
if (key === 'Comfy.RightSidePanel.IsOpen') return true
42+
return undefined
43+
},
44+
set: vi.fn()
45+
})
46+
}))
47+
48+
function renderPanel(
49+
activeTab: 'errors' | 'parameters' = 'errors',
50+
graphContext?: {
51+
rootGraph: LGraph
52+
currentGraph: LGraph
53+
node: LGraphNode
54+
}
55+
) {
56+
const pinia = createTestingPinia({ createSpy: vi.fn, stubActions: false })
57+
setActivePinia(pinia)
58+
59+
const rootGraph = graphContext?.rootGraph ?? new LGraph()
60+
const currentGraph = graphContext?.currentGraph ?? rootGraph
61+
const node = graphContext?.node ?? new LGraphNode('CheckpointLoaderSimple')
62+
if (!graphContext) {
63+
node.id = toNodeId(1)
64+
rootGraph.add(node)
65+
}
66+
mockApp.rootGraph = rootGraph
67+
68+
const canvasStore = useCanvasStore()
69+
canvasStore.currentGraph = currentGraph
70+
canvasStore.selectedItems = [markRaw(node)]
71+
72+
const rightSidePanelStore = useRightSidePanelStore()
73+
rightSidePanelStore.activeTab = activeTab
74+
const executionErrorStore = useExecutionErrorStore()
75+
const executionId = getExecutionIdByNode(rootGraph, node)
76+
if (!executionId) throw new Error('Expected selected node execution ID')
77+
const finishScan = executionErrorStore.beginAddedNodeErrorScan(
78+
rootGraph,
79+
executionId
80+
)
81+
const openPanel = vi.spyOn(rightSidePanelStore, 'openPanel')
82+
83+
const i18n = createI18n({
84+
legacy: false,
85+
locale: 'en',
86+
messages: { en: enMessages }
87+
})
88+
89+
const rendered = render(RightSidePanel, {
90+
global: {
91+
plugins: [pinia, i18n],
92+
stubs: {
93+
Button: { template: '<button><slot /></button>' },
94+
EditableText: true,
95+
Tab: { template: '<button v-bind="$attrs"><slot /></button>' },
96+
TabErrors: true,
97+
TabInfo: true,
98+
TabList: { template: '<div><slot /></div>' },
99+
TabNormalInputs: true,
100+
TabSettings: true
101+
}
102+
}
103+
})
104+
105+
return {
106+
...rendered,
107+
executionId,
108+
executionErrorStore,
109+
finishScan,
110+
graph: rootGraph,
111+
node,
112+
openPanel,
113+
rightSidePanelStore
114+
}
115+
}
116+
117+
describe('RightSidePanel active tab fallback', () => {
118+
beforeEach(() => {
119+
vi.restoreAllMocks()
120+
mockApp.rootGraph = null
121+
})
122+
123+
it('keeps the active errors tab until the selected node scan settles', async () => {
124+
const { finishScan, openPanel, rightSidePanelStore } = renderPanel()
125+
126+
expect(screen.getByTestId('panel-tab-errors')).toBeInTheDocument()
127+
expect(rightSidePanelStore.activeTab).toBe('errors')
128+
expect(openPanel).not.toHaveBeenCalled()
129+
130+
vi.spyOn(globalThis, 'queueMicrotask').mockImplementation(() => undefined)
131+
finishScan()
132+
await nextTick()
133+
134+
expect(rightSidePanelStore.activeTab).toBe('parameters')
135+
expect(openPanel).toHaveBeenCalledWith('parameters')
136+
})
137+
138+
it('keeps errors active when the scan surfaces an error before settling', async () => {
139+
const { executionId, finishScan, openPanel, rightSidePanelStore } =
140+
renderPanel()
141+
142+
useMissingModelStore().addMissingModels([
143+
{
144+
nodeId: executionId,
145+
nodeType: 'CheckpointLoaderSimple',
146+
widgetName: 'ckpt_name',
147+
isAssetSupported: false,
148+
name: 'missing.safetensors',
149+
directory: 'checkpoints',
150+
isMissing: true
151+
}
152+
])
153+
finishScan()
154+
await nextTick()
155+
156+
expect(rightSidePanelStore.activeTab).toBe('errors')
157+
expect(openPanel).not.toHaveBeenCalled()
158+
})
159+
160+
it('does not show errors solely because a scan is pending', () => {
161+
const { openPanel } = renderPanel('parameters')
162+
163+
expect(screen.queryByTestId('panel-tab-errors')).not.toBeInTheDocument()
164+
expect(openPanel).not.toHaveBeenCalled()
165+
})
166+
167+
it('does not update the panel when a pending scan finishes after unmount', async () => {
168+
const { finishScan, openPanel, unmount } = renderPanel()
169+
unmount()
170+
finishScan()
171+
await nextTick()
172+
173+
expect(openPanel).not.toHaveBeenCalled()
174+
})
175+
176+
it('keeps errors active for a pending subgraph interior node scan', () => {
177+
setActivePinia(createTestingPinia({ createSpy: vi.fn, stubActions: false }))
178+
const subgraph = createTestSubgraph()
179+
const node = new LGraphNode('CheckpointLoaderSimple')
180+
node.id = toNodeId(7)
181+
subgraph.add(node)
182+
const host = createTestSubgraphNode(subgraph, { id: 65 })
183+
const rootGraph = host.graph as LGraph
184+
rootGraph.add(host)
185+
186+
const { executionErrorStore, executionId, openPanel, rightSidePanelStore } =
187+
renderPanel('errors', { rootGraph, currentGraph: subgraph, node })
188+
189+
expect(
190+
executionErrorStore.hasPendingAddedNodeErrorScan(rootGraph, executionId)
191+
).toBe(true)
192+
expect(screen.getByTestId('panel-tab-errors')).toBeInTheDocument()
193+
expect(rightSidePanelStore.activeTab).toBe('errors')
194+
expect(openPanel).not.toHaveBeenCalled()
195+
})
196+
})

src/components/rightSidePanel/RightSidePanel.vue

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,10 @@ import TabList from '@/components/tab/TabList.vue'
99
import Button from '@/components/ui/button/Button.vue'
1010
import { useGraphHierarchy } from '@/composables/graph/useGraphHierarchy'
1111
import { app } from '@/scripts/app'
12-
import { getActiveGraphNodeIds } from '@/utils/graphTraversalUtil'
12+
import {
13+
getActiveGraphNodeIds,
14+
getExecutionIdByNode
15+
} from '@/utils/graphTraversalUtil'
1316
import { SubgraphNode } from '@/lib/litegraph/src/litegraph'
1417
import type { LGraphNode } from '@/lib/litegraph/src/litegraph'
1518
import { useSettingStore } from '@/platform/settings/settingStore'
@@ -173,12 +176,26 @@ const hasRelevantErrors = computed(() => {
173176
)
174177
})
175178
179+
const hasPendingErrorScanSelected = computed(() => {
180+
const nodes = selectedNodes.value
181+
if (!app.isGraphReady) return false
182+
const rootGraph = app.rootGraph
183+
return nodes.some((node) => {
184+
const executionId = getExecutionIdByNode(rootGraph, node)
185+
return (
186+
executionId !== null &&
187+
executionErrorStore.hasPendingAddedNodeErrorScan(rootGraph, executionId)
188+
)
189+
})
190+
})
191+
176192
const tabs = computed<RightSidePanelTabList>(() => {
177193
const list: RightSidePanelTabList = []
178194
179195
if (
180196
settingStore.get('Comfy.RightSidePanel.ShowErrorsTab') &&
181-
hasRelevantErrors.value
197+
(hasRelevantErrors.value ||
198+
(activeTab.value === 'errors' && hasPendingErrorScanSelected.value))
182199
) {
183200
list.push({
184201
label: () => t('rightSidePanel.errors'),
@@ -222,12 +239,15 @@ const tabs = computed<RightSidePanelTabList>(() => {
222239
return list
223240
})
224241
225-
// Use global state for activeTab and ensure it's valid
242+
function isActiveTabAvailable() {
243+
return (
244+
tabs.value.some((tab) => tab.value === activeTab.value) ||
245+
(activeTab.value === 'subgraph' && isSingleSubgraphNode.value)
246+
)
247+
}
248+
226249
watchEffect(() => {
227-
if (
228-
!tabs.value.some((tab) => tab.value === activeTab.value) &&
229-
!(activeTab.value === 'subgraph' && isSingleSubgraphNode.value)
230-
) {
250+
if (!isActiveTabAvailable()) {
231251
rightSidePanelStore.openPanel(tabs.value[0].value)
232252
}
233253
})

0 commit comments

Comments
 (0)