ci: add backport labels automatically when a new minor version is released #6615
Conversation
dosubot
bot
added
the
size:M
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 11/06/2025, 08:19:55 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Test Results⏰ Completed at: 11/06/2025, 08:43:22 PM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.25 MB (baseline 3.25 MB) • ⚪ 0 BMain entry bundles and manifests
Graph Workspace — 792 kB (baseline 792 kB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Views & Navigation — 8.18 kB (baseline 8.18 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Panels & Settings — 293 kB (baseline 293 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
UI Components — 12.6 kB (baseline 12.6 kB) • ⚪ 0 BReusable component library chunks
Data & Services — 10.4 kB (baseline 10.4 kB) • ⚪ 0 BStores, services, APIs, and repositories
Utilities & Hooks — 1.07 kB (baseline 1.07 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Vendor & Third-Party — 5.32 MB (baseline 5.32 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 2.55 MB (baseline 2.55 MB) • ⚪ 0 BBundles that do not match a named category
|
| if [[ -z "$BRANCH_BASE" ]]; then | ||
| echo "::error::Branch base not set; unable to manage labels" | ||
| exit 1 |
There was a problem hiding this comment.
Will the exit 1 here block subsequent steps?
There was a problem hiding this comment.
Might just need to add always() && to "Post summary"
DrJKL
left a comment
DrJKL
left a comment
There was a problem hiding this comment.
The labels list is so long already 😧
christian-byrne
force-pushed
the
chore/add-release-labels
branch
from
897ee0c to
22cf3cf
Compare
|
No dependency changes detected. Learn more about Socket for GitHub. 👍 No dependency changes detected in pull request |
We can remove the oldest release labels as well in this step, so the list of release tag labels does not grow. What do you think? Edit: done in f80f475 |
christian-byrne
force-pushed
the
chore/add-release-labels
branch
from
22cf3cf to
47251ce
Compare
christian-byrne
force-pushed
the
chore/add-release-labels
branch
from
47251ce to
f80f475
Compare
christian-byrne
added
the
claude-review
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| - name: Ensure release labels | ||
| if: steps.check_version.outputs.is_minor_bump == 'true' | ||
| env: | ||
| GH_TOKEN: ${{ secrets.PR_GH_TOKEN || secrets.GITHUB_TOKEN }} |
There was a problem hiding this comment.
[security] high Priority
Issue: Secrets are potentially exposed via environment variables with fallback pattern
Context: Using secrets.PR_GH_TOKEN || secrets.GITHUB_TOKEN pattern can leak token usage in logs
Suggestion: Use explicit conditional logic or ensure proper token scoping with minimal permissions
| exit 1 | ||
| fi | ||
| declare -A COLORS=( |
There was a problem hiding this comment.
[quality] medium Priority
Issue: Hardcoded color values in associative array without validation
Context: Colors 4361ee and 4f6ef5 are not validated and may be inconsistent with design system
Suggestion: Extract colors to constants or validate hex format with proper error handling
| fi | ||
| done | ||
| MIN_LABELS_TO_KEEP=3 |
There was a problem hiding this comment.
[quality] low Priority
Issue: Magic numbers used without clear reasoning
Context: MIN_LABELS_TO_KEEP=3 and MAX_LABELS_TO_FETCH=200 are hardcoded without explanation
Suggestion: Add inline comments explaining these thresholds or make them configurable via environment variables
| env: | ||
| GH_TOKEN: ${{ secrets.PR_GH_TOKEN || secrets.GITHUB_TOKEN }} | ||
| run: | | ||
| set -euo pipefail |
There was a problem hiding this comment.
[quality] medium Priority
Issue: Inconsistent error handling pattern
Context: Some operations use set -euo pipefail while others use manual error checking
Suggestion: Apply consistent error handling throughout the script or document why different patterns are used
| REMOVE_COUNT=$((TOTAL - MIN_LABELS_TO_KEEP)) | ||
| if (( REMOVE_COUNT > 1 )); then |
There was a problem hiding this comment.
[quality] medium Priority
Issue: Label deletion logic has potential off-by-one errors
Context: The pruning logic limits removal to 1 when REMOVE_COUNT > 1, but this might not maintain proper ordering
Suggestion: Review deletion logic to ensure oldest labels are properly removed and consider sorting by semantic version
| MIN_LABELS_TO_KEEP=3 | ||
| MAX_LABELS_TO_FETCH=200 | ||
| for PREFIX in core cloud; do |
There was a problem hiding this comment.
[architecture] low Priority
Issue: Missing conditional execution guard for label cleanup
Context: Label cleanup runs even if label creation fails, potentially causing inconsistent state
Suggestion: Add error handling and only run cleanup if label creation succeeds
| "name": "@comfyorg/comfyui-frontend", | ||
| "private": true, | ||
| "version": "1.32.2", | ||
| "version": "1.32.1", |
There was a problem hiding this comment.
[architecture] critical Priority
Issue: Version downgrade from 1.32.2 to 1.32.1 in unrelated PR
Context: This PR is about CI label automation but includes a version downgrade, indicating merge conflicts or inappropriate PR scope
Suggestion: Separate version changes into distinct PR or verify if this downgrade is intentional
| "size:collect": "node scripts/size-collect.js", | ||
| "size:report": "node scripts/size-report.js", | ||
| "collect-i18n": "pnpm exec playwright test --config=playwright.i18n.config.ts", | ||
| "dev:cloud": "cross-env DEV_SERVER_COMFYUI_URL='https://testcloud.comfy.org/' nx serve", |
There was a problem hiding this comment.
[architecture] high Priority
Issue: Removal of dev:cloud script without explanation
Context: The removal of cross-env DEV_SERVER_COMFYUI_URL cloud development script appears unrelated to label automation
Suggestion: Document why this script was removed or restore if accidentally deleted during merge
| __CONFIG__: { | ||
| mixpanel_token?: string | ||
| require_whitelist?: boolean | ||
| subscription_required?: boolean |
There was a problem hiding this comment.
[architecture] critical Priority
Issue: Massive scope creep - PR contains hundreds of unrelated changes
Context: A PR titled CI label automation includes version downgrades, CSS changes, component deletions, test removals, and config modifications
Suggestion: Split this PR into focused changes - one for CI automation, separate ones for cleanup, refactoring, and version management
| @@ -1,99 +0,0 @@ | |||
| import { computed, toValue, watch } from 'vue' | |||
There was a problem hiding this comment.
[architecture] high Priority
Issue: Deletion of useAssetWidgetData composable without migration path
Context: This composable is likely used by widget components and its removal could break asset handling functionality
Suggestion: Verify all references are properly migrated before deleting or provide deprecation path
There was a problem hiding this comment.
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: ci: add backport labels automatically when a new minor version is released (#6615)
Impact: 71 additions, 1 deletions across 1 workflow file (plus 100+ unrelated files)
Issue Distribution
- Critical: 3
- High: 3
- Medium: 3
- Low: 1
Category Breakdown
- Architecture: 5 issues
- Security: 1 issues
- Performance: 0 issues
- Code Quality: 4 issues
Key Findings
Architecture & Design
Major Concern: This PR suffers from massive scope creep. What should be a focused CI enhancement includes:
- Version downgrades (1.32.2 to 1.32.1)
- Removal of development scripts
- Deletion of composables and components
- CSS/styling changes across hundreds of files
- Test file removals
- Configuration modifications
The core CI workflow change (adding label automation) is well-structured but embedded in a problematic PR scope.
Security Considerations
The workflow uses GitHub token fallback patterns that could potentially leak token usage information in logs. While not a direct security vulnerability, it represents a security hygiene issue for CI/CD systems.
Performance Impact
The CI changes themselves should have minimal performance impact. However, the massive scope of unrelated changes makes it impossible to properly assess the full performance implications of this PR.
Integration Points
Critical Risk: The deletion of composables like useAssetWidgetData and removal of development scripts could break existing functionality. The PR scope makes it difficult to track all integration impacts.
Positive Observations
- The core label automation logic is well-structured with proper error handling
- GitHub Actions workflow follows good practices with proper conditionals
- Label creation/updating logic includes color coding and descriptions
- Cleanup logic prevents unlimited label accumulation
Next Steps
- CRITICAL: Split this PR into focused changes:
- CI label automation only
- Separate PR for version management
- Separate PR for cleanup/refactoring
- Separate PR for component deletions (with migration plan)
- Address security token handling in workflow
- Verify all deleted components have been properly migrated
- Test CI workflow in isolation before merging
This is a comprehensive automated review. The scope of changes in this PR makes manual review essential for architectural decisions.
github-actions
bot
removed
the
claude-review
| --json name \ | ||
| --limit "$MAX_LABELS_TO_FETCH" \ | ||
| --jq '.[].name' | | ||
| grep -E "^${PREFIX}/[0-9]+\.[0-9]+$" | |
| REMOVE_COUNT=$((TOTAL - MIN_LABELS_TO_KEEP)) | ||
| if (( REMOVE_COUNT > 1 )); then | ||
| REMOVE_COUNT=1 |
add the
core/x.yyandcloud/x.yylabels (used for backporting) automatically when a minor version is released (and the previous version is made into RC).By "add labels" I mean add them into the repo's list of available labels that can be used in the UI.
┆Issue is synchronized with this Notion page by Unito