Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
d23736b
refactor(groupNode): remove all @ts-expect-error suppressions
DrJKL Jan 10, 2026
56f9e9c
refactor(groupNode): improve type safety and reduce casts
DrJKL Jan 11, 2026
a1ae4aa
refactor: remove 34 @ts-expect-error suppressions from widget-related…
DrJKL Jan 11, 2026
0a99c67
refactor(litegraph): replace type assertions with proper type guards
DrJKL Jan 11, 2026
c66c14a
docs: add type safety rules for TypeScript files
DrJKL Jan 11, 2026
e64a569
docs: expand type-safety guidance with TypeScript handbook and Google…
DrJKL Jan 11, 2026
27b261e
knip fixes
DrJKL Jan 11, 2026
b674b73
fix(services): remove @ts-expect-error suppressions with proper types
DrJKL Jan 11, 2026
3ca02ad
fix: add LiteGraph types to Window interface and remove @ts-expect-er…
DrJKL Jan 11, 2026
7b69b40
fix(types): remove @ts-expect-error suppressions from src/extensions/…
DrJKL Jan 12, 2026
45f99a8
fix(utils): remove 18 @ts-expect-error suppressions
DrJKL Jan 12, 2026
e9e02c2
fix(services): remove 8 @ts-expect-error suppressions from litegraphS…
DrJKL Jan 12, 2026
0590557
fix(platform,renderer): remove 12 @ts-expect-error suppressions
DrJKL Jan 12, 2026
4fd5167
fix(types): remove @ts-expect-error in schemas, stores, and workbench
DrJKL Jan 12, 2026
cf66379
fix(types): remove @ts-expect-error in components and composables
DrJKL Jan 12, 2026
168af53
fix: remove @ts-expect-error suppressions with proper type guards
DrJKL Jan 12, 2026
1b4dd57
fix(types): remove @ts-expect-error suppressions from src/scripts
DrJKL Jan 12, 2026
eb2c4e2
fix(types): add null guards to browser_tests fixtures
DrJKL Jan 12, 2026
4ef1fd9
fix(browser_tests): remove all @ts-expect-error and type assertions
DrJKL Jan 12, 2026
7c06367
refactor: improve type safety patterns from @ts-expect-error cleanup
DrJKL Jan 12, 2026
d1317a3
Merge branch 'main' into drjkl/lowered-expectations
DrJKL Jan 12, 2026
0634364
fix: address coderabbitai review comments
DrJKL Jan 12, 2026
21310c9
fix: address coderabbitai review - XSS, null guards, defaults, priori…
DrJKL Jan 12, 2026
3a0c841
refactor: address trivial coderabbitai review comments
DrJKL Jan 12, 2026
4c46f57
Address PR feedback: refactor browser tests for better type safety
DrJKL Jan 13, 2026
4a12f4b
Address remaining PR feedback: type safety and robustness improvements
DrJKL Jan 13, 2026
634969b
knip fix
DrJKL Jan 13, 2026
73bfcdb
Merge branch 'main' into drjkl/lowered-expectations
DrJKL Jan 13, 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
2 changes: 1 addition & 1 deletion .storybook/preview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import '@/assets/css/style.css'

const ComfyUIPreset = definePreset(Aura, {
semantic: {
// @ts-expect-error fix me
// @ts-expect-error PrimeVue type issue
primary: Aura['primitive'].blue
}
})
Expand Down
1 change: 1 addition & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ See @docs/testing/*.md for detailed patterns.
## External Resources

- Vue: <https://vuejs.org/api/>
- @docs/typescript/type-safety.md
- Tailwind: <https://tailwindcss.com/docs/styling-with-utility-classes>
- VueUse: <https://vueuse.org/functions.html>
- shadcn/vue: <https://www.shadcn-vue.com/>
Expand Down
2 changes: 1 addition & 1 deletion apps/desktop-ui/.storybook/preview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { i18n } from '@/i18n'

const ComfyUIPreset = definePreset(Aura, {
semantic: {
// @ts-expect-error prime type quirk
// @ts-expect-error PrimeVue type issue
primary: Aura['primitive'].blue
}
})
Expand Down
2 changes: 1 addition & 1 deletion apps/desktop-ui/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import router from './router'

const ComfyUIPreset = definePreset(Aura, {
semantic: {
// @ts-expect-error fixme ts strict error
// @ts-expect-error PrimeVue type issue
primary: Aura['primitive'].blue
}
})
Expand Down
155 changes: 113 additions & 42 deletions browser_tests/fixtures/ComfyPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,14 @@ class ComfyMenu {
await this.themeToggleButton.click()
await this.page.evaluate(() => {
return new Promise((resolve) => {
window['app'].ui.settings.addEventListener(
'Comfy.ColorPalette.change',
resolve,
{ once: true }
)
const app = window.app
if (!app) {
resolve(undefined)
return
}
app.ui.settings.addEventListener('Comfy.ColorPalette.change', resolve, {
once: true
})

setTimeout(resolve, 5000)
})
Comment on lines +84 to 94
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Ensure the settings listener is removed if the timeout wins (leaks across tests).

setTimeout(resolve, 5000) can resolve without removing the Comfy.ColorPalette.change listener (since only { once: true } removes it on event). Consider cleaning up on timeout.

🤖 Prompt for AI Agents
In @browser_tests/fixtures/ComfyPage.ts around lines 84 - 94, The timeout path
can resolve without removing the Comfy.ColorPalette.change listener, leaking
across tests; fix by capturing the event handler in a const (e.g., handler =
(ev) => resolve(ev)), add it via
app.ui.settings.addEventListener('Comfy.ColorPalette.change', handler, { once:
true }), and in the timeout callback call
app.ui.settings.removeEventListener('Comfy.ColorPalette.change', handler) (and
clear the timeout if needed) before calling resolve so the listener is always
cleaned up whether the event or timeout wins.

Expand All @@ -94,9 +97,9 @@ class ComfyMenu {

async getThemeId() {
return await this.page.evaluate(async () => {
return await window['app'].ui.settings.getSettingValue(
'Comfy.ColorPalette'
)
const app = window.app
if (!app) return undefined
return await app.ui.settings.getSettingValue('Comfy.ColorPalette')
})
}
}
Expand Down Expand Up @@ -138,7 +141,14 @@ class ConfirmDialog {

// Wait for workflow service to finish if it's busy
await this.page.waitForFunction(
() => window['app']?.extensionManager?.workflow?.isBusy === false,
() => {
const app = window.app
if (!app) return true
const extMgr = app.extensionManager as {
workflow?: { isBusy?: boolean }
}
return extMgr.workflow?.isBusy === false
},
undefined,
{ timeout: 3000 }
)
Comment on lines 143 to 154
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Busy-wait condition can hang when workflow extension is absent/undefined.

extMgr.workflow?.isBusy === false is false when workflow or isBusy is undefined, causing a guaranteed 3s timeout even when there’s nothing to wait for. Prefer “not busy” semantics.

Proposed fix
 await this.page.waitForFunction(
   () => {
     const app = window.app
     if (!app) return true
     const extMgr = app.extensionManager as {
       workflow?: { isBusy?: boolean }
     }
-    return extMgr.workflow?.isBusy === false
+    // Treat missing workflow/isBusy as "not busy" for test stability.
+    return extMgr.workflow?.isBusy !== true
   },
   undefined,
   { timeout: 3000 }
 )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
await this.page.waitForFunction(
() => window['app']?.extensionManager?.workflow?.isBusy === false,
() => {
const app = window.app
if (!app) return true
const extMgr = app.extensionManager as {
workflow?: { isBusy?: boolean }
}
return extMgr.workflow?.isBusy === false
},
undefined,
{ timeout: 3000 }
)
await this.page.waitForFunction(
() => {
const app = window.app
if (!app) return true
const extMgr = app.extensionManager as {
workflow?: { isBusy?: boolean }
}
// Treat missing workflow/isBusy as "not busy" for test stability.
return extMgr.workflow?.isBusy !== true
},
undefined,
{ timeout: 3000 }
)
🤖 Prompt for AI Agents
In @browser_tests/fixtures/ComfyPage.ts around lines 143 - 154, The
waitForFunction busy-check treats undefined workflow/isBusy as false which
causes unnecessary timeout; update the predicate inside
this.page.waitForFunction so it treats missing workflow or missing isBusy as
"not busy" (e.g., use extMgr.workflow?.isBusy !== true or explicitly check
extMgr.workflow == null || extMgr.workflow.isBusy === false) so the function
returns true when the workflow extension is absent; keep the surrounding
structure (this.page.waitForFunction, extMgr, workflow, isBusy) intact and only
change the boolean condition to "not true" semantics.

Expand Down Expand Up @@ -256,7 +266,12 @@ export class ComfyPage {
}

await this.page.evaluate(async () => {
await window['app'].extensionManager.workflow.syncWorkflows()
const app = window.app
if (!app) return
const extMgr = app.extensionManager as {
workflow?: { syncWorkflows: () => Promise<void> }
}
if (extMgr.workflow) await extMgr.workflow.syncWorkflows()
})

// Wait for Vue to re-render the workflow list
Expand Down Expand Up @@ -360,7 +375,9 @@ export class ComfyPage {

async executeCommand(commandId: string) {
await this.page.evaluate((id: string) => {
return window['app'].extensionManager.command.execute(id)
const app = window.app
if (!app) return
return app.extensionManager.command.execute(id)
}, commandId)
}

Expand All @@ -370,7 +387,8 @@ export class ComfyPage {
) {
await this.page.evaluate(
({ commandId, commandStr }) => {
const app = window['app']
const app = window.app
if (!app) return
const randomSuffix = Math.random().toString(36).substring(2, 8)
const extensionName = `TestExtension_${randomSuffix}`

Expand All @@ -391,7 +409,8 @@ export class ComfyPage {
async registerKeybinding(keyCombo: KeyCombo, command: () => void) {
await this.page.evaluate(
({ keyCombo, commandStr }) => {
const app = window['app']
const app = window.app
if (!app) return
const randomSuffix = Math.random().toString(36).substring(2, 8)
const extensionName = `TestExtension_${randomSuffix}`
const commandId = `TestCommand_${randomSuffix}`
Expand Down Expand Up @@ -419,15 +438,19 @@ export class ComfyPage {
async setSetting(settingId: string, settingValue: any) {
return await this.page.evaluate(
async ({ id, value }) => {
await window['app'].extensionManager.setting.set(id, value)
const app = window.app
if (!app) return
await app.extensionManager.setting.set(id, value)
},
{ id: settingId, value: settingValue }
)
}

async getSetting(settingId: string) {
return await this.page.evaluate(async (id) => {
return await window['app'].extensionManager.setting.get(id)
const app = window.app
if (!app) return undefined
return await app.extensionManager.setting.get(id)
}, settingId)
}

Expand Down Expand Up @@ -873,8 +896,10 @@ export class ComfyPage {
const foundSlot = await this.page.evaluate(
async (params) => {
const { slotType, action, targetSlotName } = params
const app = window['app']
const app = window.app
if (!app) throw new Error('App not initialized')
const currentGraph = app.canvas.graph
if (!currentGraph) throw new Error('No graph available')

// Check if we're in a subgraph
if (currentGraph.constructor.name !== 'Subgraph') {
Expand All @@ -883,13 +908,24 @@ export class ComfyPage {
)
}

// Get the appropriate node and slots
// Get the appropriate node and slots (these are Subgraph-specific properties)
const subgraph = currentGraph as {
inputNode?: { onPointerDown?: (...args: unknown[]) => void }
outputNode?: { onPointerDown?: (...args: unknown[]) => void }
inputs?: Array<{
name: string
pos?: number[]
boundingRect?: number[]
}>
outputs?: Array<{
name: string
pos?: number[]
boundingRect?: number[]
}>
}
const node =
slotType === 'input'
? currentGraph.inputNode
: currentGraph.outputNode
const slots =
slotType === 'input' ? currentGraph.inputs : currentGraph.outputs
slotType === 'input' ? subgraph.inputNode : subgraph.outputNode
const slots = slotType === 'input' ? subgraph.inputs : subgraph.outputs

if (!node) {
throw new Error(`No ${slotType} node found in subgraph`)
Expand Down Expand Up @@ -970,15 +1006,19 @@ export class ComfyPage {
}

if (node.onPointerDown) {
// Call onPointerDown for test simulation
node.onPointerDown(
event,
app.canvas.pointer,
app.canvas.linkConnector
)

// Trigger double-click
if (app.canvas.pointer.onDoubleClick) {
app.canvas.pointer.onDoubleClick(event)
const onDoubleClick = app.canvas.pointer.onDoubleClick as
| ((e: unknown) => void)
| undefined
if (onDoubleClick) {
onDoubleClick(event)
}
}

Expand Down Expand Up @@ -1574,7 +1614,9 @@ export class ComfyPage {

async convertOffsetToCanvas(pos: [number, number]) {
return this.page.evaluate((pos) => {
return window['app'].canvas.ds.convertOffsetToCanvas(pos)
const app = window.app
if (!app) return pos
return app.canvas.ds.convertOffsetToCanvas(pos)
}, pos)
}

Expand All @@ -1588,14 +1630,18 @@ export class ComfyPage {
}
async getNodes(): Promise<LGraphNode[]> {
return await this.page.evaluate(() => {
return window['app'].graph.nodes
const app = window.app
if (!app?.graph?.nodes) return []
return app.graph.nodes
})
}
async getNodeRefsByType(type: string): Promise<NodeReference[]> {
return Promise.all(
(
await this.page.evaluate((type) => {
return window['app'].graph.nodes
const app = window.app
if (!app?.graph?.nodes) return []
return app.graph.nodes
.filter((n: LGraphNode) => n.type === type)
.map((n: LGraphNode) => n.id)
}, type)
Expand All @@ -1606,7 +1652,9 @@ export class ComfyPage {
return Promise.all(
(
await this.page.evaluate((title) => {
return window['app'].graph.nodes
const app = window.app
if (!app?.graph?.nodes) return []
return app.graph.nodes
.filter((n: LGraphNode) => n.title === title)
.map((n: LGraphNode) => n.id)
}, title)
Expand All @@ -1616,7 +1664,9 @@ export class ComfyPage {

async getFirstNodeRef(): Promise<NodeReference | null> {
const id = await this.page.evaluate(() => {
return window['app'].graph.nodes[0]?.id
const app = window.app
if (!app?.graph?.nodes) return undefined
return app.graph.nodes[0]?.id
})
if (!id) return null
return this.getNodeRefById(id)
Expand All @@ -1626,32 +1676,41 @@ export class ComfyPage {
}
async getUndoQueueSize() {
return this.page.evaluate(() => {
const workflow = (window['app'].extensionManager as WorkspaceStore)
.workflow.activeWorkflow
return workflow?.changeTracker.undoQueue.length
const app = window.app
if (!app) return 0
const extMgr = app.extensionManager as WorkspaceStore
return extMgr.workflow.activeWorkflow?.changeTracker.undoQueue.length ?? 0
})
}
async getRedoQueueSize() {
return this.page.evaluate(() => {
const workflow = (window['app'].extensionManager as WorkspaceStore)
.workflow.activeWorkflow
return workflow?.changeTracker.redoQueue.length
const app = window.app
if (!app) return 0
const extMgr = app.extensionManager as WorkspaceStore
return extMgr.workflow.activeWorkflow?.changeTracker.redoQueue.length ?? 0
})
}
async isCurrentWorkflowModified() {
return this.page.evaluate(() => {
return (window['app'].extensionManager as WorkspaceStore).workflow
.activeWorkflow?.isModified
const app = window.app
if (!app) return false
const extMgr = app.extensionManager as WorkspaceStore
return extMgr.workflow.activeWorkflow?.isModified ?? false
})
}
async getExportedWorkflow({ api = false }: { api?: boolean } = {}) {
return this.page.evaluate(async (api) => {
return (await window['app'].graphToPrompt())[api ? 'output' : 'workflow']
const app = window.app
if (!app) return undefined
return (await app.graphToPrompt())[api ? 'output' : 'workflow']
}, api)
}
async setFocusMode(focusMode: boolean) {
await this.page.evaluate((focusMode) => {
window['app'].extensionManager.focusMode = focusMode
const app = window.app
if (!app) return
const extMgr = app.extensionManager as { focusMode?: boolean }
extMgr.focusMode = focusMode
}, focusMode)
await this.nextFrame()
}
Expand All @@ -1664,7 +1723,9 @@ export class ComfyPage {
*/
async getGroupPosition(title: string): Promise<Position> {
const pos = await this.page.evaluate((title) => {
const groups = window['app'].graph.groups
const app = window.app
if (!app?.graph?.groups) return null
const groups = app.graph.groups
const group = groups.find((g: { title: string }) => g.title === title)
if (!group) return null
return { x: group.pos[0], y: group.pos[1] }
Expand All @@ -1686,7 +1747,8 @@ export class ComfyPage {
}): Promise<void> {
const { name, deltaX, deltaY } = options
const screenPos = await this.page.evaluate((title) => {
const app = window['app']
const app = window.app
if (!app?.graph?.groups) return null
const groups = app.graph.groups
const group = groups.find((g: { title: string }) => g.title === title)
if (!group) return null
Expand Down Expand Up @@ -1754,11 +1816,16 @@ export const comfyPageFixture = base.extend<{
}
})

interface MatcherContext {
isNot: boolean
}

const makeMatcher = function <T>(
getValue: (node: NodeReference) => Promise<T> | T,
type: string
) {
return async function (
this: MatcherContext,
node: NodeReference,
options?: { timeout?: number; intervals?: number[] }
) {
Expand All @@ -1784,7 +1851,11 @@ export const comfyExpect = expect.extend({
toBePinned: makeMatcher((n) => n.isPinned(), 'pinned'),
toBeBypassed: makeMatcher((n) => n.isBypassed(), 'bypassed'),
toBeCollapsed: makeMatcher((n) => n.isCollapsed(), 'collapsed'),
async toHaveFocus(locator: Locator, options = { timeout: 256 }) {
async toHaveFocus(
this: MatcherContext,
locator: Locator,
options = { timeout: 256 }
) {
const isFocused = await locator.evaluate(
(el) => el === document.activeElement
)
Expand Down
Loading
Loading