-
Notifications
You must be signed in to change notification settings - Fork 4.2k
refactor(worker): simplify step halt return condition #9947
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
|
Cursor Agent can help with this pull request. Just |
✅ Deploy Preview for dashboard-v2-novu-staging canceled.
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Hey there and thank you for opening this pull request! 👋 We require pull request titles to follow specific formatting rules and it looks like your proposed title needs to be adjusted. Your PR title is: Requirements:
Expected format: Details: PR title must end with 'fixes TICKET-ID' (e.g., 'fixes NOV-123') or include ticket ID in branch name |
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.
Pull request overview
Refactors shouldHaltOnStepFailure to simplify the halt decision logic by collapsing the job.type early-return into a single guarded condition.
Changes:
- Removes the explicit
!job.typeearly return. - Guards the
isActionStepTypecheck withjob.typein a single conditional.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * Action steps always stop on failure across all versions (v1 & v2) | ||
| */ | ||
| if (isActionStepType(job.type)) { | ||
| if (job.type && isActionStepType(job.type)) { |
Copilot
AI
Jan 30, 2026
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.
The PR description says this change improves robustness by only calling isActionStepType when job.type is present, but the previous implementation already short-circuited on !job.type. If there’s no intended behavior change here, consider updating the PR description to reflect that this is a refactor/simplification only (or add a note if a subtle behavior/typing issue was being addressed).
Merged 4 upstream commits: - refactor(worker): simplify step halt return condition (novuhq#9947) - refactor(api-service): make context required parameter for subscriber JWT (novuhq#9943) - refactor(api-service): add type annotations to updateServices method parameters (novuhq#9945) - refactor(worker): add type annotation to getIntegrationHandler parameter (novuhq#9946) Kept ReNovu feature unlocks for self-hosted deployments: - Settings page accessible - Environments page accessible - Settings menu in sidebar Co-Authored-By: Claude Opus 4.5 <[email protected]>
This pull request makes a small change to the
should-halt-on-step-failure.tsutility function, improving the handling of jobs that may not have a definedtypeproperty. The update ensures that the function only checks for action step types whenjob.typeis present, making the logic more robust.This pull request contains changes generated by a Cursor Cloud Agent