-
Notifications
You must be signed in to change notification settings - Fork 402
ci: comment when publish to npm/pypi finishes successfully #6628
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🎭 Playwright Test Results⏰ Completed at: 11/07/2025, 03:41:11 AM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 11/07/2025, 03:27:06 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
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 — 793 kB (baseline 793 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
|
| exit 1 | ||
| fi | ||
| NEW_VERSION=$(node -pe "const fs=require('fs');const data=JSON.parse(fs.readFileSync(process.argv[1],'utf8'));if(!data.version){process.exit(1);}data.version" "$VERSION_FILE") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[security] high Priority
Issue: Potential command injection via unquoted variable in node command
Context: VERSION_FILE variable is passed directly to node process without proper quoting, allowing potential injection
Suggestion: Quote the variable: node -pe "..." "$VERSION_FILE" or sanitize input first
| Subgraph | ||
| } from '@/lib/litegraph/src/litegraph' | ||
|
|
||
| export function isPrimitiveNode( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[quality] medium Priority
Issue: Function refactor changes arrow function to regular function without clear benefit
Context: isPrimitiveNode was changed from arrow function export to regular function, which affects consistency
Suggestion: Maintain consistent function style across the codebase or document the reasoning for this change
| isSelected(option) && !disabled, | ||
| 'hover:bg-interface-menu-component-surface-hovered': | ||
| !isSelected(option) && !disabled, | ||
| 'bg-white': isSelected(option) && !disabled, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[architecture] high Priority
Issue: Hardcoded color values replace semantic theme CSS classes
Context: Changes from semantic classes like 'bg-interface-menu-component-surface-selected' to hardcoded 'bg-white' violates design system patterns
Suggestion: Use semantic CSS values from the theme as specified in CLAUDE.md guidelines instead of hardcoded colors
| "comfy": "Comfy", | ||
| "refresh": "Refresh", | ||
| "refreshNode": "Refresh Node", | ||
| "vitePreloadErrorTitle": "New Version Available", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[quality] medium Priority
Issue: Locale strings removed without replacement or deprecated usage cleanup
Context: Multiple locale entries deleted including vitePreloadErrorTitle, templates, console without indication they're no longer used
Suggestion: Verify these strings are truly unused or provide migration path for their replacement
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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: comment when publish to npm/pypi finishes successfully (#6628)
Impact: 185 additions, 10 deletions across 29 files
Issue Distribution
- Critical: 1
- High: 2
- Medium: 2
- Low: 0
Category Breakdown
- Architecture: 2 issues
- Security: 1 issues
- Performance: 0 issues
- Code Quality: 2 issues
Key Findings
Architecture & Design
The most significant concern is PR scope mismatch. While the title indicates this PR only adds CI commenting functionality, it contains extensive unrelated changes including:
- Component deletion (LanguageSelector.vue)
- Utility file reorganization (moving files from workbench/utils to utils/)
- Styling changes from semantic CSS classes to hardcoded colors
- Locale string deletions
- Vue component modifications
This violates the principle of atomic, single-purpose PRs and makes review/rollback more difficult.
Security Considerations
The GitHub Actions script has a potential command injection vulnerability where the VERSION_FILE variable is passed unquoted to a Node.js command. While this is likely low-risk in practice due to GitHub Actions context, it should be properly sanitized.
Integration Points
The file moves from workbench/utils to utils/ suggest organizational refactoring, but the impact on other parts of the system isn't clear. The deletion of LanguageSelector component may affect i18n functionality.
Design System Compliance
Changes to Vue components replace semantic CSS classes (e.g., 'bg-interface-menu-component-surface-selected') with hardcoded colors ('bg-white'), which violates the design system guidelines outlined in CLAUDE.md.
Positive Observations
- The CI automation functionality itself is well-structured with proper error handling
- TypeScript usage is generally sound in the new utility functions
- File moves appear to follow a logical reorganization pattern
References
- Repository Architecture Guide
- CLAUDE.md guidelines regarding semantic CSS usage
Next Steps
- Critical: Split this PR - separate CI functionality from application code changes
- Address command injection vulnerability in GitHub Actions
- Restore semantic CSS classes or document design system changes
- Verify locale string deletions don't break functionality
- Document the reasoning behind component deletion and file reorganization
This is a comprehensive automated review. For architectural decisions requiring human judgment, please request additional manual review.
This change adds a reusable
post-release-summarycomposite action that automatically figures out the current/previous version, generates diff + PyPI/npm links, and posts (or updates) the release summary comment whenever the publish jobs succeed.┆Issue is synchronized with this Notion page by Unito