diff --git a/.claude/commands/comprehensive-pr-review.md b/.claude/commands/comprehensive-pr-review.md new file mode 100644 index 000000000..a3310127c --- /dev/null +++ b/.claude/commands/comprehensive-pr-review.md @@ -0,0 +1,708 @@ +# Comprehensive PR Review for ComfyUI Frontend + + +You are performing a comprehensive code review for PR #$1 in the ComfyUI frontend repository. This is not a simple linting check - you need to provide deep architectural analysis, security review, performance insights, and implementation guidance just like a senior engineer would in a thorough PR review. + +Your review should cover: +1. Architecture and design patterns +2. Security vulnerabilities and risks +3. Performance implications +4. Code quality and maintainability +5. Integration with existing systems +6. Best practices and conventions +7. Testing considerations +8. Documentation needs + + +Arguments: PR number passed via PR_NUMBER environment variable + +## Phase 0: Initialize Variables and Helper Functions + +```bash +# Validate PR_NUMBER first thing +if [ -z "$PR_NUMBER" ]; then + echo "Error: PR_NUMBER environment variable is not set" + echo "Usage: PR_NUMBER= claude run /comprehensive-pr-review" + exit 1 +fi + +# Initialize all counters at the start +CRITICAL_COUNT=0 +HIGH_COUNT=0 +MEDIUM_COUNT=0 +LOW_COUNT=0 +ARCHITECTURE_ISSUES=0 +SECURITY_ISSUES=0 +PERFORMANCE_ISSUES=0 +QUALITY_ISSUES=0 + +# Helper function for posting review comments +post_review_comment() { + local file_path=$1 + local line_number=$2 + local severity=$3 # critical/high/medium/low + local category=$4 # architecture/security/performance/quality + local issue=$5 + local context=$6 + local suggestion=$7 + + # Update counters + case $severity in + "critical") ((CRITICAL_COUNT++)) ;; + "high") ((HIGH_COUNT++)) ;; + "medium") ((MEDIUM_COUNT++)) ;; + "low") ((LOW_COUNT++)) ;; + esac + + case $category in + "architecture") ((ARCHITECTURE_ISSUES++)) ;; + "security") ((SECURITY_ISSUES++)) ;; + "performance") ((PERFORMANCE_ISSUES++)) ;; + "quality") ((QUALITY_ISSUES++)) ;; + esac + + # Post inline comment via GitHub CLI + local comment="${issue}\n${context}\n${suggestion}" + gh pr review $PR_NUMBER --comment --body "$comment" -F - <<< "$comment" +} +``` + +## Phase 1: Environment Setup and PR Context + +```bash +# Pre-flight checks +check_prerequisites() { + # Check gh CLI is available + if ! command -v gh &> /dev/null; then + echo "Error: gh CLI is not installed" + exit 1 + fi + + # In GitHub Actions, auth is handled via GITHUB_TOKEN + if [ -n "$GITHUB_ACTIONS" ] && [ -z "$GITHUB_TOKEN" ]; then + echo "Error: GITHUB_TOKEN is not set in GitHub Actions" + exit 1 + fi + + # Check if we're authenticated + if ! gh auth status &> /dev/null; then + echo "Error: Not authenticated with GitHub. Run 'gh auth login'" + exit 1 + fi + + # Set repository if not already set + if [ -z "$REPOSITORY" ]; then + REPOSITORY="Comfy-Org/ComfyUI_frontend" + fi + + # Check PR exists and is open + PR_STATE=$(gh pr view $PR_NUMBER --repo $REPOSITORY --json state -q .state 2>/dev/null || echo "NOT_FOUND") + if [ "$PR_STATE" = "NOT_FOUND" ]; then + echo "Error: PR #$PR_NUMBER not found in $REPOSITORY" + exit 1 + elif [ "$PR_STATE" != "OPEN" ]; then + echo "Error: PR #$PR_NUMBER is not open (state: $PR_STATE)" + exit 1 + fi + + # Check API rate limits + RATE_REMAINING=$(gh api /rate_limit --jq '.rate.remaining' 2>/dev/null || echo "5000") + if [ "$RATE_REMAINING" -lt 100 ]; then + echo "Warning: Low API rate limit: $RATE_REMAINING remaining" + if [ "$RATE_REMAINING" -lt 50 ]; then + echo "Error: Insufficient API rate limit for comprehensive review" + exit 1 + fi + fi + + echo "Pre-flight checks passed" +} + +# Run pre-flight checks +check_prerequisites + +echo "Starting comprehensive review of PR #$PR_NUMBER" + +# Fetch PR information with error handling +echo "Fetching PR information..." +if ! gh pr view $PR_NUMBER --repo $REPOSITORY --json files,title,body,additions,deletions,baseRefName,headRefName > pr_info.json; then + echo "Error: Failed to fetch PR information" + exit 1 +fi + +# Extract branch names +BASE_BRANCH=$(jq -r '.baseRefName' < pr_info.json) +HEAD_BRANCH=$(jq -r '.headRefName' < pr_info.json) + +# Checkout PR branch locally for better file inspection +echo "Checking out PR branch..." +git fetch origin "pull/$PR_NUMBER/head:pr-$PR_NUMBER" +git checkout "pr-$PR_NUMBER" + +# Get changed files using git locally (much faster) +git diff --name-only "origin/$BASE_BRANCH" > changed_files.txt + +# Get the diff using git locally +git diff "origin/$BASE_BRANCH" > pr_diff.txt + +# Get detailed file changes with line numbers +git diff --name-status "origin/$BASE_BRANCH" > file_changes.txt + +# For API compatibility, create a simplified pr_files.json +echo '[]' > pr_files.json +while IFS=$'\t' read -r status file; do + if [[ "$status" != "D" ]]; then # Skip deleted files + # Get the patch for this file + patch=$(git diff "origin/$BASE_BRANCH" -- "$file" | jq -Rs .) + additions=$(git diff --numstat "origin/$BASE_BRANCH" -- "$file" | awk '{print $1}') + deletions=$(git diff --numstat "origin/$BASE_BRANCH" -- "$file" | awk '{print $2}') + + jq --arg file "$file" \ + --arg patch "$patch" \ + --arg additions "$additions" \ + --arg deletions "$deletions" \ + '. += [{ + "filename": $file, + "patch": $patch, + "additions": ($additions | tonumber), + "deletions": ($deletions | tonumber) + }]' pr_files.json > pr_files.json.tmp + mv pr_files.json.tmp pr_files.json + fi +done < file_changes.txt + +# Setup caching directory +CACHE_DIR=".claude-review-cache" +mkdir -p "$CACHE_DIR" + +# Function to cache analysis results +cache_analysis() { + local file_path=$1 + local analysis_result=$2 + local file_hash=$(git hash-object "$file_path" 2>/dev/null || echo "no-hash") + + if [ "$file_hash" != "no-hash" ]; then + echo "$analysis_result" > "$CACHE_DIR/${file_hash}.cache" + fi +} + +# Function to get cached analysis +get_cached_analysis() { + local file_path=$1 + local file_hash=$(git hash-object "$file_path" 2>/dev/null || echo "no-hash") + + if [ "$file_hash" != "no-hash" ] && [ -f "$CACHE_DIR/${file_hash}.cache" ]; then + cat "$CACHE_DIR/${file_hash}.cache" + return 0 + fi + return 1 +} + +# Clean old cache entries (older than 7 days) +find "$CACHE_DIR" -name "*.cache" -mtime +7 -delete 2>/dev/null || true +``` + +## Phase 2: Load Comprehensive Knowledge Base + +```bash +# Don't create knowledge directory until we know we need it +KNOWLEDGE_FOUND=false + +# Use local cache for knowledge base to avoid repeated downloads +KNOWLEDGE_CACHE_DIR=".claude-knowledge-cache" +mkdir -p "$KNOWLEDGE_CACHE_DIR" + +# Option to use cloned prompt library for better performance +PROMPT_LIBRARY_PATH="../comfy-claude-prompt-library" +if [ -d "$PROMPT_LIBRARY_PATH" ]; then + echo "Using local prompt library at $PROMPT_LIBRARY_PATH" + USE_LOCAL_PROMPT_LIBRARY=true +else + echo "No local prompt library found, will use GitHub API" + USE_LOCAL_PROMPT_LIBRARY=false +fi + +# Function to fetch with cache +fetch_with_cache() { + local url=$1 + local output_file=$2 + local cache_file="$KNOWLEDGE_CACHE_DIR/$(echo "$url" | sed 's/[^a-zA-Z0-9]/_/g')" + + # Check if cached version exists and is less than 1 day old + if [ -f "$cache_file" ] && [ $(find "$cache_file" -mtime -1 2>/dev/null | wc -l) -gt 0 ]; then + # Create knowledge directory only when we actually have content + if [ "$KNOWLEDGE_FOUND" = "false" ]; then + mkdir -p review_knowledge + KNOWLEDGE_FOUND=true + fi + cp "$cache_file" "$output_file" + echo "Using cached version of $(basename "$output_file")" + return 0 + fi + + # Try to fetch fresh version + if curl -s -f "$url" > "$output_file.tmp"; then + # Create knowledge directory only when we actually have content + if [ "$KNOWLEDGE_FOUND" = "false" ]; then + mkdir -p review_knowledge + KNOWLEDGE_FOUND=true + fi + mv "$output_file.tmp" "$output_file" + cp "$output_file" "$cache_file" + echo "Downloaded fresh version of $(basename "$output_file")" + return 0 + else + # If fetch failed but we have a cache, use it + if [ -f "$cache_file" ]; then + if [ "$KNOWLEDGE_FOUND" = "false" ]; then + mkdir -p review_knowledge + KNOWLEDGE_FOUND=true + fi + cp "$cache_file" "$output_file" + echo "Using stale cache for $(basename "$output_file") (download failed)" + return 0 + fi + echo "Failed to load $(basename "$output_file")" + return 1 + fi +} + +# Load REPOSITORY_GUIDE.md for deep architectural understanding +echo "Loading ComfyUI Frontend repository guide..." +if [ "$USE_LOCAL_PROMPT_LIBRARY" = "true" ] && [ -f "$PROMPT_LIBRARY_PATH/project-summaries-for-agents/ComfyUI_frontend/REPOSITORY_GUIDE.md" ]; then + if [ "$KNOWLEDGE_FOUND" = "false" ]; then + mkdir -p review_knowledge + KNOWLEDGE_FOUND=true + fi + cp "$PROMPT_LIBRARY_PATH/project-summaries-for-agents/ComfyUI_frontend/REPOSITORY_GUIDE.md" "review_knowledge/repository_guide.md" + echo "Loaded repository guide from local prompt library" +else + fetch_with_cache "https://raw.githubusercontent.com/Comfy-Org/comfy-claude-prompt-library/master/project-summaries-for-agents/ComfyUI_frontend/REPOSITORY_GUIDE.md" "review_knowledge/repository_guide.md" +fi + +# 3. Discover and load relevant knowledge folders from GitHub API +echo "Discovering available knowledge folders..." +KNOWLEDGE_API_URL="https://api.github.com/repos/Comfy-Org/comfy-claude-prompt-library/contents/.claude/knowledge" +if KNOWLEDGE_FOLDERS=$(curl -s "$KNOWLEDGE_API_URL" | jq -r '.[] | select(.type=="dir") | .name' 2>/dev/null); then + echo "Available knowledge folders: $KNOWLEDGE_FOLDERS" + + # Analyze changed files to determine which knowledge folders might be relevant + CHANGED_FILES=$(cat changed_files.txt) + PR_TITLE=$(jq -r '.title' < pr_info.json) + PR_BODY=$(jq -r '.body // ""' < pr_info.json) + + # For each knowledge folder, check if it might be relevant to the PR + for folder in $KNOWLEDGE_FOLDERS; do + # Simple heuristic: if folder name appears in changed file paths or PR context + if echo "$CHANGED_FILES $PR_TITLE $PR_BODY" | grep -qi "$folder"; then + echo "Loading knowledge folder: $folder" + # Fetch all files in that knowledge folder + FOLDER_API_URL="https://api.github.com/repos/Comfy-Org/comfy-claude-prompt-library/contents/.claude/knowledge/$folder" + curl -s "$FOLDER_API_URL" | jq -r '.[] | select(.type=="file") | .download_url' 2>/dev/null | \ + while read url; do + if [ -n "$url" ]; then + filename=$(basename "$url") + fetch_with_cache "$url" "review_knowledge/${folder}_${filename}" + fi + done + fi + done +else + echo "Could not discover knowledge folders" +fi + +# 4. Load validation rules from the repository +echo "Loading validation rules..." +VALIDATION_API_URL="https://api.github.com/repos/Comfy-Org/comfy-claude-prompt-library/contents/.claude/commands/validation" +if VALIDATION_FILES=$(curl -s "$VALIDATION_API_URL" | jq -r '.[] | select(.name | contains("frontend") or contains("security") or contains("performance")) | .download_url' 2>/dev/null); then + for url in $VALIDATION_FILES; do + if [ -n "$url" ]; then + filename=$(basename "$url") + fetch_with_cache "$url" "review_knowledge/validation_${filename}" + fi + done +else + echo "Could not load validation rules" +fi + +# 5. Load local project guidelines +if [ -f "CLAUDE.md" ]; then + if [ "$KNOWLEDGE_FOUND" = "false" ]; then + mkdir -p review_knowledge + KNOWLEDGE_FOUND=true + fi + cp CLAUDE.md review_knowledge/local_claude.md +fi +if [ -f ".github/CLAUDE.md" ]; then + if [ "$KNOWLEDGE_FOUND" = "false" ]; then + mkdir -p review_knowledge + KNOWLEDGE_FOUND=true + fi + cp .github/CLAUDE.md review_knowledge/github_claude.md +fi +``` + +## Phase 3: Deep Analysis Instructions + +Perform a comprehensive analysis covering these areas: + +### 3.1 Architectural Analysis +Based on the repository guide and project summary, evaluate: +- Does this change align with the established architecture patterns? +- Are domain boundaries respected? +- Is the extension system used appropriately? +- Are components properly organized by feature? +- Does it follow the established service/composable/store patterns? + +### 3.2 Code Quality Beyond Linting +- Cyclomatic complexity and cognitive load +- SOLID principles adherence +- DRY violations that aren't caught by simple duplication checks +- Proper abstraction levels +- Interface design and API clarity +- No leftover debug code (console.log, commented code, TODO comments) + +### 3.3 Library Usage Enforcement +CRITICAL: Never re-implement functionality that exists in our standard libraries: +- **Tailwind CSS**: Use utility classes instead of custom CSS or style attributes +- **PrimeVue**: Never re-implement components that exist in PrimeVue (buttons, modals, dropdowns, etc.) +- **VueUse**: Never re-implement composables that exist in VueUse (useLocalStorage, useDebounceFn, etc.) +- **Lodash**: Never re-implement utility functions (debounce, throttle, cloneDeep, etc.) +- **Common components**: Reuse components from src/components/common/ +- **DOMPurify**: Always use for HTML sanitization +- **Fuse.js**: Use for fuzzy search functionality +- **Marked**: Use for markdown parsing +- **Pinia**: Use for global state management, not custom solutions +- **Zod**: Use for form validation with zodResolver pattern +- **Tiptap**: Use for rich text/markdown editing +- **Xterm.js**: Use for terminal emulation +- **Axios**: Use for HTTP client initialization + +### 3.4 Security Deep Dive +Beyond obvious vulnerabilities: +- Authentication/authorization implications +- Data validation completeness +- State management security +- Cross-origin concerns +- Extension security boundaries + +### 3.5 Performance Analysis +- Render performance implications +- Layout thrashing prevention +- Memory leak potential +- Network request optimization +- State management efficiency + +### 3.6 Integration Concerns +- Breaking changes to internal APIs +- Extension compatibility +- Backward compatibility +- Migration requirements + +## Phase 4: Create Detailed Review Comments + +CRITICAL: Keep comments extremely concise and effective. Use only as many words as absolutely necessary. +- NO markdown formatting (no #, ##, ###, **, etc.) +- NO emojis +- Get to the point immediately +- Burden the reader as little as possible + +For each issue found, create a concise inline comment with: +1. What's wrong (one line) +2. Why it matters (one line) +3. How to fix it (one line) +4. Code example only if essential + +```bash +# Helper function for comprehensive comments +post_review_comment() { + local file_path=$1 + local line_number=$2 + local severity=$3 # critical/high/medium/low + local category=$4 # architecture/security/performance/quality + local issue=$5 + local context=$6 + local suggestion=$7 + local example=$8 + + local body="### [$category] $severity Priority + +**Issue**: $issue + +**Context**: $context + +**Suggestion**: $suggestion" + + if [ -n "$example" ]; then + body="$body + +**Example**: +\`\`\`typescript +$example +\`\`\`" + fi + + body="$body + +*Related: See [repository guide](https://github.com/Comfy-Org/comfy-claude-prompt-library/blob/master/project-summaries-for-agents/ComfyUI_frontend/REPOSITORY_GUIDE.md) for patterns*" + + gh api -X POST /repos/$REPOSITORY/pulls/$PR_NUMBER/comments \ + -f path="$file_path" \ + -f line=$line_number \ + -f body="$body" \ + -f commit_id="$COMMIT_SHA" \ + -f side='RIGHT' || echo "Failed to post comment at $file_path:$line_number" +} +``` + +## Phase 5: Validation Rules Application + +Apply ALL validation rules from the loaded knowledge, but focus on the changed lines: + +### From Frontend Standards +- Vue 3 Composition API patterns +- Component communication patterns +- Proper use of composables +- TypeScript strict mode compliance +- Bundle optimization + +### From Security Audit +- Input validation +- XSS prevention +- CSRF protection +- Secure state management +- API security + +### From Performance Check +- Render optimization +- Memory management +- Network efficiency +- Bundle size impact + +## Phase 6: Contextual Review Based on PR Type + +Analyze the PR description and changes to determine the type: + +```bash +# Extract PR metadata with error handling +if [ ! -f pr_info.json ]; then + echo "Error: pr_info.json not found" + exit 1 +fi + +PR_TITLE=$(jq -r '.title // "Unknown"' < pr_info.json) +PR_BODY=$(jq -r '.body // ""' < pr_info.json) +FILE_COUNT=$(wc -l < changed_files.txt) +ADDITIONS=$(jq -r '.additions // 0' < pr_info.json) +DELETIONS=$(jq -r '.deletions // 0' < pr_info.json) + +# Determine PR type and apply specific review criteria +if echo "$PR_TITLE $PR_BODY" | grep -qiE "(feature|feat)"; then + echo "Detected feature PR - applying feature review criteria" + # Check for tests, documentation, backward compatibility +elif echo "$PR_TITLE $PR_BODY" | grep -qiE "(fix|bug)"; then + echo "Detected bug fix - checking root cause and regression tests" + # Verify fix addresses root cause, includes tests +elif echo "$PR_TITLE $PR_BODY" | grep -qiE "(refactor)"; then + echo "Detected refactoring - ensuring behavior preservation" + # Check that tests still pass, no behavior changes +fi +``` + +## Phase 7: Generate Comprehensive Summary + +After all inline comments, create a detailed summary: + +```bash +# Initialize metrics tracking +REVIEW_START_TIME=$(date +%s) + +# Create the comprehensive summary +gh pr review $PR_NUMBER --comment --body "# Comprehensive PR Review + +This review is generated by Claude. It may not always be accurate, as with human reviewers. If you believe that any of the comments are invalid or incorrect, please state why for each. For others, please implement the changes in one way or another. + +## Review Summary + +**PR**: $PR_TITLE (#$PR_NUMBER) +**Impact**: $ADDITIONS additions, $DELETIONS deletions across $FILE_COUNT files + +### Issue Distribution +- Critical: $CRITICAL_COUNT +- High: $HIGH_COUNT +- Medium: $MEDIUM_COUNT +- Low: $LOW_COUNT + +### Category Breakdown +- Architecture: $ARCHITECTURE_ISSUES issues +- Security: $SECURITY_ISSUES issues +- Performance: $PERFORMANCE_ISSUES issues +- Code Quality: $QUALITY_ISSUES issues + +## Key Findings + +### Architecture & Design +[Detailed architectural analysis based on repository patterns] + +### Security Considerations +[Security implications beyond basic vulnerabilities] + +### Performance Impact +[Performance analysis including bundle size, render impact] + +### Integration Points +[How this affects other systems, extensions, etc.] + +## Positive Observations +[What was done well, good patterns followed] + +## References +- [Repository Architecture Guide](https://github.com/Comfy-Org/comfy-claude-prompt-library/blob/master/project-summaries-for-agents/ComfyUI_frontend/REPOSITORY_GUIDE.md) +- [Frontend Standards](https://github.com/Comfy-Org/comfy-claude-prompt-library/blob/master/.claude/commands/validation/frontend-code-standards.md) +- [Security Guidelines](https://github.com/Comfy-Org/comfy-claude-prompt-library/blob/master/.claude/commands/validation/security-audit.md) + +## Next Steps +1. Address critical issues before merge +2. Consider architectural feedback for long-term maintainability +3. Add tests for uncovered scenarios +4. Update documentation if needed + +--- +*This is a comprehensive automated review. For architectural decisions requiring human judgment, please request additional manual review.*" +``` + +## Important: Think Deeply + +When reviewing: +1. **Think hard** about architectural implications +2. Consider how changes affect the entire system +3. Look for subtle bugs and edge cases +4. Evaluate maintainability over time +5. Consider extension developer experience +6. Think about migration paths + +This is a COMPREHENSIVE review, not a linting pass. Provide the same quality feedback a senior engineer would give after careful consideration. + +## Phase 8: Track Review Metrics + +After completing the review, save metrics for analysis: + +```bash +# Calculate review duration +REVIEW_END_TIME=$(date +%s) +REVIEW_DURATION=$((REVIEW_END_TIME - REVIEW_START_TIME)) + +# Calculate total issues +TOTAL_ISSUES=$((CRITICAL_COUNT + HIGH_COUNT + MEDIUM_COUNT + LOW_COUNT)) + +# Create metrics directory if it doesn't exist +METRICS_DIR=".claude/review-metrics" +mkdir -p "$METRICS_DIR" + +# Generate metrics file +METRICS_FILE="$METRICS_DIR/metrics-$(date +%Y%m).json" + +# Create or update monthly metrics file +if [ -f "$METRICS_FILE" ]; then + # Append to existing file + jq -n \ + --arg pr "$PR_NUMBER" \ + --arg title "$PR_TITLE" \ + --arg timestamp "$(date -u +%Y-%m-%dT%H:%M:%SZ)" \ + --arg duration "$REVIEW_DURATION" \ + --arg files "$FILE_COUNT" \ + --arg additions "$ADDITIONS" \ + --arg deletions "$DELETIONS" \ + --arg total "$TOTAL_ISSUES" \ + --arg critical "$CRITICAL_COUNT" \ + --arg high "$HIGH_COUNT" \ + --arg medium "$MEDIUM_COUNT" \ + --arg low "$LOW_COUNT" \ + --arg architecture "$ARCHITECTURE_ISSUES" \ + --arg security "$SECURITY_ISSUES" \ + --arg performance "$PERFORMANCE_ISSUES" \ + --arg quality "$QUALITY_ISSUES" \ + '{ + pr_number: $pr, + pr_title: $title, + timestamp: $timestamp, + review_duration_seconds: ($duration | tonumber), + files_reviewed: ($files | tonumber), + lines_added: ($additions | tonumber), + lines_deleted: ($deletions | tonumber), + issues: { + total: ($total | tonumber), + by_severity: { + critical: ($critical | tonumber), + high: ($high | tonumber), + medium: ($medium | tonumber), + low: ($low | tonumber) + }, + by_category: { + architecture: ($architecture | tonumber), + security: ($security | tonumber), + performance: ($performance | tonumber), + quality: ($quality | tonumber) + } + } + }' > "$METRICS_FILE.new" + + # Merge with existing data + jq -s '.[0] + [.[1]]' "$METRICS_FILE" "$METRICS_FILE.new" > "$METRICS_FILE.tmp" + mv "$METRICS_FILE.tmp" "$METRICS_FILE" + rm "$METRICS_FILE.new" +else + # Create new file + jq -n \ + --arg pr "$PR_NUMBER" \ + --arg title "$PR_TITLE" \ + --arg timestamp "$(date -u +%Y-%m-%dT%H:%M:%SZ)" \ + --arg duration "$REVIEW_DURATION" \ + --arg files "$FILE_COUNT" \ + --arg additions "$ADDITIONS" \ + --arg deletions "$DELETIONS" \ + --arg total "$TOTAL_ISSUES" \ + --arg critical "$CRITICAL_COUNT" \ + --arg high "$HIGH_COUNT" \ + --arg medium "$MEDIUM_COUNT" \ + --arg low "$LOW_COUNT" \ + --arg architecture "$ARCHITECTURE_ISSUES" \ + --arg security "$SECURITY_ISSUES" \ + --arg performance "$PERFORMANCE_ISSUES" \ + --arg quality "$QUALITY_ISSUES" \ + '[{ + pr_number: $pr, + pr_title: $title, + timestamp: $timestamp, + review_duration_seconds: ($duration | tonumber), + files_reviewed: ($files | tonumber), + lines_added: ($additions | tonumber), + lines_deleted: ($deletions | tonumber), + issues: { + total: ($total | tonumber), + by_severity: { + critical: ($critical | tonumber), + high: ($high | tonumber), + medium: ($medium | tonumber), + low: ($low | tonumber) + }, + by_category: { + architecture: ($architecture | tonumber), + security: ($security | tonumber), + performance: ($performance | tonumber), + quality: ($quality | tonumber) + } + } + }]' > "$METRICS_FILE" +fi + +echo "Review metrics saved to $METRICS_FILE" +``` + +This creates monthly metrics files (e.g., `metrics-202407.json`) that track: +- Which PRs were reviewed +- How long reviews took +- Types and severity of issues found +- Trends over time + +You can later analyze these to see patterns and improve your development process. \ No newline at end of file diff --git a/.claude/commands/create-frontend-release.md b/.claude/commands/create-frontend-release.md new file mode 100644 index 000000000..0e0b0036d --- /dev/null +++ b/.claude/commands/create-frontend-release.md @@ -0,0 +1,625 @@ +# Create Frontend Release + +This command guides you through creating a comprehensive frontend release with semantic versioning analysis, automated change detection, security scanning, and multi-stage human verification. + + +Create a frontend release with version type: $ARGUMENTS + +Expected format: Version increment type and optional description +Examples: +- `patch` - Bug fixes only +- `minor` - New features, backward compatible +- `major` - Breaking changes +- `prerelease` - Alpha/beta/rc releases +- `patch "Critical security fixes"` - With custom description +- `minor --skip-changelog` - Skip automated changelog generation +- `minor --dry-run` - Simulate release without executing + +If no arguments provided, the command will always perform prerelease if the current version is prerelease, or patch in other cases. This command will never perform minor or major releases without explicit direction. + + +## Prerequisites + +Before starting, ensure: +- You have push access to the repository +- GitHub CLI (`gh`) is authenticated +- You're on a clean main branch working tree +- All intended changes are merged to main +- You understand the scope of changes being released + +## Critical Checks Before Starting + +### 1. Check Current Version Status +```bash +# Get current version and check if it's a pre-release +CURRENT_VERSION=$(node -p "require('./package.json').version") +if [[ "$CURRENT_VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+- ]]; then + echo "โš ๏ธ Current version $CURRENT_VERSION is a pre-release" + echo "Consider releasing stable (e.g., 1.24.0-1 โ†’ 1.24.0) first" +fi +``` + +### 2. Find Last Stable Release +```bash +# Get last stable release tag (no pre-release suffix) +LAST_STABLE=$(git tag -l "v*" | grep -v "\-" | sort -V | tail -1) +echo "Last stable release: $LAST_STABLE" +``` + +## Configuration Options + +**Environment Variables:** +- `RELEASE_SKIP_SECURITY_SCAN=true` - Skip security audit +- `RELEASE_AUTO_APPROVE=true` - Skip some confirmation prompts +- `RELEASE_DRY_RUN=true` - Simulate release without executing + +## Release Process + +### Step 1: Environment Safety Check + +1. Verify clean working directory: + ```bash + git status --porcelain + ``` +2. Confirm on main branch: + ```bash + git branch --show-current + ``` +3. Pull latest changes: + ```bash + git pull origin main + ``` +4. Check GitHub CLI authentication: + ```bash + gh auth status + ``` +5. Verify npm/PyPI publishing access (dry run) +6. **CONFIRMATION REQUIRED**: Environment ready for release? + +### Step 2: Analyze Recent Changes + +1. Get current version from package.json +2. **IMPORTANT**: Determine correct base for comparison: + ```bash + # If current version is pre-release, use last stable release + if [[ "$CURRENT_VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+- ]]; then + BASE_TAG=$LAST_STABLE + else + BASE_TAG=$(git describe --tags --abbrev=0) + fi + ``` +3. Find commits since base release (CRITICAL: use --first-parent): + ```bash + git log ${BASE_TAG}..HEAD --oneline --no-merges --first-parent + ``` +4. Count total commits: + ```bash + COMMIT_COUNT=$(git log ${BASE_TAG}..HEAD --oneline --no-merges --first-parent | wc -l) + echo "Found $COMMIT_COUNT commits since $BASE_TAG" + ``` +5. Analyze commits for: + - Breaking changes (BREAKING CHANGE, !, feat()) + - New features (feat:, feature:) + - Bug fixes (fix:, bugfix:) + - Documentation changes (docs:) + - Dependency updates +6. **VERIFY PR TARGET BRANCHES**: + ```bash + # Get merged PRs and verify they were merged to main + gh pr list --state merged --limit 50 --json number,title,baseRefName,mergedAt | \ + jq -r '.[] | select(.baseRefName == "main") | "\(.number): \(.title)"' + ``` +7. **HUMAN ANALYSIS**: Review change summary and verify scope + +### Step 3: Version Preview + +**Version Preview:** +- Current: `${CURRENT_VERSION}` +- Proposed: Show exact version number +- **CONFIRMATION REQUIRED**: Proceed with version `X.Y.Z`? + +### Step 4: Security and Dependency Audit + +1. Run security audit: + ```bash + npm audit --audit-level moderate + ``` +2. Check for known vulnerabilities in dependencies +3. Scan for hardcoded secrets or credentials: + ```bash + git log -p ${BASE_TAG}..HEAD | grep -iE "(password|key|secret|token)" || echo "No sensitive data found" + ``` +4. Verify no sensitive data in recent commits +5. **SECURITY REVIEW**: Address any critical findings before proceeding? + +### Step 5: Pre-Release Testing + +1. Run complete test suite: + ```bash + npm run test:unit + npm run test:component + npm run test:browser + ``` +2. Run type checking: + ```bash + npm run typecheck + ``` +3. Run linting (may have issues with missing packages): + ```bash + npm run lint || echo "Lint issues - verify if critical" + ``` +4. Test build process: + ```bash + npm run build + npm run build:types + ``` +5. **QUALITY GATE**: All tests and builds passing? + +### Step 6: Breaking Change Analysis + +1. Analyze API changes in: + - Public TypeScript interfaces + - Extension APIs + - Component props + - CLAUDE.md guidelines +2. Check for: + - Removed public functions/classes + - Changed function signatures + - Deprecated feature removals + - Configuration changes +3. Generate breaking change summary +4. **COMPATIBILITY REVIEW**: Breaking changes documented and justified? + +### Step 7: Generate and Save Changelog + +1. Extract commit messages since base release: + ```bash + git log ${BASE_TAG}..HEAD --oneline --no-merges --first-parent > commits.txt + ``` +2. **CRITICAL**: Verify PR inclusion by checking merge location: + ```bash + # For each significant PR mentioned, verify it's on main + for PR in ${SIGNIFICANT_PRS}; do + COMMIT=$(gh pr view $PR --json mergeCommit -q .mergeCommit.oid) + git branch -r --contains $COMMIT | grep -q "origin/main" || \ + echo "WARNING: PR #$PR not on main branch!" + done + ``` +3. Group by type: + - ๐Ÿš€ **Features** (feat:) + - ๐Ÿ› **Bug Fixes** (fix:) + - ๐Ÿ’ฅ **Breaking Changes** (BREAKING CHANGE) + - ๐Ÿ“š **Documentation** (docs:) + - ๐Ÿ”ง **Maintenance** (chore:, refactor:) + - โฌ†๏ธ **Dependencies** (deps:, dependency updates) +4. Include PR numbers and links +5. Add issue references (Fixes #123) +6. **Save changelog locally:** + ```bash + # Save to dated file for history + echo "$CHANGELOG" > release-notes-${NEW_VERSION}-$(date +%Y%m%d).md + + # Save to current for easy access + echo "$CHANGELOG" > CURRENT_RELEASE_NOTES.md + ``` +7. **CHANGELOG REVIEW**: Verify all PRs listed are actually on main branch + +### Step 8: Create Enhanced Release Notes + +1. Create comprehensive user-facing release notes including: + - **What's New**: Major features and improvements + - **Bug Fixes**: User-visible fixes + - **Breaking Changes**: Migration guide if applicable + - **Dependencies**: Major dependency updates + - **Performance**: Notable performance improvements + - **Contributors**: Thank contributors for their work +2. Reference related documentation updates +3. Include screenshots for UI changes (if available) +4. **Save release notes:** + ```bash + # Enhanced release notes for GitHub + echo "$RELEASE_NOTES" > github-release-notes-${NEW_VERSION}.md + ``` +5. **CONTENT REVIEW**: Release notes clear and helpful for users? + +### Step 9: Create Version Bump PR + +**For standard version bumps (patch/minor/major):** +```bash +# Trigger the workflow +gh workflow run version-bump.yaml -f version_type=${VERSION_TYPE} + +# Workflow runs quickly - usually creates PR within 30 seconds +echo "Workflow triggered. Waiting for PR creation..." +``` + +**For releasing a stable version:** +1. Must manually create branch and update version: + ```bash + git checkout -b version-bump-${NEW_VERSION} + # Edit package.json to remove pre-release suffix + git add package.json + git commit -m "${NEW_VERSION}" + git push origin version-bump-${NEW_VERSION} + ``` + +2. Wait for PR creation (if using workflow) or create manually: + ```bash + # For workflow-created PRs - wait and find it + sleep 30 + # Look for PR from comfy-pr-bot (not github-actions) + PR_NUMBER=$(gh pr list --author comfy-pr-bot --limit 1 --json number --jq '.[0].number') + + # Verify we got the PR + if [ -z "$PR_NUMBER" ]; then + echo "PR not found yet. Checking recent PRs..." + gh pr list --limit 5 --json number,title,author + fi + + # For manual PRs + gh pr create --title "${NEW_VERSION}" \ + --body-file enhanced-pr-description.md \ + --label "Release" + ``` +3. **Create enhanced PR description:** + ```bash + cat > enhanced-pr-description.md << EOF + # Release v${NEW_VERSION} + + ## Version Change + \`${CURRENT_VERSION}\` โ†’ \`${NEW_VERSION}\` (${VERSION_TYPE}) + + ## Changelog + ${CHANGELOG} + + ## Breaking Changes + ${BREAKING_CHANGES} + + ## Testing Performed + - โœ… Full test suite (unit, component, browser) + - โœ… TypeScript compilation + - โœ… Linting checks + - โœ… Build verification + - โœ… Security audit + + ## Distribution Channels + - GitHub Release (with dist.zip) + - PyPI Package (comfyui-frontend-package) + - npm Package (@comfyorg/comfyui-frontend-types) + + ## Post-Release Tasks + - [ ] Verify all distribution channels + - [ ] Update external documentation + - [ ] Monitor for issues + EOF + ``` +4. Update PR with enhanced description: + ```bash + gh pr edit ${PR_NUMBER} --body-file enhanced-pr-description.md + ``` +5. Add changelog as comment for easy reference: + ```bash + gh pr comment ${PR_NUMBER} --body-file CURRENT_RELEASE_NOTES.md + ``` +6. **PR REVIEW**: Version bump PR created and enhanced correctly? + +### Step 11: Critical Release PR Verification + +1. **CRITICAL**: Verify PR has "Release" label: + ```bash + gh pr view ${PR_NUMBER} --json labels | jq -r '.labels[].name' | grep -q "Release" || \ + echo "ERROR: Release label missing! Add it immediately!" + ``` +2. Check for update-locales commits: + ```bash + # WARNING: update-locales may add [skip ci] which blocks release workflow! + gh pr view ${PR_NUMBER} --json commits | grep -q "skip ci" && \ + echo "WARNING: [skip ci] detected - release workflow may not trigger!" + ``` +3. Verify version number in package.json +4. Review all changed files +5. Ensure no unintended changes included +6. Wait for required PR checks: + ```bash + gh pr checks ${PR_NUMBER} --watch + ``` +7. **FINAL CODE REVIEW**: Release label present and no [skip ci]? + +### Step 12: Pre-Merge Validation + +1. **Review Requirements**: Release PRs require approval +2. Monitor CI checks - watch for update-locales +3. **CRITICAL WARNING**: If update-locales adds [skip ci], the release workflow won't trigger! +4. Check no new commits to main since PR creation +5. **DEPLOYMENT READINESS**: Ready to merge? + +### Step 13: Execute Release + +1. **FINAL CONFIRMATION**: Merge PR to trigger release? +2. Merge the Release PR: + ```bash + gh pr merge ${PR_NUMBER} --merge + ``` +3. **IMMEDIATELY CHECK**: Did release workflow trigger? + ```bash + sleep 10 + gh run list --workflow=release.yaml --limit=1 + ``` +4. If workflow didn't trigger due to [skip ci]: + ```bash + echo "ERROR: Release workflow didn't trigger!" + echo "Options:" + echo "1. Create patch release (e.g., 1.24.1) to trigger workflow" + echo "2. Investigate manual release options" + ``` +5. If workflow triggered, monitor execution: + ```bash + WORKFLOW_RUN_ID=$(gh run list --workflow=release.yaml --limit=1 --json databaseId --jq '.[0].databaseId') + gh run watch ${WORKFLOW_RUN_ID} + ``` + +### Step 14: Enhance GitHub Release + +1. Wait for automatic release creation: + ```bash + # Wait for release to be created + while ! gh release view v${NEW_VERSION} >/dev/null 2>&1; do + echo "Waiting for release creation..." + sleep 10 + done + ``` + +2. **Enhance the GitHub release:** + ```bash + # Update release with our enhanced notes + gh release edit v${NEW_VERSION} \ + --title "๐Ÿš€ ComfyUI Frontend v${NEW_VERSION}" \ + --notes-file github-release-notes-${NEW_VERSION}.md \ + --latest + + # Add any additional assets if needed + # gh release upload v${NEW_VERSION} additional-assets.zip + ``` + +3. **Verify release details:** + ```bash + gh release view v${NEW_VERSION} + ``` + +### Step 15: Verify Multi-Channel Distribution + +1. **GitHub Release:** + ```bash + gh release view v${NEW_VERSION} --json assets,body,createdAt,tagName + ``` + - โœ… Check release notes + - โœ… Verify dist.zip attachment + - โœ… Confirm release marked as latest (for main branch) + +2. **PyPI Package:** + ```bash + # Check PyPI availability (may take a few minutes) + for i in {1..10}; do + if curl -s https://pypi.org/pypi/comfyui-frontend-package/json | jq -r '.releases | keys[]' | grep -q ${NEW_VERSION}; then + echo "โœ… PyPI package available" + break + fi + echo "โณ Waiting for PyPI package... (attempt $i/10)" + sleep 30 + done + ``` + +3. **npm Package:** + ```bash + # Check npm availability + for i in {1..10}; do + if npm view @comfyorg/comfyui-frontend-types@${NEW_VERSION} version >/dev/null 2>&1; then + echo "โœ… npm package available" + break + fi + echo "โณ Waiting for npm package... (attempt $i/10)" + sleep 30 + done + ``` + +4. **DISTRIBUTION VERIFICATION**: All channels published successfully? + +### Step 16: Post-Release Monitoring Setup + +1. **Monitor immediate release health:** + ```bash + # Check for immediate issues + gh issue list --label "bug" --state open --limit 5 --json title,number,createdAt + + # Monitor download metrics (if accessible) + gh release view v${NEW_VERSION} --json assets --jq '.assets[].downloadCount' + ``` + +2. **Update documentation tracking:** + ```bash + cat > post-release-checklist.md << EOF + # Post-Release Checklist for v${NEW_VERSION} + + ## Immediate Tasks (Next 24 hours) + - [ ] Monitor error rates and user feedback + - [ ] Watch for critical issues + - [ ] Verify documentation is up to date + - [ ] Check community channels for questions + + ## Short-term Tasks (Next week) + - [ ] Update external integration guides + - [ ] Monitor adoption metrics + - [ ] Gather user feedback + - [ ] Plan next release cycle + + ## Long-term Tasks + - [ ] Analyze release process improvements + - [ ] Update release templates based on learnings + - [ ] Document any new patterns discovered + + ## Key Metrics to Track + - Download counts: GitHub, PyPI, npm + - Issue reports related to v${NEW_VERSION} + - Community feedback and adoption + - Performance impact measurements + EOF + ``` + +3. **Create release summary:** + ```bash + cat > release-summary-${NEW_VERSION}.md << EOF + # Release Summary: ComfyUI Frontend v${NEW_VERSION} + + **Released:** $(date) + **Type:** ${VERSION_TYPE} + **Duration:** ~${RELEASE_DURATION} minutes + **Release Commit:** ${RELEASE_COMMIT} + + ## Metrics + - **Commits Included:** ${COMMITS_COUNT} + - **Contributors:** ${CONTRIBUTORS_COUNT} + - **Files Changed:** ${FILES_CHANGED} + - **Lines Added/Removed:** +${LINES_ADDED}/-${LINES_REMOVED} + + ## Distribution Status + - โœ… GitHub Release: Published + - โœ… PyPI Package: Available + - โœ… npm Types: Available + + ## Next Steps + - Monitor for 24-48 hours + - Address any critical issues immediately + - Plan next release cycle + + ## Files Generated + - \`release-notes-${NEW_VERSION}-$(date +%Y%m%d).md\` - Detailed changelog + - \`github-release-notes-${NEW_VERSION}.md\` - GitHub release notes + - \`post-release-checklist.md\` - Follow-up tasks + EOF + ``` + +4. **RELEASE COMPLETION**: All post-release setup completed? + +## Advanced Safety Features + +### Rollback Procedures + +**Pre-Merge Rollback:** +```bash +# Close version bump PR and reset +gh pr close ${PR_NUMBER} +git reset --hard origin/main +git clean -fd +``` + +**Post-Merge Rollback:** +```bash +# Create immediate patch release with reverts +git revert ${RELEASE_COMMIT} +# Follow this command again with patch version +``` + +**Emergency Procedures:** +```bash +# Document incident +cat > release-incident-${NEW_VERSION}.md << EOF +# Release Incident Report + +**Version:** ${NEW_VERSION} +**Issue:** [Describe the problem] +**Impact:** [Severity and scope] +**Resolution:** [Steps taken] +**Prevention:** [Future improvements] +EOF + +# Contact package registries for critical issues +echo "For critical security issues, consider:" +echo "- PyPI: Contact support for package yanking" +echo "- npm: Use 'npm unpublish' within 72 hours" +echo "- GitHub: Update release with warning notes" +``` + +### Quality Gates Summary + +The command implements multiple quality gates: + +1. **๐Ÿ”’ Security Gate**: Vulnerability scanning, secret detection +2. **๐Ÿงช Quality Gate**: Full test suite, linting, type checking +3. **๐Ÿ“‹ Content Gate**: Changelog accuracy, release notes quality +4. **๐Ÿ”„ Process Gate**: Release timing verification +5. **โœ… Verification Gate**: Multi-channel publishing confirmation +6. **๐Ÿ“Š Monitoring Gate**: Post-release health tracking + +## Common Scenarios + +### Scenario 1: Regular Feature Release +```bash +/project:create-frontend-release minor +``` +- Analyzes features since last release +- Generates changelog automatically +- Creates comprehensive release notes + +### Scenario 2: Critical Security Patch +```bash +/project:create-frontend-release patch "Security fixes for CVE-2024-XXXX" +``` +- Expedited security scanning +- Enhanced monitoring setup + +### Scenario 3: Major Version with Breaking Changes +```bash +/project:create-frontend-release major +``` +- Comprehensive breaking change analysis +- Migration guide generation + +### Scenario 4: Pre-release Testing +```bash +/project:create-frontend-release prerelease +``` +- Creates alpha/beta/rc versions +- Draft release status +- Python package specs require that prereleases use alpha/beta/rc as the preid + +## Common Issues and Solutions + +### Issue: Pre-release Version Confusion +**Problem**: Not sure whether to promote pre-release or create new version +**Solution**: +- Follow semver standards: a prerelease version is followed by a normal release. It should have the same major, minor, and patch versions as the prerelease. + +### Issue: Wrong Commit Count +**Problem**: Changelog includes commits from other branches +**Solution**: Always use `--first-parent` flag with git log + +**Update**: Sometimes update-locales doesn't add [skip ci] - always verify! + +### Issue: Missing PRs in Changelog +**Problem**: PR was merged to different branch +**Solution**: Verify PR merge target with: +```bash +gh pr view ${PR_NUMBER} --json baseRefName +``` + +### Issue: Release Failed Due to [skip ci] +**Problem**: Release workflow didn't trigger after merge +**Prevention**: Always avoid this scenario +- Ensure that `[skip ci]` or similar flags are NOT in the `HEAD` commit message of the PR + - Push a new, empty commit to the PR +- Always double-check this immediately before merging + +**Recovery Strategy**: +1. Revert version in a new PR (e.g., 1.24.0 โ†’ 1.24.0-1) +2. Merge the revert PR +3. Run version bump workflow again +4. This creates a fresh PR without [skip ci] +Benefits: Cleaner than creating extra version numbers + +## Key Learnings & Notes + +1. **PR Author**: Version bump PRs are created by `comfy-pr-bot`, not `github-actions` +2. **Workflow Speed**: Version bump workflow typically completes in ~20-30 seconds +3. **Update-locales Behavior**: Inconsistent - sometimes adds [skip ci], sometimes doesn't +4. **Recovery Options**: Reverting version is cleaner than creating extra versions + diff --git a/.claude/commands/create-hotfix-release.md b/.claude/commands/create-hotfix-release.md new file mode 100644 index 000000000..b1e521a29 --- /dev/null +++ b/.claude/commands/create-hotfix-release.md @@ -0,0 +1,222 @@ +# Create Hotfix Release + +This command guides you through creating a patch/hotfix release for ComfyUI Frontend with comprehensive safety checks and human confirmations at each step. + + +Create a hotfix release by cherry-picking commits or PR commits from main to a core branch: $ARGUMENTS + +Expected format: Comma-separated list of commits or PR numbers +Examples: +- `abc123,def456,ghi789` (commits) +- `#1234,#5678` (PRs) +- `abc123,#1234,def456` (mixed) + +If no arguments provided, the command will help identify the correct core branch and guide you through selecting commits/PRs. + + +## Prerequisites + +Before starting, ensure: +- You have push access to the repository +- GitHub CLI (`gh`) is authenticated +- You're on a clean working tree +- You understand the commits/PRs you're cherry-picking + +## Hotfix Release Process + +### Step 1: Identify Target Core Branch + +1. Fetch the current ComfyUI requirements.txt from master branch: + ```bash + curl -s https://raw.githubusercontent.com/comfyanonymous/ComfyUI/master/requirements.txt | grep "comfyui-frontend-package" + ``` +2. Extract the `comfyui-frontend-package` version (e.g., `comfyui-frontend-package==1.23.4`) +3. Parse version to get major.minor (e.g., `1.23.4` โ†’ `1.23`) +4. Determine core branch: `core/.` (e.g., `core/1.23`) +5. Verify the core branch exists: `git ls-remote origin refs/heads/core/*` +6. **CONFIRMATION REQUIRED**: Is `core/X.Y` the correct target branch? + +### Step 2: Parse and Validate Arguments + +1. Parse the comma-separated list of commits/PRs +2. For each item: + - If starts with `#`: Treat as PR number + - Otherwise: Treat as commit hash +3. For PR numbers: + - Fetch PR details using `gh pr view ` + - Extract the merge commit if PR is merged + - If PR has multiple commits, list them all + - **CONFIRMATION REQUIRED**: Use merge commit or cherry-pick individual commits? +4. Validate all commit hashes exist in the repository + +### Step 3: Analyze Target Changes + +1. For each commit/PR to cherry-pick: + - Display commit hash, author, date + - Show PR title and number (if applicable) + - Display commit message + - Show files changed and diff statistics + - Check if already in core branch: `git branch --contains ` +2. Identify potential conflicts by checking changed files +3. **CONFIRMATION REQUIRED**: Proceed with these commits? + +### Step 4: Create Hotfix Branch + +1. Checkout the core branch (e.g., `core/1.23`) +2. Pull latest changes: `git pull origin core/X.Y` +3. Display current version from package.json +4. Create hotfix branch: `hotfix/-` + - Example: `hotfix/1.23.4-20241120` +5. **CONFIRMATION REQUIRED**: Created branch correctly? + +### Step 5: Cherry-pick Changes + +For each commit: +1. Attempt cherry-pick: `git cherry-pick ` +2. If conflicts occur: + - Display conflict details + - Show conflicting sections + - Provide resolution guidance + - **CONFIRMATION REQUIRED**: Conflicts resolved correctly? +3. After successful cherry-pick: + - Show the changes: `git show HEAD` + - Run validation: `npm run typecheck && npm run lint` +4. **CONFIRMATION REQUIRED**: Cherry-pick successful and valid? + +### Step 6: Create PR to Core Branch + +1. Push the hotfix branch: `git push origin hotfix/-` +2. Create PR using gh CLI: + ```bash + gh pr create --base core/X.Y --head hotfix/- \ + --title "[Hotfix] Cherry-pick fixes to core/X.Y" \ + --body "Cherry-picked commits: ..." + ``` +3. Add appropriate labels (but NOT "Release" yet) +4. PR body should include: + - List of cherry-picked commits/PRs + - Original issue references + - Testing instructions + - Impact assessment +5. **CONFIRMATION REQUIRED**: PR created correctly? + +### Step 7: Wait for Tests + +1. Monitor PR checks: `gh pr checks` +2. Display test results as they complete +3. If any tests fail: + - Show failure details + - Analyze if related to cherry-picks + - **DECISION REQUIRED**: Fix and continue, or abort? +4. Wait for all required checks to pass +5. **CONFIRMATION REQUIRED**: All tests passing? + +### Step 8: Merge Hotfix PR + +1. Verify all checks have passed +2. Check for required approvals +3. Merge the PR: `gh pr merge --merge` +4. Delete the hotfix branch +5. **CONFIRMATION REQUIRED**: PR merged successfully? + +### Step 9: Create Version Bump + +1. Checkout the core branch: `git checkout core/X.Y` +2. Pull latest changes: `git pull origin core/X.Y` +3. Read current version from package.json +4. Determine patch version increment: + - Current: `1.23.4` โ†’ New: `1.23.5` +5. Create release branch named with new version: `release/1.23.5` +6. Update version in package.json to `1.23.5` +7. Commit: `git commit -m "[release] Bump version to 1.23.5"` +8. **CONFIRMATION REQUIRED**: Version bump correct? + +### Step 10: Create Release PR + +1. Push release branch: `git push origin release/1.23.5` +2. Create PR with Release label: + ```bash + gh pr create --base core/X.Y --head release/1.23.5 \ + --title "[Release] v1.23.5" \ + --body "..." \ + --label "Release" + ``` +3. **CRITICAL**: Verify "Release" label is added +4. PR description should include: + - Version: `1.23.4` โ†’ `1.23.5` + - Included fixes (link to previous PR) + - Release notes for users +5. **CONFIRMATION REQUIRED**: Release PR has "Release" label? + +### Step 11: Monitor Release Process + +1. Wait for PR checks to pass +2. **FINAL CONFIRMATION**: Ready to trigger release by merging? +3. Merge the PR: `gh pr merge --merge` +4. Monitor release workflow: + ```bash + gh run list --workflow=release.yaml --limit=1 + gh run watch + ``` +5. Track progress: + - GitHub release draft/publication + - PyPI upload + - npm types publication + +### Step 12: Post-Release Verification + +1. Verify GitHub release: + ```bash + gh release view v1.23.5 + ``` +2. Check PyPI package: + ```bash + pip index versions comfyui-frontend-package | grep 1.23.5 + ``` +3. Verify npm package: + ```bash + npm view @comfyorg/comfyui-frontend-types@1.23.5 + ``` +4. Generate release summary with: + - Version released + - Commits included + - Issues fixed + - Distribution status +5. **CONFIRMATION REQUIRED**: Release completed successfully? + +## Safety Checks + +Throughout the process: +- Always verify core branch matches ComfyUI's requirements.txt +- For PRs: Ensure using correct commits (merge vs individual) +- Check version numbers follow semantic versioning +- **Critical**: "Release" label must be on version bump PR +- Validate cherry-picks don't break core branch stability +- Keep audit trail of all operations + +## Rollback Procedures + +If something goes wrong: +- Before push: `git reset --hard origin/core/X.Y` +- After PR creation: Close PR and start over +- After failed release: Create new patch version with fixes +- Document any issues for future reference + +## Important Notes + +- Core branch version will be behind main - this is expected +- The "Release" label triggers the PyPI/npm publication +- PR numbers must include the `#` prefix +- Mixed commits/PRs are supported but review carefully +- Always wait for full test suite before proceeding + +## Expected Timeline + +- Step 1-3: ~10 minutes (analysis) +- Steps 4-6: ~15-30 minutes (cherry-picking) +- Step 7: ~10-20 minutes (tests) +- Steps 8-10: ~10 minutes (version bump) +- Step 11-12: ~15-20 minutes (release) +- Total: ~60-90 minutes + +This process ensures a safe, verified hotfix release with multiple confirmation points and clear tracking of what changes are being released. \ No newline at end of file diff --git a/.github/CLAUDE.md b/.github/CLAUDE.md new file mode 100644 index 000000000..9a95d8cd0 --- /dev/null +++ b/.github/CLAUDE.md @@ -0,0 +1,36 @@ +# ComfyUI Frontend - Claude Review Context + +This file provides additional context for the automated PR review system. + +## Quick Reference + +### PrimeVue Component Migrations + +When reviewing, flag these deprecated components: +- `Dropdown` โ†’ Use `Select` from 'primevue/select' +- `OverlayPanel` โ†’ Use `Popover` from 'primevue/popover' +- `Calendar` โ†’ Use `DatePicker` from 'primevue/datepicker' +- `InputSwitch` โ†’ Use `ToggleSwitch` from 'primevue/toggleswitch' +- `Sidebar` โ†’ Use `Drawer` from 'primevue/drawer' +- `Chips` โ†’ Use `AutoComplete` with multiple enabled and typeahead disabled +- `TabMenu` โ†’ Use `Tabs` without panels +- `Steps` โ†’ Use `Stepper` without panels +- `InlineMessage` โ†’ Use `Message` component + +### API Utilities Reference + +- `api.apiURL()` - Backend API calls (/prompt, /queue, /view, etc.) +- `api.fileURL()` - Static file access (templates, extensions) +- `$t()` / `i18n.global.t()` - Internationalization +- `DOMPurify.sanitize()` - HTML sanitization + +## Review Scope + +This automated review performs comprehensive analysis including: +- Architecture and design patterns +- Security vulnerabilities +- Performance implications +- Code quality and maintainability +- Integration concerns + +For implementation details, see `.claude/commands/comprehensive-pr-review.md`. \ No newline at end of file diff --git a/.github/workflows/claude-pr-review.yml b/.github/workflows/claude-pr-review.yml new file mode 100644 index 000000000..1b7f86934 --- /dev/null +++ b/.github/workflows/claude-pr-review.yml @@ -0,0 +1,75 @@ +name: Claude PR Review + +permissions: + contents: read + pull-requests: write + issues: write + +on: + pull_request: + types: [labeled] + +jobs: + wait-for-ci: + runs-on: ubuntu-latest + if: github.event.label.name == 'claude-review' + outputs: + should-proceed: ${{ steps.check-status.outputs.proceed }} + steps: + - name: Wait for other CI checks + uses: lewagon/wait-on-check-action@v1.3.1 + with: + ref: ${{ github.event.pull_request.head.sha }} + check-regexp: '^(ESLint|Prettier Check|Tests CI|Vitest Tests)' + wait-interval: 30 + repo-token: ${{ secrets.GITHUB_TOKEN }} + + - name: Check if we should proceed + id: check-status + run: | + # Get all check runs for this commit + CHECK_RUNS=$(gh api repos/${{ github.repository }}/commits/${{ github.event.pull_request.head.sha }}/check-runs --jq '.check_runs[] | select(.name | test("ESLint|Prettier Check|Tests CI|Vitest Tests")) | {name, conclusion}') + + # Check if any required checks failed + if echo "$CHECK_RUNS" | grep -q '"conclusion": "failure"'; then + echo "Some CI checks failed - skipping Claude review" + echo "proceed=false" >> $GITHUB_OUTPUT + else + echo "All CI checks passed - proceeding with Claude review" + echo "proceed=true" >> $GITHUB_OUTPUT + fi + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + + claude-review: + needs: wait-for-ci + if: needs.wait-for-ci.outputs.should-proceed == 'true' + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: '20' + + - name: Install dependencies for analysis tools + run: | + npm install -g typescript @vue/compiler-sfc + + - name: Run Claude PR Review + uses: anthropics/claude-code-action@main + with: + prompt_file: .claude/commands/comprehensive-pr-review.md + anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} + max_turns: 1 + timeout_minutes: 30 + env: + PR_NUMBER: ${{ github.event.pull_request.number }} + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + COMMIT_SHA: ${{ github.event.pull_request.head.sha }} + BASE_SHA: ${{ github.event.pull_request.base.sha }} + REPOSITORY: ${{ github.repository }} \ No newline at end of file diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml new file mode 100644 index 000000000..12ea6cfdb --- /dev/null +++ b/.github/workflows/pr-checks.yml @@ -0,0 +1,154 @@ +name: PR Checks +on: + pull_request: + types: [opened, edited, synchronize, reopened] + +permissions: + contents: read + pull-requests: read + +jobs: + analyze: + runs-on: ubuntu-latest + outputs: + should_run: ${{ steps.check-changes.outputs.should_run }} + has_browser_tests: ${{ steps.check-coverage.outputs.has_browser_tests }} + has_screen_recording: ${{ steps.check-recording.outputs.has_recording }} + steps: + - name: Checkout code + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Ensure base branch is available + run: | + # Fetch the specific base commit to ensure it's available for git diff + git fetch origin ${{ github.event.pull_request.base.sha }} + + - name: Check if significant changes exist + id: check-changes + run: | + # Get list of changed files + CHANGED_FILES=$(git diff --name-only ${{ github.event.pull_request.base.sha }}...${{ github.event.pull_request.head.sha }}) + + # Filter for src/ files + SRC_FILES=$(echo "$CHANGED_FILES" | grep '^src/' || true) + + if [ -z "$SRC_FILES" ]; then + echo "No src/ files changed" + echo "should_run=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + + # Count lines changed in src files + TOTAL_LINES=0 + for file in $SRC_FILES; do + if [ -f "$file" ]; then + # Count added lines (non-empty) + ADDED=$(git diff ${{ github.event.pull_request.base.sha }}...${{ github.event.pull_request.head.sha }} -- "$file" | grep '^+' | grep -v '^+++' | grep -v '^+$' | wc -l) + # Count removed lines (non-empty) + REMOVED=$(git diff ${{ github.event.pull_request.base.sha }}...${{ github.event.pull_request.head.sha }} -- "$file" | grep '^-' | grep -v '^---' | grep -v '^-$' | wc -l) + TOTAL_LINES=$((TOTAL_LINES + ADDED + REMOVED)) + fi + done + + echo "Total lines changed in src/: $TOTAL_LINES" + + if [ $TOTAL_LINES -gt 3 ]; then + echo "should_run=true" >> "$GITHUB_OUTPUT" + else + echo "should_run=false" >> "$GITHUB_OUTPUT" + fi + + - name: Check browser test coverage + id: check-coverage + if: steps.check-changes.outputs.should_run == 'true' + run: | + # Check if browser tests were updated + BROWSER_TEST_CHANGES=$(git diff --name-only ${{ github.event.pull_request.base.sha }}...${{ github.event.pull_request.head.sha }} | grep '^browser_tests/.*\.ts$' || true) + + if [ -n "$BROWSER_TEST_CHANGES" ]; then + echo "has_browser_tests=true" >> "$GITHUB_OUTPUT" + else + echo "has_browser_tests=false" >> "$GITHUB_OUTPUT" + fi + + - name: Check for screen recording + id: check-recording + if: steps.check-changes.outputs.should_run == 'true' + run: | + # Check PR body for screen recording + PR_BODY="${{ github.event.pull_request.body }}" + + # Check for GitHub user attachments or YouTube links + if echo "$PR_BODY" | grep -qiE 'github\.com/user-attachments/assets/[a-f0-9-]+|youtube\.com/watch|youtu\.be/'; then + echo "has_recording=true" >> "$GITHUB_OUTPUT" + else + echo "has_recording=false" >> "$GITHUB_OUTPUT" + fi + + - name: Final check and create results + id: final-check + if: always() + run: | + # Initialize results + WARNINGS_JSON="" + + # Only run checks if should_run is true + if [ "${{ steps.check-changes.outputs.should_run }}" == "true" ]; then + # Check browser test coverage + if [ "${{ steps.check-coverage.outputs.has_browser_tests }}" != "true" ]; then + if [ -n "$WARNINGS_JSON" ]; then + WARNINGS_JSON="${WARNINGS_JSON}," + fi + WARNINGS_JSON="${WARNINGS_JSON}{\"message\":\"โš ๏ธ **Warning: E2E Test Coverage Missing**\\n\\nIf this PR modifies behavior that can be covered by browser-based E2E tests, those tests are required. PRs lacking applicable test coverage may not be reviewed until added. Please add or update browser tests to ensure code quality and prevent regressions.\"}" + fi + + # Check screen recording + if [ "${{ steps.check-recording.outputs.has_recording }}" != "true" ]; then + if [ -n "$WARNINGS_JSON" ]; then + WARNINGS_JSON="${WARNINGS_JSON}," + fi + WARNINGS_JSON="${WARNINGS_JSON}{\"message\":\"โš ๏ธ **Warning: Visual Documentation Missing**\\n\\nIf this PR changes user-facing behavior, visual proof (screen recording or screenshot) is required. PRs without applicable visual documentation may not be reviewed until provided.\\nYou can add it by:\\n\\n- GitHub: Drag & drop media directly into the PR description\\n\\n- YouTube: Include a link to a short demo\"}" + fi + fi + + # Create results JSON + if [ -n "$WARNINGS_JSON" ]; then + # Create JSON with warnings + cat > pr-check-results.json << EOF + { + "fails": [], + "warnings": [$WARNINGS_JSON], + "messages": [], + "markdowns": [] + } + EOF + echo "failed=false" >> "$GITHUB_OUTPUT" + else + # Create JSON with success + cat > pr-check-results.json << 'EOF' + { + "fails": [], + "warnings": [], + "messages": [], + "markdowns": [] + } + EOF + echo "failed=false" >> "$GITHUB_OUTPUT" + fi + + # Write PR metadata + echo "${{ github.event.pull_request.number }}" > pr-number.txt + echo "${{ github.event.pull_request.head.sha }}" > pr-sha.txt + + - name: Upload results + uses: actions/upload-artifact@v4 + if: always() + with: + name: pr-check-results-${{ github.run_id }} + path: | + pr-check-results.json + pr-number.txt + pr-sha.txt + retention-days: 1 \ No newline at end of file diff --git a/.github/workflows/pr-comment.yml b/.github/workflows/pr-comment.yml new file mode 100644 index 000000000..5ae8cf83a --- /dev/null +++ b/.github/workflows/pr-comment.yml @@ -0,0 +1,149 @@ +name: PR Comment +on: + workflow_run: + workflows: ["PR Checks"] + types: [completed] + +permissions: + pull-requests: write + issues: write + statuses: write + +jobs: + comment: + if: github.event.workflow_run.event == 'pull_request' + runs-on: ubuntu-latest + steps: + - name: Download artifacts + uses: actions/download-artifact@v4 + with: + name: pr-check-results-${{ github.event.workflow_run.id }} + path: /tmp/pr-artifacts + github-token: ${{ secrets.GITHUB_TOKEN }} + run-id: ${{ github.event.workflow_run.id }} + + - name: Post results + uses: actions/github-script@v7 + with: + script: | + const fs = require('fs'); + const path = require('path'); + + // Helper function to safely read files + function safeReadFile(filePath) { + try { + if (!fs.existsSync(filePath)) return null; + return fs.readFileSync(filePath, 'utf8').trim(); + } catch (e) { + console.error(`Error reading ${filePath}:`, e); + return null; + } + } + + // Read artifact files + const artifactDir = '/tmp/pr-artifacts'; + const prNumber = safeReadFile(path.join(artifactDir, 'pr-number.txt')); + const prSha = safeReadFile(path.join(artifactDir, 'pr-sha.txt')); + const resultsJson = safeReadFile(path.join(artifactDir, 'pr-check-results.json')); + + // Validate PR number + if (!prNumber || isNaN(parseInt(prNumber))) { + throw new Error('Invalid or missing PR number'); + } + + // Parse and validate results + let results; + try { + results = JSON.parse(resultsJson || '{}'); + } catch (e) { + console.error('Failed to parse check results:', e); + + // Post error comment + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: parseInt(prNumber), + body: `โš ๏ธ PR checks failed to complete properly. Error parsing results: ${e.message}` + }); + return; + } + + // Format check messages + const messages = []; + + if (results.fails && results.fails.length > 0) { + messages.push('### โŒ Failures\n' + results.fails.map(f => f.message).join('\n\n')); + } + + if (results.warnings && results.warnings.length > 0) { + messages.push('### โš ๏ธ Warnings\n' + results.warnings.map(w => w.message).join('\n\n')); + } + + if (results.messages && results.messages.length > 0) { + messages.push('### ๐Ÿ’ฌ Messages\n' + results.messages.map(m => m.message).join('\n\n')); + } + + if (results.markdowns && results.markdowns.length > 0) { + messages.push(...results.markdowns.map(m => m.message)); + } + + // Find existing bot comment + const comments = await github.rest.issues.listComments({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: parseInt(prNumber) + }); + + const botComment = comments.data.find(comment => + comment.user.type === 'Bot' && + comment.body.includes('') + ); + + // Post comment if there are any messages + if (messages.length > 0) { + const body = messages.join('\n\n'); + const commentBody = `\n${body}`; + + if (botComment) { + await github.rest.issues.updateComment({ + owner: context.repo.owner, + repo: context.repo.repo, + comment_id: botComment.id, + body: commentBody + }); + } else { + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: parseInt(prNumber), + body: commentBody + }); + } + } else { + // No messages - delete existing comment if present + if (botComment) { + await github.rest.issues.deleteComment({ + owner: context.repo.owner, + repo: context.repo.repo, + comment_id: botComment.id + }); + } + } + + // Set commit status based on failures + if (prSha) { + const hasFailures = results.fails && results.fails.length > 0; + const hasWarnings = results.warnings && results.warnings.length > 0; + await github.rest.repos.createCommitStatus({ + owner: context.repo.owner, + repo: context.repo.repo, + sha: prSha, + state: hasFailures ? 'failure' : 'success', + context: 'pr-checks', + description: hasFailures + ? `${results.fails.length} check(s) failed` + : hasWarnings + ? `${results.warnings.length} warning(s)` + : 'All checks passed' + }); + } \ No newline at end of file diff --git a/.i18nrc.cjs b/.i18nrc.cjs index 7b27108fe..01a04cd97 100644 --- a/.i18nrc.cjs +++ b/.i18nrc.cjs @@ -9,9 +9,10 @@ module.exports = defineConfig({ entry: 'src/locales/en', entryLocale: 'en', output: 'src/locales', - outputLocales: ['zh', 'ru', 'ja', 'ko', 'fr', 'es'], + outputLocales: ['zh', 'zh-TW', 'ru', 'ja', 'ko', 'fr', 'es'], reference: `Special names to keep untranslated: flux, photomaker, clip, vae, cfg, stable audio, stable cascade, stable zero, controlnet, lora, HiDream. 'latent' is the short form of 'latent space'. 'mask' is in the context of image processing. + Note: For Traditional Chinese (Taiwan), use Taiwan-specific terminology and traditional characters. ` }); diff --git a/README.md b/README.md index 0004bdecf..d490ae628 100644 --- a/README.md +++ b/README.md @@ -529,7 +529,7 @@ Have another idea? Drop into Discord or open an issue, and let's chat! ### Prerequisites & Technology Stack - **Required Software**: - - Node.js (v16 or later) and npm + - Node.js (v16 or later; v20/v22 strongly recommended) and npm - Git for version control - A running ComfyUI backend instance diff --git a/browser_tests/README.md b/browser_tests/README.md index 88bd865f8..1aeaa6e54 100644 --- a/browser_tests/README.md +++ b/browser_tests/README.md @@ -14,7 +14,7 @@ Clone to your `custom_nodes` dir _ComfyUI_devtools adds additional API endpoints and nodes to ComfyUI for browser testing._ ### Node.js & Playwright Prerequisites -Ensure you have Node.js v20 or later installed. Then, set up the Chromium test driver: +Ensure you have Node.js v20 or v22 installed. Then, set up the Chromium test driver: ```bash npx playwright install chromium --with-deps ``` diff --git a/browser_tests/assets/nested-subgraph.json b/browser_tests/assets/nested-subgraph.json new file mode 100644 index 000000000..d65e0c29c --- /dev/null +++ b/browser_tests/assets/nested-subgraph.json @@ -0,0 +1,716 @@ +{ + "id": "976d6e9a-927d-42db-abd4-96bfc0ecf8d9", + "revision": 0, + "last_node_id": 10, + "last_link_id": 11, + "nodes": [ + { + "id": 10, + "type": "8beb610f-ddd1-4489-ae0d-2f732a4042ae", + "pos": [ + 532, + 412.5 + ], + "size": [ + 140, + 46 + ], + "flags": {}, + "order": 0, + "mode": 0, + "inputs": [], + "outputs": [ + { + "name": "LATENT", + "type": "LATENT", + "links": [ + 10 + ] + }, + { + "name": "VAE", + "type": "VAE", + "links": [ + 11 + ] + } + ], + "title": "subgraph 2", + "properties": {}, + "widgets_values": [] + }, + { + "id": 8, + "type": "VAEDecode", + "pos": [ + 758.2109985351562, + 398.3681335449219 + ], + "size": [ + 210, + 46 + ], + "flags": {}, + "order": 1, + "mode": 0, + "inputs": [ + { + "name": "samples", + "type": "LATENT", + "link": 10 + }, + { + "name": "vae", + "type": "VAE", + "link": 11 + } + ], + "outputs": [ + { + "name": "IMAGE", + "type": "IMAGE", + "slot_index": 0, + "links": [ + 9 + ] + } + ], + "properties": { + "Node name for S&R": "VAEDecode" + }, + "widgets_values": [] + }, + { + "id": 9, + "type": "SaveImage", + "pos": [ + 1028.9615478515625, + 381.83746337890625 + ], + "size": [ + 210, + 270 + ], + "flags": {}, + "order": 2, + "mode": 0, + "inputs": [ + { + "name": "images", + "type": "IMAGE", + "link": 9 + } + ], + "outputs": [], + "properties": {}, + "widgets_values": [ + "ComfyUI" + ] + } + ], + "links": [ + [ + 9, + 8, + 0, + 9, + 0, + "IMAGE" + ], + [ + 10, + 10, + 0, + 8, + 0, + "LATENT" + ], + [ + 11, + 10, + 1, + 8, + 1, + "VAE" + ] + ], + "groups": [], + "definitions": { + "subgraphs": [ + { + "id": "8beb610f-ddd1-4489-ae0d-2f732a4042ae", + "version": 1, + "state": { + "lastGroupId": 0, + "lastNodeId": 10, + "lastLinkId": 14, + "lastRerouteId": 0 + }, + "revision": 0, + "config": {}, + "name": "subgraph 2", + "inputNode": { + "id": -10, + "bounding": [ + -154, + 415.5, + 120, + 40 + ] + }, + "outputNode": { + "id": -20, + "bounding": [ + 1238, + 395.5, + 120, + 80 + ] + }, + "inputs": [], + "outputs": [ + { + "id": "4d6c7e4e-971e-4f78-9218-9a604db53a4b", + "name": "LATENT", + "type": "LATENT", + "linkIds": [ + 7 + ], + "localized_name": "LATENT", + "pos": { + "0": 1258, + "1": 415.5 + } + }, + { + "id": "f8201d4f-7fc6-4a1b-b8c9-9f0716d9c09a", + "name": "VAE", + "type": "VAE", + "linkIds": [ + 14 + ], + "localized_name": "VAE", + "pos": { + "0": 1258, + "1": 435.5 + } + } + ], + "widgets": [], + "nodes": [ + { + "id": 6, + "type": "CLIPTextEncode", + "pos": [ + 415, + 186 + ], + "size": [ + 422.84503173828125, + 164.31304931640625 + ], + "flags": {}, + "order": 0, + "mode": 0, + "inputs": [ + { + "localized_name": "clip", + "name": "clip", + "type": "CLIP", + "link": 13 + } + ], + "outputs": [ + { + "localized_name": "CONDITIONING", + "name": "CONDITIONING", + "type": "CONDITIONING", + "slot_index": 0, + "links": [ + 4 + ] + } + ], + "properties": { + "Node name for S&R": "CLIPTextEncode" + }, + "widgets_values": [ + "beautiful scenery nature glass bottle landscape, , purple galaxy bottle," + ] + }, + { + "id": 3, + "type": "KSampler", + "pos": [ + 863, + 186 + ], + "size": [ + 315, + 262 + ], + "flags": {}, + "order": 2, + "mode": 0, + "inputs": [ + { + "localized_name": "model", + "name": "model", + "type": "MODEL", + "link": 12 + }, + { + "localized_name": "positive", + "name": "positive", + "type": "CONDITIONING", + "link": 4 + }, + { + "localized_name": "negative", + "name": "negative", + "type": "CONDITIONING", + "link": 10 + }, + { + "localized_name": "latent_image", + "name": "latent_image", + "type": "LATENT", + "link": 11 + } + ], + "outputs": [ + { + "localized_name": "LATENT", + "name": "LATENT", + "type": "LATENT", + "slot_index": 0, + "links": [ + 7 + ] + } + ], + "properties": { + "Node name for S&R": "KSampler" + }, + "widgets_values": [ + 32115495257102, + "randomize", + 20, + 8, + "euler", + "normal", + 1 + ] + }, + { + "id": 10, + "type": "dbe5763f-440b-47b4-82ac-454f1f98b0e3", + "pos": [ + 194.13900756835938, + 657.3333740234375 + ], + "size": [ + 140, + 106 + ], + "flags": {}, + "order": 1, + "mode": 0, + "inputs": [], + "outputs": [ + { + "localized_name": "CONDITIONING", + "name": "CONDITIONING", + "type": "CONDITIONING", + "links": [ + 10 + ] + }, + { + "localized_name": "LATENT", + "name": "LATENT", + "type": "LATENT", + "links": [ + 11 + ] + }, + { + "localized_name": "MODEL", + "name": "MODEL", + "type": "MODEL", + "links": [ + 12 + ] + }, + { + "localized_name": "CLIP", + "name": "CLIP", + "type": "CLIP", + "links": [ + 13 + ] + }, + { + "localized_name": "VAE", + "name": "VAE", + "type": "VAE", + "links": [ + 14 + ] + } + ], + "title": "subgraph 3", + "properties": {}, + "widgets_values": [] + } + ], + "groups": [], + "links": [ + { + "id": 4, + "origin_id": 6, + "origin_slot": 0, + "target_id": 3, + "target_slot": 1, + "type": "CONDITIONING" + }, + { + "id": 7, + "origin_id": 3, + "origin_slot": 0, + "target_id": -20, + "target_slot": 0, + "type": "LATENT" + }, + { + "id": 10, + "origin_id": 10, + "origin_slot": 0, + "target_id": 3, + "target_slot": 2, + "type": "CONDITIONING" + }, + { + "id": 11, + "origin_id": 10, + "origin_slot": 1, + "target_id": 3, + "target_slot": 3, + "type": "LATENT" + }, + { + "id": 12, + "origin_id": 10, + "origin_slot": 2, + "target_id": 3, + "target_slot": 0, + "type": "MODEL" + }, + { + "id": 13, + "origin_id": 10, + "origin_slot": 3, + "target_id": 6, + "target_slot": 0, + "type": "CLIP" + }, + { + "id": 14, + "origin_id": 10, + "origin_slot": 4, + "target_id": -20, + "target_slot": 1, + "type": "VAE" + } + ], + "extra": {} + }, + { + "id": "dbe5763f-440b-47b4-82ac-454f1f98b0e3", + "version": 1, + "state": { + "lastGroupId": 0, + "lastNodeId": 9, + "lastLinkId": 9, + "lastRerouteId": 0 + }, + "revision": 0, + "config": {}, + "name": "subgraph 3", + "inputNode": { + "id": -10, + "bounding": [ + -154, + 517, + 120, + 40 + ] + }, + "outputNode": { + "id": -20, + "bounding": [ + 898.2780151367188, + 467, + 128.6640625, + 140 + ] + }, + "inputs": [], + "outputs": [ + { + "id": "b4882169-329b-43f6-a373-81abfbdea55b", + "name": "CONDITIONING", + "type": "CONDITIONING", + "linkIds": [ + 6 + ], + "localized_name": "CONDITIONING", + "pos": { + "0": 918.2780151367188, + "1": 487 + } + }, + { + "id": "01f51f96-a741-428e-8772-9557ee50b609", + "name": "LATENT", + "type": "LATENT", + "linkIds": [ + 2 + ], + "localized_name": "LATENT", + "pos": { + "0": 918.2780151367188, + "1": 507 + } + }, + { + "id": "47fa906e-d80b-45c3-a596-211a0e59d4a1", + "name": "MODEL", + "type": "MODEL", + "linkIds": [ + 1 + ], + "localized_name": "MODEL", + "pos": { + "0": 918.2780151367188, + "1": 527 + } + }, + { + "id": "f03dccd7-10e8-4513-9994-15854a92d192", + "name": "CLIP", + "type": "CLIP", + "linkIds": [ + 3 + ], + "localized_name": "CLIP", + "pos": { + "0": 918.2780151367188, + "1": 547 + } + }, + { + "id": "a666877f-e34f-49bc-8a78-b26156656b83", + "name": "VAE", + "type": "VAE", + "linkIds": [ + 8 + ], + "localized_name": "VAE", + "pos": { + "0": 918.2780151367188, + "1": 567 + } + } + ], + "widgets": [], + "nodes": [ + { + "id": 7, + "type": "CLIPTextEncode", + "pos": [ + 413, + 389 + ], + "size": [ + 425.27801513671875, + 180.6060791015625 + ], + "flags": {}, + "order": 2, + "mode": 0, + "inputs": [ + { + "localized_name": "clip", + "name": "clip", + "type": "CLIP", + "link": 5 + } + ], + "outputs": [ + { + "localized_name": "CONDITIONING", + "name": "CONDITIONING", + "type": "CONDITIONING", + "slot_index": 0, + "links": [ + 6 + ] + } + ], + "properties": { + "Node name for S&R": "CLIPTextEncode" + }, + "widgets_values": [ + "text, watermark" + ] + }, + { + "id": 5, + "type": "EmptyLatentImage", + "pos": [ + 473, + 609 + ], + "size": [ + 315, + 106 + ], + "flags": {}, + "order": 0, + "mode": 0, + "inputs": [], + "outputs": [ + { + "localized_name": "LATENT", + "name": "LATENT", + "type": "LATENT", + "slot_index": 0, + "links": [ + 2 + ] + } + ], + "properties": { + "Node name for S&R": "EmptyLatentImage" + }, + "widgets_values": [ + 512, + 512, + 1 + ] + }, + { + "id": 4, + "type": "CheckpointLoaderSimple", + "pos": [ + 26, + 474 + ], + "size": [ + 315, + 98 + ], + "flags": {}, + "order": 1, + "mode": 0, + "inputs": [], + "outputs": [ + { + "localized_name": "MODEL", + "name": "MODEL", + "type": "MODEL", + "slot_index": 0, + "links": [ + 1 + ] + }, + { + "localized_name": "CLIP", + "name": "CLIP", + "type": "CLIP", + "slot_index": 1, + "links": [ + 3, + 5 + ] + }, + { + "localized_name": "VAE", + "name": "VAE", + "type": "VAE", + "slot_index": 2, + "links": [ + 8 + ] + } + ], + "properties": { + "Node name for S&R": "CheckpointLoaderSimple" + }, + "widgets_values": [ + "v1-5-pruned-emaonly-fp16.safetensors" + ] + } + ], + "groups": [], + "links": [ + { + "id": 5, + "origin_id": 4, + "origin_slot": 1, + "target_id": 7, + "target_slot": 0, + "type": "CLIP" + }, + { + "id": 6, + "origin_id": 7, + "origin_slot": 0, + "target_id": -20, + "target_slot": 0, + "type": "CONDITIONING" + }, + { + "id": 2, + "origin_id": 5, + "origin_slot": 0, + "target_id": -20, + "target_slot": 1, + "type": "LATENT" + }, + { + "id": 1, + "origin_id": 4, + "origin_slot": 0, + "target_id": -20, + "target_slot": 2, + "type": "MODEL" + }, + { + "id": 3, + "origin_id": 4, + "origin_slot": 1, + "target_id": -20, + "target_slot": 3, + "type": "CLIP" + }, + { + "id": 8, + "origin_id": 4, + "origin_slot": 2, + "target_id": -20, + "target_slot": 4, + "type": "VAE" + } + ], + "extra": {} + } + ] + }, + "config": {}, + "extra": { + "frontendVersion": "1.24.0-1" + }, + "version": 0.4 +} \ No newline at end of file diff --git a/browser_tests/tests/featureFlags.spec.ts b/browser_tests/tests/featureFlags.spec.ts new file mode 100644 index 000000000..73eb35f47 --- /dev/null +++ b/browser_tests/tests/featureFlags.spec.ts @@ -0,0 +1,382 @@ +import { expect } from '@playwright/test' + +import { comfyPageFixture as test } from '../fixtures/ComfyPage' + +test.describe('Feature Flags', () => { + test('Client and server exchange feature flags on connection', async ({ + comfyPage + }) => { + // Navigate to a new page to capture the initial WebSocket connection + const newPage = await comfyPage.page.context().newPage() + + // Set up monitoring before navigation + await newPage.addInitScript(() => { + // This runs before any page scripts + window.__capturedMessages = { + clientFeatureFlags: null, + serverFeatureFlags: null + } + + // Capture outgoing client messages + const originalSend = WebSocket.prototype.send + WebSocket.prototype.send = function (data) { + try { + const parsed = JSON.parse(data) + if (parsed.type === 'feature_flags') { + window.__capturedMessages.clientFeatureFlags = parsed + } + } catch (e) { + // Not JSON, ignore + } + return originalSend.call(this, data) + } + + // Monitor for server feature flags + const checkInterval = setInterval(() => { + if ( + window['app']?.api?.serverFeatureFlags && + Object.keys(window['app'].api.serverFeatureFlags).length > 0 + ) { + window.__capturedMessages.serverFeatureFlags = + window['app'].api.serverFeatureFlags + clearInterval(checkInterval) + } + }, 100) + + // Clear after 10 seconds + setTimeout(() => clearInterval(checkInterval), 10000) + }) + + // Navigate to the app + await newPage.goto(comfyPage.url) + + // Wait for both client and server feature flags + await newPage.waitForFunction( + () => + window.__capturedMessages.clientFeatureFlags !== null && + window.__capturedMessages.serverFeatureFlags !== null, + { timeout: 10000 } + ) + + // Get the captured messages + const messages = await newPage.evaluate(() => window.__capturedMessages) + + // Verify client sent feature flags + expect(messages.clientFeatureFlags).toBeTruthy() + expect(messages.clientFeatureFlags).toHaveProperty('type', 'feature_flags') + expect(messages.clientFeatureFlags).toHaveProperty('data') + expect(messages.clientFeatureFlags.data).toHaveProperty( + 'supports_preview_metadata' + ) + expect( + typeof messages.clientFeatureFlags.data.supports_preview_metadata + ).toBe('boolean') + + // Verify server sent feature flags back + expect(messages.serverFeatureFlags).toBeTruthy() + expect(messages.serverFeatureFlags).toHaveProperty( + 'supports_preview_metadata' + ) + expect(typeof messages.serverFeatureFlags.supports_preview_metadata).toBe( + 'boolean' + ) + expect(messages.serverFeatureFlags).toHaveProperty('max_upload_size') + expect(typeof messages.serverFeatureFlags.max_upload_size).toBe('number') + expect(Object.keys(messages.serverFeatureFlags).length).toBeGreaterThan(0) + + await newPage.close() + }) + + test('Server feature flags are received and accessible', async ({ + comfyPage + }) => { + // Wait for connection to establish + await comfyPage.page.waitForTimeout(1000) + + // Get the actual server feature flags from the backend + const serverFlags = await comfyPage.page.evaluate(() => { + return window['app'].api.serverFeatureFlags + }) + + // Verify we received real feature flags from the backend + expect(serverFlags).toBeTruthy() + expect(Object.keys(serverFlags).length).toBeGreaterThan(0) + + // The backend should send feature flags + expect(serverFlags).toHaveProperty('supports_preview_metadata') + expect(typeof serverFlags.supports_preview_metadata).toBe('boolean') + expect(serverFlags).toHaveProperty('max_upload_size') + expect(typeof serverFlags.max_upload_size).toBe('number') + }) + + test('serverSupportsFeature method works with real backend flags', async ({ + comfyPage + }) => { + // Wait for connection + await comfyPage.page.waitForTimeout(1000) + + // Test serverSupportsFeature with real backend flags + const supportsPreviewMetadata = await comfyPage.page.evaluate(() => { + return window['app'].api.serverSupportsFeature( + 'supports_preview_metadata' + ) + }) + // The method should return a boolean based on the backend's value + expect(typeof supportsPreviewMetadata).toBe('boolean') + + // Test non-existent feature - should always return false + const supportsNonExistent = await comfyPage.page.evaluate(() => { + return window['app'].api.serverSupportsFeature('non_existent_feature_xyz') + }) + expect(supportsNonExistent).toBe(false) + + // Test that the method only returns true for boolean true values + const testResults = await comfyPage.page.evaluate(() => { + // Temporarily modify serverFeatureFlags to test behavior + const original = window['app'].api.serverFeatureFlags + window['app'].api.serverFeatureFlags = { + bool_true: true, + bool_false: false, + string_value: 'yes', + number_value: 1, + null_value: null + } + + const results = { + bool_true: window['app'].api.serverSupportsFeature('bool_true'), + bool_false: window['app'].api.serverSupportsFeature('bool_false'), + string_value: window['app'].api.serverSupportsFeature('string_value'), + number_value: window['app'].api.serverSupportsFeature('number_value'), + null_value: window['app'].api.serverSupportsFeature('null_value') + } + + // Restore original + window['app'].api.serverFeatureFlags = original + return results + }) + + // serverSupportsFeature should only return true for boolean true values + expect(testResults.bool_true).toBe(true) + expect(testResults.bool_false).toBe(false) + expect(testResults.string_value).toBe(false) + expect(testResults.number_value).toBe(false) + expect(testResults.null_value).toBe(false) + }) + + test('getServerFeature method works with real backend data', async ({ + comfyPage + }) => { + // Wait for connection + await comfyPage.page.waitForTimeout(1000) + + // Test getServerFeature method + const previewMetadataValue = await comfyPage.page.evaluate(() => { + return window['app'].api.getServerFeature('supports_preview_metadata') + }) + expect(typeof previewMetadataValue).toBe('boolean') + + // Test getting max_upload_size + const maxUploadSize = await comfyPage.page.evaluate(() => { + return window['app'].api.getServerFeature('max_upload_size') + }) + expect(typeof maxUploadSize).toBe('number') + expect(maxUploadSize).toBeGreaterThan(0) + + // Test getServerFeature with default value for non-existent feature + const defaultValue = await comfyPage.page.evaluate(() => { + return window['app'].api.getServerFeature( + 'non_existent_feature_xyz', + 'default' + ) + }) + expect(defaultValue).toBe('default') + }) + + test('getServerFeatures returns all backend feature flags', async ({ + comfyPage + }) => { + // Wait for connection + await comfyPage.page.waitForTimeout(1000) + + // Test getServerFeatures returns all flags + const allFeatures = await comfyPage.page.evaluate(() => { + return window['app'].api.getServerFeatures() + }) + + expect(allFeatures).toBeTruthy() + expect(allFeatures).toHaveProperty('supports_preview_metadata') + expect(typeof allFeatures.supports_preview_metadata).toBe('boolean') + expect(allFeatures).toHaveProperty('max_upload_size') + expect(Object.keys(allFeatures).length).toBeGreaterThan(0) + }) + + test('Client feature flags are immutable', async ({ comfyPage }) => { + // Test that getClientFeatureFlags returns a copy + const immutabilityTest = await comfyPage.page.evaluate(() => { + const flags1 = window['app'].api.getClientFeatureFlags() + const flags2 = window['app'].api.getClientFeatureFlags() + + // Modify the first object + flags1.test_modification = true + + // Get flags again to check if original was modified + const flags3 = window['app'].api.getClientFeatureFlags() + + return { + areEqual: flags1 === flags2, + hasModification: flags3.test_modification !== undefined, + hasSupportsPreview: flags1.supports_preview_metadata !== undefined, + supportsPreviewValue: flags1.supports_preview_metadata + } + }) + + // Verify they are different objects (not the same reference) + expect(immutabilityTest.areEqual).toBe(false) + + // Verify modification didn't affect the original + expect(immutabilityTest.hasModification).toBe(false) + + // Verify the flags contain expected properties + expect(immutabilityTest.hasSupportsPreview).toBe(true) + expect(typeof immutabilityTest.supportsPreviewValue).toBe('boolean') // From clientFeatureFlags.json + }) + + test('Server features are immutable when accessed via getServerFeatures', async ({ + comfyPage + }) => { + // Wait for connection to establish + await comfyPage.page.waitForTimeout(1000) + + const immutabilityTest = await comfyPage.page.evaluate(() => { + // Get a copy of server features + const features1 = window['app'].api.getServerFeatures() + + // Try to modify it + features1.supports_preview_metadata = false + features1.new_feature = 'added' + + // Get another copy + const features2 = window['app'].api.getServerFeatures() + + return { + modifiedValue: features1.supports_preview_metadata, + originalValue: features2.supports_preview_metadata, + hasNewFeature: features2.new_feature !== undefined, + hasSupportsPreview: features2.supports_preview_metadata !== undefined + } + }) + + // The modification should only affect the copy + expect(immutabilityTest.modifiedValue).toBe(false) + expect(typeof immutabilityTest.originalValue).toBe('boolean') // Backend sends boolean for supports_preview_metadata + expect(immutabilityTest.hasNewFeature).toBe(false) + expect(immutabilityTest.hasSupportsPreview).toBe(true) + }) + + test('Feature flags are negotiated early in connection lifecycle', async ({ + comfyPage + }) => { + // This test verifies that feature flags are available early in the app lifecycle + // which is important for protocol negotiation + + // Create a new page to ensure clean state + const newPage = await comfyPage.page.context().newPage() + + // Set up monitoring before navigation + await newPage.addInitScript(() => { + // Track when various app components are ready + ;(window as any).__appReadiness = { + featureFlagsReceived: false, + apiInitialized: false, + appInitialized: false + } + + // Monitor when feature flags arrive by checking periodically + const checkFeatureFlags = setInterval(() => { + if ( + window['app']?.api?.serverFeatureFlags?.supports_preview_metadata !== + undefined + ) { + ;(window as any).__appReadiness.featureFlagsReceived = true + clearInterval(checkFeatureFlags) + } + }, 10) + + // Monitor API initialization + const checkApi = setInterval(() => { + if (window['app']?.api) { + ;(window as any).__appReadiness.apiInitialized = true + clearInterval(checkApi) + } + }, 10) + + // Monitor app initialization + const checkApp = setInterval(() => { + if (window['app']?.graph) { + ;(window as any).__appReadiness.appInitialized = true + clearInterval(checkApp) + } + }, 10) + + // Clean up after 10 seconds + setTimeout(() => { + clearInterval(checkFeatureFlags) + clearInterval(checkApi) + clearInterval(checkApp) + }, 10000) + }) + + // Navigate to the app + await newPage.goto(comfyPage.url) + + // Wait for feature flags to be received + await newPage.waitForFunction( + () => + window['app']?.api?.serverFeatureFlags?.supports_preview_metadata !== + undefined, + { + timeout: 10000 + } + ) + + // Get readiness state + const readiness = await newPage.evaluate(() => { + return { + ...(window as any).__appReadiness, + currentFlags: window['app'].api.serverFeatureFlags + } + }) + + // Verify feature flags are available + expect(readiness.currentFlags).toHaveProperty('supports_preview_metadata') + expect(typeof readiness.currentFlags.supports_preview_metadata).toBe( + 'boolean' + ) + expect(readiness.currentFlags).toHaveProperty('max_upload_size') + + // Verify feature flags were received (we detected them via polling) + expect(readiness.featureFlagsReceived).toBe(true) + + // Verify API was initialized (feature flags require API) + expect(readiness.apiInitialized).toBe(true) + + await newPage.close() + }) + + test('Backend /features endpoint returns feature flags', async ({ + comfyPage + }) => { + // Test the HTTP endpoint directly + const response = await comfyPage.page.request.get( + `${comfyPage.url}/api/features` + ) + expect(response.ok()).toBe(true) + + const features = await response.json() + expect(features).toBeTruthy() + expect(features).toHaveProperty('supports_preview_metadata') + expect(typeof features.supports_preview_metadata).toBe('boolean') + expect(features).toHaveProperty('max_upload_size') + expect(Object.keys(features).length).toBeGreaterThan(0) + }) +}) diff --git a/browser_tests/tests/primitiveNode.spec.ts-snapshots/primitive-node-chromium-linux.png b/browser_tests/tests/primitiveNode.spec.ts-snapshots/primitive-node-chromium-linux.png index f133bdf6f..9f713b4a1 100644 Binary files a/browser_tests/tests/primitiveNode.spec.ts-snapshots/primitive-node-chromium-linux.png and b/browser_tests/tests/primitiveNode.spec.ts-snapshots/primitive-node-chromium-linux.png differ diff --git a/browser_tests/tests/primitiveNode.spec.ts-snapshots/primitive-node-connected-chromium-linux.png b/browser_tests/tests/primitiveNode.spec.ts-snapshots/primitive-node-connected-chromium-linux.png index 2adb3ad8d..3d0809c96 100644 Binary files a/browser_tests/tests/primitiveNode.spec.ts-snapshots/primitive-node-connected-chromium-linux.png and b/browser_tests/tests/primitiveNode.spec.ts-snapshots/primitive-node-connected-chromium-linux.png differ diff --git a/browser_tests/tests/primitiveNode.spec.ts-snapshots/static-primitive-connected-chromium-linux.png b/browser_tests/tests/primitiveNode.spec.ts-snapshots/static-primitive-connected-chromium-linux.png index 0341dac37..588674402 100644 Binary files a/browser_tests/tests/primitiveNode.spec.ts-snapshots/static-primitive-connected-chromium-linux.png and b/browser_tests/tests/primitiveNode.spec.ts-snapshots/static-primitive-connected-chromium-linux.png differ diff --git a/browser_tests/tests/releaseNotifications.spec.ts b/browser_tests/tests/releaseNotifications.spec.ts index 5e5e58001..19d09327d 100644 --- a/browser_tests/tests/releaseNotifications.spec.ts +++ b/browser_tests/tests/releaseNotifications.spec.ts @@ -130,4 +130,239 @@ test.describe('Release Notifications', () => { whatsNewSection.locator('text=No recent releases') ).toBeVisible() }) + + test('should hide "What\'s New" section when notifications are disabled', async ({ + comfyPage + }) => { + // Disable version update notifications + await comfyPage.setSetting('Comfy.Notification.ShowVersionUpdates', false) + + // Mock release API with test data + await comfyPage.page.route('**/releases**', async (route) => { + const url = route.request().url() + if ( + url.includes('api.comfy.org') || + url.includes('stagingapi.comfy.org') + ) { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify([ + { + id: 1, + project: 'comfyui', + version: 'v0.3.44', + attention: 'high', + content: '## New Features\n\n- Added awesome feature', + published_at: new Date().toISOString() + } + ]) + }) + } else { + await route.continue() + } + }) + + await comfyPage.setup({ mockReleases: false }) + + // Open help center + const helpCenterButton = comfyPage.page.locator('.comfy-help-center-btn') + await helpCenterButton.waitFor({ state: 'visible' }) + await helpCenterButton.click() + + // Verify help center menu appears + const helpMenu = comfyPage.page.locator('.help-center-menu') + await expect(helpMenu).toBeVisible() + + // Verify "What's New?" section is hidden + const whatsNewSection = comfyPage.page.locator('.whats-new-section') + await expect(whatsNewSection).not.toBeVisible() + + // Should not show any popups or toasts + await expect(comfyPage.page.locator('.whats-new-popup')).not.toBeVisible() + await expect( + comfyPage.page.locator('.release-notification-toast') + ).not.toBeVisible() + }) + + test('should not make API calls when notifications are disabled', async ({ + comfyPage + }) => { + // Disable version update notifications + await comfyPage.setSetting('Comfy.Notification.ShowVersionUpdates', false) + + // Track API calls + let apiCallCount = 0 + await comfyPage.page.route('**/releases**', async (route) => { + const url = route.request().url() + if ( + url.includes('api.comfy.org') || + url.includes('stagingapi.comfy.org') + ) { + apiCallCount++ + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify([]) + }) + } else { + await route.continue() + } + }) + + await comfyPage.setup({ mockReleases: false }) + + // Wait a bit to ensure any potential API calls would have been made + await comfyPage.page.waitForTimeout(1000) + + // Verify no API calls were made + expect(apiCallCount).toBe(0) + }) + + test('should show "What\'s New" section when notifications are enabled', async ({ + comfyPage + }) => { + // Enable version update notifications (default behavior) + await comfyPage.setSetting('Comfy.Notification.ShowVersionUpdates', true) + + // Mock release API with test data + await comfyPage.page.route('**/releases**', async (route) => { + const url = route.request().url() + if ( + url.includes('api.comfy.org') || + url.includes('stagingapi.comfy.org') + ) { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify([ + { + id: 1, + project: 'comfyui', + version: 'v0.3.44', + attention: 'medium', + content: '## New Features\n\n- Added awesome feature', + published_at: new Date().toISOString() + } + ]) + }) + } else { + await route.continue() + } + }) + + await comfyPage.setup({ mockReleases: false }) + + // Open help center + const helpCenterButton = comfyPage.page.locator('.comfy-help-center-btn') + await helpCenterButton.waitFor({ state: 'visible' }) + await helpCenterButton.click() + + // Verify help center menu appears + const helpMenu = comfyPage.page.locator('.help-center-menu') + await expect(helpMenu).toBeVisible() + + // Verify "What's New?" section is visible + const whatsNewSection = comfyPage.page.locator('.whats-new-section') + await expect(whatsNewSection).toBeVisible() + + // Should show the release + await expect( + whatsNewSection.locator('text=Comfy v0.3.44 Release') + ).toBeVisible() + }) + + test('should toggle "What\'s New" section when setting changes', async ({ + comfyPage + }) => { + // Mock release API with test data + await comfyPage.page.route('**/releases**', async (route) => { + const url = route.request().url() + if ( + url.includes('api.comfy.org') || + url.includes('stagingapi.comfy.org') + ) { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify([ + { + id: 1, + project: 'comfyui', + version: 'v0.3.44', + attention: 'low', + content: '## Bug Fixes\n\n- Fixed minor issue', + published_at: new Date().toISOString() + } + ]) + }) + } else { + await route.continue() + } + }) + + // Start with notifications enabled + await comfyPage.setSetting('Comfy.Notification.ShowVersionUpdates', true) + await comfyPage.setup({ mockReleases: false }) + + // Open help center + const helpCenterButton = comfyPage.page.locator('.comfy-help-center-btn') + await helpCenterButton.waitFor({ state: 'visible' }) + await helpCenterButton.click() + + // Verify "What's New?" section is visible + const whatsNewSection = comfyPage.page.locator('.whats-new-section') + await expect(whatsNewSection).toBeVisible() + + // Close help center + await comfyPage.page.click('.help-center-backdrop') + + // Disable notifications + await comfyPage.setSetting('Comfy.Notification.ShowVersionUpdates', false) + + // Reopen help center + await helpCenterButton.click() + + // Verify "What's New?" section is now hidden + await expect(whatsNewSection).not.toBeVisible() + }) + + test('should handle edge case with empty releases and disabled notifications', async ({ + comfyPage + }) => { + // Disable notifications + await comfyPage.setSetting('Comfy.Notification.ShowVersionUpdates', false) + + // Mock empty releases + await comfyPage.page.route('**/releases**', async (route) => { + const url = route.request().url() + if ( + url.includes('api.comfy.org') || + url.includes('stagingapi.comfy.org') + ) { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify([]) + }) + } else { + await route.continue() + } + }) + + await comfyPage.setup({ mockReleases: false }) + + // Open help center + const helpCenterButton = comfyPage.page.locator('.comfy-help-center-btn') + await helpCenterButton.waitFor({ state: 'visible' }) + await helpCenterButton.click() + + // Verify help center still works + const helpMenu = comfyPage.page.locator('.help-center-menu') + await expect(helpMenu).toBeVisible() + + // Section should be hidden regardless of empty releases + const whatsNewSection = comfyPage.page.locator('.whats-new-section') + await expect(whatsNewSection).not.toBeVisible() + }) }) diff --git a/browser_tests/tests/subgraphBreadcrumb.spec.ts b/browser_tests/tests/subgraphBreadcrumb.spec.ts new file mode 100644 index 000000000..d0bcb3360 --- /dev/null +++ b/browser_tests/tests/subgraphBreadcrumb.spec.ts @@ -0,0 +1,82 @@ +import { expect } from '@playwright/test' + +import { comfyPageFixture as test } from '../fixtures/ComfyPage' + +test.describe('Subgraph Breadcrumb Title Sync', () => { + test.beforeEach(async ({ comfyPage }) => { + await comfyPage.setSetting('Comfy.UseNewMenu', 'Top') + }) + + test('Breadcrumb updates when subgraph node title is changed', async ({ + comfyPage + }) => { + // Load a workflow with subgraphs + await comfyPage.loadWorkflow('nested-subgraph') + await comfyPage.nextFrame() + + // Get the subgraph node by ID (node 10 is the subgraph) + const subgraphNode = await comfyPage.getNodeRefById('10') + + // Get node position and double-click on it to enter the subgraph + const nodePos = await subgraphNode.getPosition() + const nodeSize = await subgraphNode.getSize() + await comfyPage.canvas.dblclick({ + position: { + x: nodePos.x + nodeSize.width / 2, + y: nodePos.y + nodeSize.height / 2 + 10 + } + }) + await comfyPage.nextFrame() + + // Wait for breadcrumb to appear + await comfyPage.page.waitForSelector('.subgraph-breadcrumb', { + state: 'visible', + timeout: 20000 + }) + + // Get initial breadcrumb text + const breadcrumb = comfyPage.page.locator('.subgraph-breadcrumb') + const initialBreadcrumbText = await breadcrumb.textContent() + + // Go back to main graph + await comfyPage.page.keyboard.press('Escape') + + // Double-click on the title area of the subgraph node to edit + await comfyPage.canvas.dblclick({ + position: { + x: nodePos.x + nodeSize.width / 2, + y: nodePos.y - 10 // Title area is above the node body + }, + delay: 5 + }) + + // Wait for title editor to appear + await expect(comfyPage.page.locator('.node-title-editor')).toBeVisible() + + // Clear existing text and type new title + await comfyPage.page.keyboard.press('Control+a') + const newTitle = 'Updated Subgraph Title' + await comfyPage.page.keyboard.type(newTitle) + await comfyPage.page.keyboard.press('Enter') + + // Wait a frame for the update to complete + await comfyPage.nextFrame() + + // Enter the subgraph again + await comfyPage.canvas.dblclick({ + position: { + x: nodePos.x + nodeSize.width / 2, + y: nodePos.y + nodeSize.height / 2 + }, + delay: 5 + }) + + // Wait for breadcrumb + await comfyPage.page.waitForSelector('.subgraph-breadcrumb') + + // Check that breadcrumb now shows the new title + const updatedBreadcrumbText = await breadcrumb.textContent() + expect(updatedBreadcrumbText).toContain(newTitle) + expect(updatedBreadcrumbText).not.toBe(initialBreadcrumbText) + }) +}) diff --git a/browser_tests/tests/templates.spec.ts-snapshots/template-card-after-hover-chromium-linux.png b/browser_tests/tests/templates.spec.ts-snapshots/template-card-after-hover-chromium-linux.png index b767dbe20..b415de47d 100644 Binary files a/browser_tests/tests/templates.spec.ts-snapshots/template-card-after-hover-chromium-linux.png and b/browser_tests/tests/templates.spec.ts-snapshots/template-card-after-hover-chromium-linux.png differ diff --git a/browser_tests/tests/templates.spec.ts-snapshots/template-card-before-hover-chromium-linux.png b/browser_tests/tests/templates.spec.ts-snapshots/template-card-before-hover-chromium-linux.png index cfebed5da..67e118ac2 100644 Binary files a/browser_tests/tests/templates.spec.ts-snapshots/template-card-before-hover-chromium-linux.png and b/browser_tests/tests/templates.spec.ts-snapshots/template-card-before-hover-chromium-linux.png differ diff --git a/browser_tests/tests/templates.spec.ts-snapshots/template-grid-desktop-chromium-linux.png b/browser_tests/tests/templates.spec.ts-snapshots/template-grid-desktop-chromium-linux.png index a378f877a..cc07421fe 100644 Binary files a/browser_tests/tests/templates.spec.ts-snapshots/template-grid-desktop-chromium-linux.png and b/browser_tests/tests/templates.spec.ts-snapshots/template-grid-desktop-chromium-linux.png differ diff --git a/browser_tests/tests/templates.spec.ts-snapshots/template-grid-mobile-chromium-linux.png b/browser_tests/tests/templates.spec.ts-snapshots/template-grid-mobile-chromium-linux.png index 577965c23..6095d0bb7 100644 Binary files a/browser_tests/tests/templates.spec.ts-snapshots/template-grid-mobile-chromium-linux.png and b/browser_tests/tests/templates.spec.ts-snapshots/template-grid-mobile-chromium-linux.png differ diff --git a/browser_tests/tests/templates.spec.ts-snapshots/template-grid-tablet-chromium-linux.png b/browser_tests/tests/templates.spec.ts-snapshots/template-grid-tablet-chromium-linux.png index 2bf7298b2..7602d5588 100644 Binary files a/browser_tests/tests/templates.spec.ts-snapshots/template-grid-tablet-chromium-linux.png and b/browser_tests/tests/templates.spec.ts-snapshots/template-grid-tablet-chromium-linux.png differ diff --git a/browser_tests/tests/useSettingSearch.spec.ts b/browser_tests/tests/useSettingSearch.spec.ts new file mode 100644 index 000000000..69a40ced9 --- /dev/null +++ b/browser_tests/tests/useSettingSearch.spec.ts @@ -0,0 +1,289 @@ +import { expect } from '@playwright/test' + +import { comfyPageFixture as test } from '../fixtures/ComfyPage' + +test.describe('Settings Search functionality', () => { + test.beforeEach(async ({ comfyPage }) => { + // Register test settings to verify hidden/deprecated filtering + await comfyPage.page.evaluate(() => { + window['app'].registerExtension({ + name: 'TestSettingsExtension', + settings: [ + { + id: 'TestHiddenSetting', + name: 'Test Hidden Setting', + type: 'hidden', + defaultValue: 'hidden_value', + category: ['Test', 'Hidden'] + }, + { + id: 'TestDeprecatedSetting', + name: 'Test Deprecated Setting', + type: 'text', + defaultValue: 'deprecated_value', + deprecated: true, + category: ['Test', 'Deprecated'] + }, + { + id: 'TestVisibleSetting', + name: 'Test Visible Setting', + type: 'text', + defaultValue: 'visible_value', + category: ['Test', 'Visible'] + } + ] + }) + }) + }) + + test('can open settings dialog and use search box', async ({ comfyPage }) => { + // Open settings dialog + await comfyPage.page.keyboard.press('Control+,') + const settingsDialog = comfyPage.page.locator('.settings-container') + await expect(settingsDialog).toBeVisible() + + // Find the search box + const searchBox = comfyPage.page.locator('.settings-search-box input') + await expect(searchBox).toBeVisible() + + // Verify search box has the correct placeholder + await expect(searchBox).toHaveAttribute( + 'placeholder', + expect.stringContaining('Search') + ) + }) + + test('search box is functional and accepts input', async ({ comfyPage }) => { + // Open settings dialog + await comfyPage.page.keyboard.press('Control+,') + const settingsDialog = comfyPage.page.locator('.settings-container') + await expect(settingsDialog).toBeVisible() + + // Find and interact with the search box + const searchBox = comfyPage.page.locator('.settings-search-box input') + await searchBox.fill('Comfy') + + // Verify the input was accepted + await expect(searchBox).toHaveValue('Comfy') + }) + + test('search box clears properly', async ({ comfyPage }) => { + // Open settings dialog + await comfyPage.page.keyboard.press('Control+,') + const settingsDialog = comfyPage.page.locator('.settings-container') + await expect(settingsDialog).toBeVisible() + + // Find and interact with the search box + const searchBox = comfyPage.page.locator('.settings-search-box input') + await searchBox.fill('test') + await expect(searchBox).toHaveValue('test') + + // Clear the search box + await searchBox.clear() + await expect(searchBox).toHaveValue('') + }) + + test('settings categories are visible in sidebar', async ({ comfyPage }) => { + // Open settings dialog + await comfyPage.page.keyboard.press('Control+,') + const settingsDialog = comfyPage.page.locator('.settings-container') + await expect(settingsDialog).toBeVisible() + + // Check that the sidebar has categories + const categories = comfyPage.page.locator( + '.settings-sidebar .p-listbox-option' + ) + expect(await categories.count()).toBeGreaterThan(0) + + // Check that at least one category is visible + await expect(categories.first()).toBeVisible() + }) + + test('can select different categories in sidebar', async ({ comfyPage }) => { + // Open settings dialog + await comfyPage.page.keyboard.press('Control+,') + const settingsDialog = comfyPage.page.locator('.settings-container') + await expect(settingsDialog).toBeVisible() + + // Get categories and click on different ones + const categories = comfyPage.page.locator( + '.settings-sidebar .p-listbox-option' + ) + const categoryCount = await categories.count() + + if (categoryCount > 1) { + // Click on the second category + await categories.nth(1).click() + + // Verify the category is selected + await expect(categories.nth(1)).toHaveClass(/p-listbox-option-selected/) + } + }) + + test('settings content area is visible', async ({ comfyPage }) => { + // Open settings dialog + await comfyPage.page.keyboard.press('Control+,') + const settingsDialog = comfyPage.page.locator('.settings-container') + await expect(settingsDialog).toBeVisible() + + // Check that the content area is visible + const contentArea = comfyPage.page.locator('.settings-content') + await expect(contentArea).toBeVisible() + + // Check that tab panels are visible + const tabPanels = comfyPage.page.locator('.settings-tab-panels') + await expect(tabPanels).toBeVisible() + }) + + test('search functionality affects UI state', async ({ comfyPage }) => { + // Open settings dialog + await comfyPage.page.keyboard.press('Control+,') + const settingsDialog = comfyPage.page.locator('.settings-container') + await expect(settingsDialog).toBeVisible() + + // Find the search box + const searchBox = comfyPage.page.locator('.settings-search-box input') + + // Type in search box + await searchBox.fill('graph') + await comfyPage.page.waitForTimeout(200) // Wait for debounce + + // Verify that the search input is handled + await expect(searchBox).toHaveValue('graph') + }) + + test('settings dialog can be closed', async ({ comfyPage }) => { + // Open settings dialog + await comfyPage.page.keyboard.press('Control+,') + const settingsDialog = comfyPage.page.locator('.settings-container') + await expect(settingsDialog).toBeVisible() + + // Close with escape key + await comfyPage.page.keyboard.press('Escape') + + // Verify dialog is closed + await expect(settingsDialog).not.toBeVisible() + }) + + test('search box has proper debouncing behavior', async ({ comfyPage }) => { + // Open settings dialog + await comfyPage.page.keyboard.press('Control+,') + const settingsDialog = comfyPage.page.locator('.settings-container') + await expect(settingsDialog).toBeVisible() + + // Type rapidly in search box + const searchBox = comfyPage.page.locator('.settings-search-box input') + await searchBox.fill('a') + await searchBox.fill('ab') + await searchBox.fill('abc') + await searchBox.fill('abcd') + + // Wait for debounce + await comfyPage.page.waitForTimeout(200) + + // Verify final value + await expect(searchBox).toHaveValue('abcd') + }) + + test('search excludes hidden settings from results', async ({ + comfyPage + }) => { + // Open settings dialog + await comfyPage.page.keyboard.press('Control+,') + const settingsDialog = comfyPage.page.locator('.settings-container') + await expect(settingsDialog).toBeVisible() + + // Search for our test settings + const searchBox = comfyPage.page.locator('.settings-search-box input') + await searchBox.fill('Test') + await comfyPage.page.waitForTimeout(300) // Wait for debounce + + // Get all settings content + const settingsContent = comfyPage.page.locator('.settings-tab-panels') + + // Should show visible setting but not hidden setting + await expect(settingsContent).toContainText('Test Visible Setting') + await expect(settingsContent).not.toContainText('Test Hidden Setting') + }) + + test('search excludes deprecated settings from results', async ({ + comfyPage + }) => { + // Open settings dialog + await comfyPage.page.keyboard.press('Control+,') + const settingsDialog = comfyPage.page.locator('.settings-container') + await expect(settingsDialog).toBeVisible() + + // Search for our test settings + const searchBox = comfyPage.page.locator('.settings-search-box input') + await searchBox.fill('Test') + await comfyPage.page.waitForTimeout(300) // Wait for debounce + + // Get all settings content + const settingsContent = comfyPage.page.locator('.settings-tab-panels') + + // Should show visible setting but not deprecated setting + await expect(settingsContent).toContainText('Test Visible Setting') + await expect(settingsContent).not.toContainText('Test Deprecated Setting') + }) + + test('search shows visible settings but excludes hidden and deprecated', async ({ + comfyPage + }) => { + // Open settings dialog + await comfyPage.page.keyboard.press('Control+,') + const settingsDialog = comfyPage.page.locator('.settings-container') + await expect(settingsDialog).toBeVisible() + + // Search for our test settings + const searchBox = comfyPage.page.locator('.settings-search-box input') + await searchBox.fill('Test') + await comfyPage.page.waitForTimeout(300) // Wait for debounce + + // Get all settings content + const settingsContent = comfyPage.page.locator('.settings-tab-panels') + + // Should only show the visible setting + await expect(settingsContent).toContainText('Test Visible Setting') + + // Should not show hidden or deprecated settings + await expect(settingsContent).not.toContainText('Test Hidden Setting') + await expect(settingsContent).not.toContainText('Test Deprecated Setting') + }) + + test('search by setting name excludes hidden and deprecated', async ({ + comfyPage + }) => { + // Open settings dialog + await comfyPage.page.keyboard.press('Control+,') + const settingsDialog = comfyPage.page.locator('.settings-container') + await expect(settingsDialog).toBeVisible() + + const searchBox = comfyPage.page.locator('.settings-search-box input') + const settingsContent = comfyPage.page.locator('.settings-tab-panels') + + // Search specifically for hidden setting by name + await searchBox.clear() + await searchBox.fill('Hidden') + await comfyPage.page.waitForTimeout(300) + + // Should not show the hidden setting even when searching by name + await expect(settingsContent).not.toContainText('Test Hidden Setting') + + // Search specifically for deprecated setting by name + await searchBox.clear() + await searchBox.fill('Deprecated') + await comfyPage.page.waitForTimeout(300) + + // Should not show the deprecated setting even when searching by name + await expect(settingsContent).not.toContainText('Test Deprecated Setting') + + // Search for visible setting by name - should work + await searchBox.clear() + await searchBox.fill('Visible') + await comfyPage.page.waitForTimeout(300) + + // Should show the visible setting + await expect(settingsContent).toContainText('Test Visible Setting') + }) +}) diff --git a/package-lock.json b/package-lock.json index 8caacea01..fa5cee570 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,18 +1,18 @@ { "name": "@comfyorg/comfyui-frontend", - "version": "1.24.0-1", + "version": "1.24.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@comfyorg/comfyui-frontend", - "version": "1.24.0-1", + "version": "1.24.1", "license": "GPL-3.0-only", "dependencies": { "@alloc/quick-lru": "^5.2.0", "@atlaskit/pragmatic-drag-and-drop": "^1.3.1", "@comfyorg/comfyui-electron-types": "^0.4.43", - "@comfyorg/litegraph": "^0.16.4", + "@comfyorg/litegraph": "^0.16.9", "@primevue/forms": "^4.2.5", "@primevue/themes": "^4.2.5", "@sentry/vue": "^8.48.0", @@ -949,9 +949,9 @@ "license": "GPL-3.0-only" }, "node_modules/@comfyorg/litegraph": { - "version": "0.16.4", - "resolved": "https://registry.npmjs.org/@comfyorg/litegraph/-/litegraph-0.16.4.tgz", - "integrity": "sha512-g4zhMxiCoE/WMap65fMNncQoTzn8ebMojjdv/b3Pc5RYmPQrwA32SDAvT/ndpkUonkOYmT9DgVmTxQv7LDQ6tA==", + "version": "0.16.9", + "resolved": "https://registry.npmjs.org/@comfyorg/litegraph/-/litegraph-0.16.9.tgz", + "integrity": "sha512-ZsvqkLqdG65e2UyM8oTOUTv/7VFEyGbG/C9dCZnhxdNq30UaE+F0iLaKq/17u6w4yewyZuqIn5MoOtjpxPqLDQ==", "license": "MIT" }, "node_modules/@cspotcode/source-map-support": { diff --git a/package.json b/package.json index 50edef4a4..2dfd9aa87 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "@comfyorg/comfyui-frontend", "private": true, - "version": "1.24.0-1", + "version": "1.24.1", "type": "module", "repository": "https://github.com/Comfy-Org/ComfyUI_frontend", "homepage": "https://comfy.org", @@ -77,7 +77,7 @@ "@alloc/quick-lru": "^5.2.0", "@atlaskit/pragmatic-drag-and-drop": "^1.3.1", "@comfyorg/comfyui-electron-types": "^0.4.43", - "@comfyorg/litegraph": "^0.16.4", + "@comfyorg/litegraph": "^0.16.9", "@primevue/forms": "^4.2.5", "@primevue/themes": "^4.2.5", "@sentry/vue": "^8.48.0", diff --git a/scripts/check-unused-i18n-keys.ts b/scripts/check-unused-i18n-keys.ts index 2be36a483..f459b8c23 100755 --- a/scripts/check-unused-i18n-keys.ts +++ b/scripts/check-unused-i18n-keys.ts @@ -17,7 +17,15 @@ const IGNORE_PATTERNS = [ /^templateWorkflows\./, // Template workflows are loaded dynamically /^dataTypes\./, // Data types might be referenced dynamically /^contextMenu\./, // Context menu items might be dynamic - /^color\./ // Color names might be used dynamically + /^color\./, // Color names might be used dynamically + // Auto-generated categories from collect-i18n-general.ts + /^menuLabels\./, // Menu labels generated from command labels + /^settingsCategories\./, // Settings categories generated from setting definitions + /^serverConfigItems\./, // Server config items generated from SERVER_CONFIG_ITEMS + /^serverConfigCategories\./, // Server config categories generated from config categories + /^nodeCategories\./, // Node categories generated from node definitions + // Setting option values that are dynamically generated + /\.options\./ // All setting options are rendered dynamically ] // Get list of staged locale files @@ -97,17 +105,21 @@ function shouldIgnoreKey(key: string): boolean { // Search for key usage in source files function isKeyUsed(key: string, sourceFiles: string[]): boolean { + // Escape special regex characters + const escapeRegex = (str: string) => + str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') + const escapedKey = escapeRegex(key) + const lastPart = key.split('.').pop() + const escapedLastPart = lastPart ? escapeRegex(lastPart) : '' + // Common patterns for i18n key usage const patterns = [ // Direct usage: $t('key'), t('key'), i18n.t('key') - new RegExp(`[t$]\\s*\\(\\s*['"\`]${key}['"\`]`, 'g'), + new RegExp(`[t$]\\s*\\(\\s*['"\`]${escapedKey}['"\`]`, 'g'), // With namespace: $t('g.key'), t('namespace.key') - new RegExp( - `[t$]\\s*\\(\\s*['"\`][^'"]+\\.${key.split('.').pop()}['"\`]`, - 'g' - ), + new RegExp(`[t$]\\s*\\(\\s*['"\`][^'"]+\\.${escapedLastPart}['"\`]`, 'g'), // Dynamic keys might reference parts of the key - new RegExp(`['"\`]${key}['"\`]`, 'g') + new RegExp(`['"\`]${escapedKey}['"\`]`, 'g') ] for (const file of sourceFiles) { @@ -154,7 +166,7 @@ async function checkNewUnusedKeys() { // Report results if (unusedNewKeys.length > 0) { - console.log('\nโŒ Found unused NEW i18n keys:\n') + console.log('\nโš ๏ธ Warning: Found unused NEW i18n keys:\n') for (const key of unusedNewKeys.sort()) { console.log(` - ${key}`) @@ -164,9 +176,10 @@ async function checkNewUnusedKeys() { console.log( '\nThese keys were added but are not used anywhere in the codebase.' ) - console.log('Please either use them or remove them before committing.') + console.log('Consider using them or removing them in a future update.') - process.exit(1) + // Changed from process.exit(1) to process.exit(0) for warning only + process.exit(0) } else { // Silent success - no output needed } diff --git a/src/components/graph/TitleEditor.vue b/src/components/graph/TitleEditor.vue index eabc03bb3..f3e49e77f 100644 --- a/src/components/graph/TitleEditor.vue +++ b/src/components/graph/TitleEditor.vue @@ -41,7 +41,15 @@ const previousCanvasDraggable = ref(true) const onEdit = (newValue: string) => { if (titleEditorStore.titleEditorTarget && newValue.trim() !== '') { - titleEditorStore.titleEditorTarget.title = newValue.trim() + const trimmedTitle = newValue.trim() + titleEditorStore.titleEditorTarget.title = trimmedTitle + + // If this is a subgraph node, sync the runtime subgraph name for breadcrumb reactivity + const target = titleEditorStore.titleEditorTarget + if (target instanceof LGraphNode && target.isSubgraphNode?.()) { + target.subgraph.name = trimmedTitle + } + app.graph.setDirtyCanvas(true, true) } showInput.value = false diff --git a/src/components/helpcenter/HelpCenterMenuContent.vue b/src/components/helpcenter/HelpCenterMenuContent.vue index c9f708937..45a6da119 100644 --- a/src/components/helpcenter/HelpCenterMenuContent.vue +++ b/src/components/helpcenter/HelpCenterMenuContent.vue @@ -54,7 +54,7 @@ -
+

{{ $t('helpCenter.whatsNew') }}

@@ -126,6 +126,7 @@ import { useI18n } from 'vue-i18n' import { type ReleaseNote } from '@/services/releaseService' import { useCommandStore } from '@/stores/commandStore' import { useReleaseStore } from '@/stores/releaseStore' +import { useSettingStore } from '@/stores/settingStore' import { electronAPI, isElectron } from '@/utils/envUtil' import { formatVersionAnchor } from '@/utils/formatUtil' @@ -161,13 +162,14 @@ const TIME_UNITS = { const SUBMENU_CONFIG = { DELAY_MS: 100, OFFSET_PX: 8, - Z_INDEX: 1002 + Z_INDEX: 10001 } as const // Composables const { t, locale } = useI18n() const releaseStore = useReleaseStore() const commandStore = useCommandStore() +const settingStore = useSettingStore() // Emits const emit = defineEmits<{ @@ -182,6 +184,9 @@ let hoverTimeout: number | null = null // Computed const hasReleases = computed(() => releaseStore.releases.length > 0) +const showVersionUpdates = computed(() => + settingStore.get('Comfy.Notification.ShowVersionUpdates') +) const moreMenuItem = computed(() => menuItems.value.find((item) => item.key === 'more') diff --git a/src/components/helpcenter/WhatsNewPopup.vue b/src/components/helpcenter/WhatsNewPopup.vue index e8c4c9d1b..26d1dc8a0 100644 --- a/src/components/helpcenter/WhatsNewPopup.vue +++ b/src/components/helpcenter/WhatsNewPopup.vue @@ -32,28 +32,32 @@
-
-
- -
- - {{ $t('whatsNewPopup.learnMore') }} - - - + +
+ + {{ $t('whatsNewPopup.learnMore') }} + + + +
@@ -68,7 +72,7 @@ import type { ReleaseNote } from '@/services/releaseService' import { useReleaseStore } from '@/stores/releaseStore' import { formatVersionAnchor } from '@/utils/formatUtil' -const { locale } = useI18n() +const { locale, t } = useI18n() const releaseStore = useReleaseStore() // Local state for dismissed status @@ -101,13 +105,12 @@ const changelogUrl = computed(() => { // Format release content for display using marked const formattedContent = computed(() => { if (!latestRelease.value?.content) { - return '

No release notes available.

' + return `

${t('whatsNewPopup.noReleaseNotes')}

` } try { // Use marked to parse markdown to HTML return marked(latestRelease.value.content, { - breaks: true, // Convert line breaks to
gfm: true // Enable GitHub Flavored Markdown }) } catch (error) { @@ -199,14 +202,10 @@ defineExpose({ } .whats-new-popup { - padding: 32px 32px 24px; background: #353535; border-radius: 12px; max-width: 400px; width: 400px; - display: flex; - flex-direction: column; - gap: 32px; outline: 1px solid #4e4e4e; outline-offset: -1px; box-shadow: 0px 8px 32px rgba(0, 0, 0, 0.3); @@ -217,6 +216,11 @@ defineExpose({ .popup-content { display: flex; flex-direction: column; + gap: 24px; + max-height: 80vh; + overflow-y: auto; + padding: 32px 32px 24px; + border-radius: 12px; } /* Close button */ @@ -224,17 +228,17 @@ defineExpose({ position: absolute; top: 0; right: 0; - width: 31px; - height: 31px; - padding: 6px 7px; + width: 32px; + height: 32px; + padding: 6px; background: #7c7c7c; - border-radius: 15.5px; + border-radius: 16px; border: none; cursor: pointer; display: flex; justify-content: center; align-items: center; - transform: translate(50%, -50%); + transform: translate(30%, -30%); transition: background-color 0.2s ease, transform 0.1s ease; @@ -247,7 +251,7 @@ defineExpose({ .close-button:active { background: #6a6a6a; - transform: translate(50%, -50%) scale(0.95); + transform: translate(30%, -30%) scale(0.95); } .close-icon { @@ -288,73 +292,45 @@ defineExpose({ .content-text { color: white; font-size: 14px; - font-family: 'Inter', sans-serif; - font-weight: 400; line-height: 1.5; word-wrap: break-word; } /* Style the markdown content */ +/* Title */ +.content-text :deep(*) { + box-sizing: border-box; + margin: 0; + padding: 0; +} + .content-text :deep(h1) { - color: white; - font-size: 20px; - font-weight: 700; - margin: 0 0 16px 0; - line-height: 1.3; -} - -.content-text :deep(h2) { - color: white; - font-size: 18px; - font-weight: 600; - margin: 16px 0 12px 0; - line-height: 1.3; -} - -.content-text :deep(h2:first-child) { - margin-top: 0; -} - -.content-text :deep(h3) { - color: white; font-size: 16px; - font-weight: 600; - margin: 12px 0 8px 0; - line-height: 1.3; + font-weight: 700; + margin-bottom: 8px; } -.content-text :deep(h3:first-child) { - margin-top: 0; -} - -.content-text :deep(h4) { - color: white; - font-size: 14px; - font-weight: 600; - margin: 8px 0 6px 0; -} - -.content-text :deep(h4:first-child) { - margin-top: 0; +/* Version subtitle - targets the first p tag after h1 */ +.content-text :deep(h1 + p) { + color: #c0c0c0; + font-size: 16px; + font-weight: 500; + margin-bottom: 16px; + opacity: 0.8; } +/* Regular paragraphs - short description */ .content-text :deep(p) { - margin: 0 0 12px 0; - line-height: 1.6; -} - -.content-text :deep(p:first-child) { - margin-top: 0; -} - -.content-text :deep(p:last-child) { - margin-bottom: 0; + margin-bottom: 16px; + color: #e0e0e0; } +/* List */ .content-text :deep(ul), .content-text :deep(ol) { - margin: 0 0 12px 0; - padding-left: 24px; + margin-bottom: 16px; + padding-left: 0; + list-style: none; } .content-text :deep(ul:first-child), @@ -367,12 +343,63 @@ defineExpose({ margin-bottom: 0; } +/* List items */ +.content-text :deep(li) { + margin-bottom: 8px; + position: relative; + padding-left: 20px; +} + +.content-text :deep(li:last-child) { + margin-bottom: 0; +} + +/* Custom bullet points */ +.content-text :deep(li::before) { + content: ''; + position: absolute; + left: 0; + top: 10px; + display: flex; + width: 8px; + height: 8px; + justify-content: center; + align-items: center; + aspect-ratio: 1/1; + border-radius: 100px; + background: #60a5fa; +} + +/* List item strong text */ +.content-text :deep(li strong) { + color: #fff; + font-size: 14px; + display: block; + margin-bottom: 4px; +} + +.content-text :deep(li p) { + font-size: 12px; + margin-bottom: 0; + line-height: 2; +} + +/* Code styling */ +.content-text :deep(code) { + background-color: #2a2a2a; + border: 1px solid #4a4a4a; + border-radius: 4px; + padding: 2px 6px; + color: #f8f8f2; + white-space: nowrap; +} + /* Remove top margin for first media element */ .content-text :deep(img:first-child), .content-text :deep(video:first-child), .content-text :deep(iframe:first-child) { margin-top: -32px; /* Align with the top edge of the popup content */ - margin-bottom: 12px; + margin-bottom: 24px; } /* Media elements */ @@ -381,8 +408,7 @@ defineExpose({ .content-text :deep(iframe) { width: calc(100% + 64px); height: auto; - border-radius: 6px; - margin: 12px -32px; + margin: 24px -32px; display: block; } @@ -397,7 +423,6 @@ defineExpose({ .learn-more-link { color: #60a5fa; font-size: 14px; - font-family: 'Inter', sans-serif; font-weight: 500; line-height: 18.2px; text-decoration: none; @@ -417,7 +442,6 @@ defineExpose({ border: none; color: #121212; font-size: 14px; - font-family: 'Inter', sans-serif; font-weight: 500; cursor: pointer; } diff --git a/src/components/sidebar/SideToolbar.vue b/src/components/sidebar/SideToolbar.vue index 120747b54..4b7e6df00 100644 --- a/src/components/sidebar/SideToolbar.vue +++ b/src/components/sidebar/SideToolbar.vue @@ -6,7 +6,8 @@ :key="tab.id" :icon="tab.icon" :icon-badge="tab.iconBadge" - :tooltip="tab.tooltip + getTabTooltipSuffix(tab)" + :tooltip="tab.tooltip" + :tooltip-suffix="getTabTooltipSuffix(tab)" :selected="tab.id === selectedTab?.id" :class="tab.id + '-tab-button'" @click="onTabClick(tab)" diff --git a/src/components/sidebar/SidebarHelpCenterIcon.vue b/src/components/sidebar/SidebarHelpCenterIcon.vue index 30537b75a..c37cd973e 100644 --- a/src/components/sidebar/SidebarHelpCenterIcon.vue +++ b/src/components/sidebar/SidebarHelpCenterIcon.vue @@ -46,7 +46,7 @@ - +
{ left: 0; right: 0; bottom: 0; - z-index: 999; + z-index: 9999; background: transparent; } .help-center-popup { position: absolute; bottom: 1rem; - z-index: 1000; + z-index: 10000; animation: slideInUp 0.2s ease-out; pointer-events: auto; } diff --git a/src/components/sidebar/SidebarIcon.spec.ts b/src/components/sidebar/SidebarIcon.spec.ts index 806ce69ca..ef02f3d9e 100644 --- a/src/components/sidebar/SidebarIcon.spec.ts +++ b/src/components/sidebar/SidebarIcon.spec.ts @@ -4,6 +4,7 @@ import PrimeVue from 'primevue/config' import OverlayBadge from 'primevue/overlaybadge' import Tooltip from 'primevue/tooltip' import { describe, expect, it } from 'vitest' +import { createI18n } from 'vue-i18n' import SidebarIcon from './SidebarIcon.vue' @@ -15,6 +16,14 @@ type SidebarIconProps = { iconBadge?: string | (() => string | null) } +const i18n = createI18n({ + legacy: false, + locale: 'en', + messages: { + en: {} + } +}) + describe('SidebarIcon', () => { const exampleProps: SidebarIconProps = { icon: 'pi pi-cog', @@ -24,7 +33,7 @@ describe('SidebarIcon', () => { const mountSidebarIcon = (props: Partial, options = {}) => { return mount(SidebarIcon, { global: { - plugins: [PrimeVue], + plugins: [PrimeVue, i18n], directives: { tooltip: Tooltip }, components: { OverlayBadge, Button } }, diff --git a/src/components/sidebar/SidebarIcon.vue b/src/components/sidebar/SidebarIcon.vue index 3a3761704..1c1dffc1b 100644 --- a/src/components/sidebar/SidebarIcon.vue +++ b/src/components/sidebar/SidebarIcon.vue @@ -1,6 +1,10 @@