Skip to content

[fix] Fix "Require confirmation before clearing workflow" setting not working #4587

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

Merged
merged 4 commits into from
Jul 30, 2025

Conversation

christian-byrne
Copy link
Contributor

@christian-byrne christian-byrne commented Jul 30, 2025

Description

Fixes the "Require confirmation when clearing workflow" setting (Comfy.ConfirmClear) not working with:

  • The subgraph breadcrumb dropdown menu
  • The new V3 menu system
  • Keybindings assigned to the clear workflow command

The setting only worked correctly in the old UI menu.

Root Cause Analysis

The Original Issue: A typo in src/composables/useCoreCommands.ts line 171

!settingStore.get('Comfy.ComfirmClear') // ❌ Typo: 'ComfirmClear'

Why TypeScript Didn't Catch It: Instead of fixing the typo in the code, someone added the typo as a valid setting to src/schemas/apiSchema.ts

'Comfy.ConfirmClear': z.boolean(),   // ✅ Correct spelling (line 379)
'Comfy.ComfirmClear': z.boolean(),   // ❌ Typo added to schema (line 471)

Changes

  1. Fixed the typo in src/composables/useCoreCommands.ts:

    • 'Comfy.ComfirmClear''Comfy.ConfirmClear'
  2. Removed the duplicate typo setting from src/schemas/apiSchema.ts:

    • Removed 'Comfy.ComfirmClear': z.boolean(),
    • Kept the correct 'Comfy.ConfirmClear': z.boolean(),

Fixes #4585

┆Issue is synchronized with this Notion page by Unito

…CoreCommands

This fixes the "Require confirmation when clearing workflow" setting not working
with the new V3 menu system, breadcrumb dropdown, and keybindings.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@christian-byrne christian-byrne requested a review from a team as a code owner July 30, 2025 04:40
@christian-byrne christian-byrne changed the title [fix] Remove duplicate Comfy.ConfirmClear setting and fix typo in useCoreCommands [fix] Fix "Prompt for confirmation before clearing workflow" setting not working Jul 30, 2025
@christian-byrne christian-byrne changed the title [fix] Fix "Prompt for confirmation before clearing workflow" setting not working [fix] Fix "Require confirmation before clearing workflow" setting not working Jul 30, 2025
@christian-byrne
Copy link
Contributor Author

A Playwright test case now fails because it was using the ClearWorkflow command and not expecting the confirm dialog:

  1 failed
    [chromium] › browser_tests/tests/groupNode.spec.ts:264:5 › Group Node › Copy and paste › Copies and pastes group node after clearing workflow 

@christian-byrne christian-byrne requested a review from a team as a code owner July 30, 2025 05:09
@christian-byrne christian-byrne merged commit 596c51d into main Jul 30, 2025
12 checks passed
@christian-byrne christian-byrne deleted the fix-confirm-clear-typo branch July 30, 2025 07:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Comfy.ConfirmClear setting not working with new command system due to typo
1 participant