Skip to content

Conversation

@christian-byrne
Copy link
Contributor

@christian-byrne christian-byrne commented Sep 3, 2025

Summary

Updates the frontend PR functionality (#309) to support the ComfyUI frontend's transition to nx and pnpm:

Selection_2083

Background

The ComfyUI frontend has migrated to nx + pnpm and now strictly enforces pnpm usage via only-allow pnpm preinstall hook. The existing frontend PR feature (#309) fails on new frontend PRs because it tries to use npm install.

Changes Made

Core Logic (comfy_cli/command/install.py):

  • Enhanced verify_node_tools() to check for pnpm and auto-install when missing
  • Updated build commands: npm installpnpm install
  • Added user consent flow before installing pnpm globally
  • Maintained existing function signatures for backward compatibility

Testing (tests/comfy_cli/command/):

  • Updated existing tests for pnpm expectations
  • Added tests for auto-install success/failure scenarios
  • Added tests for user consent (accept/decline) flows

User Experience

When pnpm exists: ✅ Proceeds normally
When pnpm missing:

  1. Shows: "pnpm is not installed but is required for the modern frontend"
  2. Prompts: "Install pnpm automatically using npm? (This will run: npm install -g pnpm)"
  3. User accepts: Installs pnpm → continues building
  4. User declines: Shows manual instructions → exits gracefully

@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Sep 3, 2025
- Update verify_node_tools to automatically install pnpm when missing
- Change build commands from npm to pnpm for modern frontend compatibility
- Add user consent prompt before installing pnpm globally
- Update unit tests to cover pnpm verification and auto-install scenarios
- Maintain backward compatibility with existing function signatures

Addresses frontend transition to nx/pnpm build system while ensuring
robust user experience for QA, ops, and GTM teams.
@codecov
Copy link

codecov bot commented Sep 3, 2025

Codecov Report

❌ Patch coverage is 76.66667% with 7 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
comfy_cli/command/install.py 76.66% 7 Missing ⚠️
@@            Coverage Diff             @@
##             main     #314      +/-   ##
==========================================
+ Coverage   50.44%   50.63%   +0.18%     
==========================================
  Files          32       32              
  Lines        3380     3405      +25     
==========================================
+ Hits         1705     1724      +19     
- Misses       1675     1681       +6     
Files with missing lines Coverage Δ
comfy_cli/command/install.py 58.27% <76.66%> (+1.09%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dosubot dosubot bot added the enhancement New feature or request label Sep 3, 2025
@christian-byrne christian-byrne force-pushed the feat/update-frontend-pr-nx-pnpm branch from 02b6cc9 to 53306e8 Compare September 3, 2025 04:51
Copy link
Contributor

@bigcat88 bigcat88 left a comment

Choose a reason for hiding this comment

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

checked with pre-installed pnpm - everything works fine

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Sep 3, 2025
@christian-byrne christian-byrne merged commit 2244b89 into main Sep 3, 2025
14 checks passed
@christian-byrne christian-byrne deleted the feat/update-frontend-pr-nx-pnpm branch September 3, 2025 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request lgtm This PR has been approved by a maintainer size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants