Skip to content

Conversation

viva-jinyi
Copy link
Collaborator

@viva-jinyi viva-jinyi commented Sep 18, 2025

Summary

  • Updated frontend to align with backend changes in ComfyUI core PR #7555
  • Changed manager startup argument from --disable-manager (opt-out) to --enable-manager (opt-in)
  • Manager is now disabled by default unless explicitly enabled

Changes

  • Modified useManagerState.ts to check for --enable-manager flag presence
  • Inverted logic: manager is disabled when the flag is NOT present
  • Updated all related tests to reflect the new opt-in behavior
  • Fixed edge case where systemStats is null

Related

Test Plan

  • All unit tests pass
  • Verified manager state logic with different flag combinations
  • TypeScript type checking passes
  • Linting passes

🤖 Generated with Claude Code

┆Issue is synchronized with this Notion page by Unito

- Updated useManagerState to check for --enable-manager flag presence
- Inverted logic: manager is now disabled by default unless --enable-manager is present
- Updated all related tests to reflect the new opt-in behavior
- Aligns with backend changes in ComfyUI core PR #7555
- When systemStats is null, --enable-manager flag cannot be checked
- Manager should be disabled in this case as the flag is not present
- Updated test expectation to reflect correct behavior
@viva-jinyi viva-jinyi requested review from a team as code owners September 18, 2025 12:56
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Sep 18, 2025
Copy link

github-actions bot commented Sep 18, 2025

🎭 Playwright Test Results

All tests passed!

⏰ Completed at: 09/18/2025, 01:08:04 PM UTC

📈 Summary

  • Total Tests: 450
  • Passed: 421 ✅
  • Failed: 0
  • Flaky: 0
  • Skipped: 29 ⏭️

📊 Test Reports by Browser

  • chromium: View Report • ✅ 414 / ❌ 0 / ⚠️ 0 / ⏭️ 29
  • chromium-2x: View Report • ✅ 2 / ❌ 0 / ⚠️ 0 / ⏭️ 0
  • chromium-0.5x: View Report • ✅ 1 / ❌ 0 / ⚠️ 0 / ⏭️ 0
  • mobile-chrome: View Report • ✅ 4 / ❌ 0 / ⚠️ 0 / ⏭️ 0

🎉 Click on the links above to view detailed test results for each browser configuration.

@christian-byrne christian-byrne added needs-backport Fix/change that needs to be cherry-picked to the current feature freeze branch 1.26 labels Sep 18, 2025
@christian-byrne christian-byrne merged commit a41b8a6 into main Sep 18, 2025
36 checks passed
@christian-byrne christian-byrne deleted the manager/flag-change branch September 18, 2025 18:45
github-actions bot pushed a commit that referenced this pull request Sep 18, 2025
…ger (#5635)

## Summary
- Updated frontend to align with backend changes in ComfyUI core PR
#7555
- Changed manager startup argument from `--disable-manager` (opt-out) to
`--enable-manager` (opt-in)
- Manager is now disabled by default unless explicitly enabled

## Changes
- Modified `useManagerState.ts` to check for `--enable-manager` flag
presence
- Inverted logic: manager is disabled when the flag is NOT present
- Updated all related tests to reflect the new opt-in behavior
- Fixed edge case where `systemStats` is null

## Related
- Backend PR: comfyanonymous/ComfyUI#7555

## Test Plan
- [x] All unit tests pass
- [x] Verified manager state logic with different flag combinations
- [x] TypeScript type checking passes
- [x] Linting passes

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

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-5635-refactor-Change-manager-flag-from-disable-manager-to-enable-manager-2726d73d36508153a88bd9f152132b2a)
by [Unito](https://www.unito.io)
@comfy-pr-bot
Copy link
Member

@viva-jinyi Successfully backported to #5639

christian-byrne pushed a commit that referenced this pull request Sep 18, 2025
…to --enable-manager (#5639)

Backport of #5635 to `core/1.26`

Automatically created by backport workflow.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-5639-backport-1-26-refactor-Change-manager-flag-from-disable-manager-to-enable-manager-2726d73d36508120a29fcaefd91dbf40)
by [Unito](https://www.unito.io)

Co-authored-by: Jin Yi <jin12cc@gmail.com>
@christian-byrne christian-byrne added needs-backport Fix/change that needs to be cherry-picked to the current feature freeze branch 1.27 and removed needs-backport Fix/change that needs to be cherry-picked to the current feature freeze branch 1.26 1.27 labels Sep 18, 2025
christian-byrne added a commit that referenced this pull request Sep 18, 2025
## What's Changed

### 🐛 Bug Fixes
- Change manager flag from --disable-manager to --enable-manager to
align with backend changes (#5635)

This hotfix ensures frontend compatibility with ComfyUI core PR #7555,
changing the manager startup behavior from opt-out to opt-in.

**Full Changelog**:
v1.26.11...v1.26.12
EOF < /dev/null

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-5641-Release-v1-26-12-2726d73d36508141aae1efa8f2bc4b08)
by [Unito](https://www.unito.io)
@christian-byrne christian-byrne added needs-backport Fix/change that needs to be cherry-picked to the current feature freeze branch 1.27 labels Sep 18, 2025
christian-byrne pushed a commit that referenced this pull request Sep 19, 2025
…ger (#5635)

## Summary
- Updated frontend to align with backend changes in ComfyUI core PR
#7555
- Changed manager startup argument from `--disable-manager` (opt-out) to
`--enable-manager` (opt-in)
- Manager is now disabled by default unless explicitly enabled

## Changes
- Modified `useManagerState.ts` to check for `--enable-manager` flag
presence
- Inverted logic: manager is disabled when the flag is NOT present
- Updated all related tests to reflect the new opt-in behavior
- Fixed edge case where `systemStats` is null

## Related
- Backend PR: comfyanonymous/ComfyUI#7555

## Test Plan
- [x] All unit tests pass
- [x] Verified manager state logic with different flag combinations
- [x] TypeScript type checking passes
- [x] Linting passes

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

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-5635-refactor-Change-manager-flag-from-disable-manager-to-enable-manager-2726d73d36508153a88bd9f152132b2a)
by [Unito](https://www.unito.io)
christian-byrne added a commit that referenced this pull request Sep 19, 2025
Cherry-pick of PR #5635: refactor: Change manager flag from
--disable-manager to --enable-manager

## Summary
- Cherry-picked commit a41b8a6
- Updates frontend to align with backend changes in ComfyUI core PR
#7555
- Changed manager startup argument from `--disable-manager` (opt-out) to
`--enable-manager` (opt-in)
- Manager is now disabled by default unless explicitly enabled

## Original Changes
- Modified `useManagerState.ts` to check for `--enable-manager` flag
presence
- Inverted logic: manager is disabled when the flag is NOT present
- Updated all related tests to reflect the new opt-in behavior
- Fixed edge case where `systemStats` is null

## Testing
- ✅ TypeScript type checking passed
- ✅ ESLint linting passed
- ✅ Prettier formatting passed
- ✅ Knip found no issues
- ✅ Cherry-pick applied cleanly with no conflicts

## Related
- Original PR: #5635
- Backend PR: comfyanonymous/ComfyUI#7555

This hotfix ensures the frontend manager flag logic matches the backend
changes.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-5646-backport-Cherry-pick-manager-flag-refactor-to-core-1-27-2736d73d365081d38d8fddd6e451e156)
by [Unito](https://www.unito.io)

Co-authored-by: Jin Yi <jin12cc@gmail.com>
Myestery pushed a commit that referenced this pull request Sep 19, 2025
…ger (#5635)

## Summary
- Updated frontend to align with backend changes in ComfyUI core PR
#7555
- Changed manager startup argument from `--disable-manager` (opt-out) to
`--enable-manager` (opt-in)
- Manager is now disabled by default unless explicitly enabled

## Changes
- Modified `useManagerState.ts` to check for `--enable-manager` flag
presence
- Inverted logic: manager is disabled when the flag is NOT present
- Updated all related tests to reflect the new opt-in behavior
- Fixed edge case where `systemStats` is null

## Related
- Backend PR: comfyanonymous/ComfyUI#7555

## Test Plan
- [x] All unit tests pass
- [x] Verified manager state logic with different flag combinations
- [x] TypeScript type checking passes
- [x] Linting passes

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

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-5635-refactor-Change-manager-flag-from-disable-manager-to-enable-manager-2726d73d36508153a88bd9f152132b2a)
by [Unito](https://www.unito.io)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.27 area:manager needs-backport Fix/change that needs to be cherry-picked to the current feature freeze branch size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants