fix: improve update command validation and empty value handling#158
Merged
felipefreitag merged 5 commits intoresend:mainfrom Mar 26, 2026
Merged
Conversation
Contributor
There was a problem hiding this comment.
1 issue found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/commands/broadcasts/update.ts">
<violation number="1" location="src/commands/broadcasts/update.ts:71">
P2: Empty-string option values now pass the null-based no_changes guard but are ignored by downstream truthy checks (`opts.htmlFile`, `opts.textFile`, `opts.reactEmail`), potentially resulting in an empty update payload. This is inconsistent with the new empty-value handling and can cause confusing no-op updates.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Move react-email conflict, html+htmlFile exclusion, and stdin conflict checks before pickId so users aren't prompted for an ID before seeing a validation error. Fix opts.var truthy check to use != null for consistency. Add happy-path tests for templates update.
felipefreitag
approved these changes
Mar 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change fixes two update-command problems that could make the CLI behave in a confusing way.
The first issue was in
templates update. If you ran the command without actually passing any fields to update, the CLI could still try to resolve or prompt for a template id before finally telling you there was nothing to change. That made the command feel backwards. It now checks forno_changesfirst and fails immediately instead of trying to fetch or pick a template unnecessarily.The second issue was inconsistent handling of empty string values in update flows.
broadcasts updatewas using truthy checks, which meant values like--text ""or--html ""could be treated like nothing was passed and silently dropped from the SDK payload.templates updatealso still had a few truthy checks in its downstream validation and warning paths, so empty string values could bypass conflict checks or skip warnings there too. Both commands now use explicit null/undefined checks so intentionally empty values are handled consistently and predictably.I added regression coverage for both commands, including checks that
templates updatefails before opening the picker when there are no changes, and checks that empty string values still trigger the correct validation, warnings, and payload behavior.Summary by cubic
Fixes confusing update flows in the CLI.
templates updatenow validates and fails early without prompting for an id when there are no changes or conflicting options; both update commands treat empty strings and empty option values (including empty file paths) as intentional input.templates update: run no-change and conflict validation (react-email vs html/html-file, html+html-file, stdin conflicts) before resolving the id; returns specific errors without prompts.--text "",--html "",--html-file "",--text-file "",--react-email ""are treated as provided; file inputs win with warnings;--varis respected when present.Written for commit d1075b4. Summary will update on new commits.