Merged
Conversation
Contributor
There was a problem hiding this comment.
cubic analysis
7 issues found across 13 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:89">
P1: According to linked Linear issue ENG-4686, `--text -` should read from stdin, but this change only supports stdin via `--text-file -`, so `--text -` still sends a literal `"-"` body.</violation>
</file>
<file name="src/commands/emails/send.ts">
<violation number="1" location="src/commands/emails/send.ts:275">
P2: According to linked Linear issue ENG-4686, `--text -` should read stdin, but this code still treats `-` as a literal text value unless `--text-file` is used.</violation>
</file>
<file name="src/commands/templates/create.ts">
<violation number="1" location="src/commands/templates/create.ts:100">
P2: `--text -` is still treated as a literal string instead of reading stdin, so the ENG-4686 behavior remains unfixed for this command.
According to linked Linear issue ENG-4686, this should read stdin.</violation>
</file>
<file name="tests/commands/emails/send.test.ts">
<violation number="1" location="tests/commands/emails/send.test.ts:607">
P2: According to linked Linear issue ENG-4686, stdin piping must work with `--text -`, but the new coverage validates `--text-file -` instead. Add/adjust tests (and implementation if needed) for `--text -` so the reported bug is actually enforced.</violation>
</file>
<file name="src/commands/broadcasts/create.ts">
<violation number="1" location="src/commands/broadcasts/create.ts:27">
P2: According to linked Linear issue ENG-4686, stdin piping is required for `--text -`, but this change adds `--text-file -` instead and does not implement `--text -` stdin behavior.</violation>
<violation number="2" location="src/commands/broadcasts/create.ts:49">
P3: The new help text says users must provide exactly one body option, but the command accepts both HTML and text together. Update wording to match actual behavior.</violation>
</file>
<file name="tests/commands/broadcasts/update.test.ts">
<violation number="1" location="tests/commands/broadcasts/update.test.ts:248">
P2: According to linked Linear issue ENG-4686, tests should validate stdin piping via `--text -`, but the new cases only validate `--text-file`, leaving the required bug path untested.</violation>
</file>
Linked issue analysis
Linked issue: ENG-4686: Stdin piping with --text -
| Status | Acceptance criteria | Notes |
|---|---|---|
| ❌ | When user passes --text -, the CLI should read stdin (Unix convention) | Introduced --text-file but did not make --text accept '-' |
| ✅ | Provide a --text-file option that accepts '-' to read stdin | --text-file option added and used to set text from readFile |
| ✅ | readFile implementation treats '-' as stdin and reads from fd 0 | readFile reads from fd 0 when filePath === '-' |
| ✅ | Support piping stdin for batch command via --file - | Batch command help/examples updated and tests check readFile called |
| ✅ | Error when both --html-file and --text-file are '-' (avoid reading stdin twice) | Added explicit checks and outputError when both equal '-' |
| ✅ | Warn when both inline (--text/--html) and file flags provided; file wins | Writes warning to stderr and uses file value when both provided |
| ✅ | Add tests covering stdin/file behavior across relevant commands | Many tests added for send, batch, templates, broadcasts for '-' behaviour |
| ✅ | Detect empty required flags and error invalid_options (e.g., empty --from/--subject) | Added empty-flag validation and tests for empty flags |
| ✅ | Update help text & examples to indicate using '-' for stdin where supported | Help texts and examples updated across commands to mention '-' usage |
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| } | ||
|
|
||
| let html = opts.html; | ||
| let text = opts.text; |
Contributor
There was a problem hiding this comment.
P1: According to linked Linear issue ENG-4686, --text - should read from stdin, but this change only supports stdin via --text-file -, so --text - still sends a literal "-" body.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/commands/broadcasts/update.ts, line 89:
<comment>According to linked Linear issue ENG-4686, `--text -` should read from stdin, but this change only supports stdin via `--text-file -`, so `--text -` still sends a literal `"-"` body.</comment>
<file context>
@@ -54,24 +61,51 @@ Variable interpolation:
+ }
+
let html = opts.html;
+ let text = opts.text;
if (opts.htmlFile) {
</file context>
| } | ||
| }); | ||
|
|
||
| test('--text-file - reads from stdin', async () => { |
Contributor
There was a problem hiding this comment.
P2: According to linked Linear issue ENG-4686, stdin piping must work with --text -, but the new coverage validates --text-file - instead. Add/adjust tests (and implementation if needed) for --text - so the reported bug is actually enforced.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/commands/emails/send.test.ts, line 607:
<comment>According to linked Linear issue ENG-4686, stdin piping must work with `--text -`, but the new coverage validates `--text-file -` instead. Add/adjust tests (and implementation if needed) for `--text -` so the reported bug is actually enforced.</comment>
<file context>
@@ -572,6 +572,279 @@ describe('send command', () => {
+ }
+ });
+
+ test('--text-file - reads from stdin', async () => {
+ spies = setupOutputSpies();
+
</file context>
vcapretz
approved these changes
Mar 19, 2026
Add --text-file option to broadcasts create/update, templates
create/update. Update --html-file and --file descriptions to mention
stdin ("-"). Add --html/--html-file and --text/--text-file conflict
warnings for broadcasts. Add dual-stdin validation across all commands.
Add type assertion for CreateEmailOptions discriminated union in send.ts, add stdin pipe examples to broadcasts create/update, and simplify test mock.
Align with the pattern used in send and broadcasts commands: warn to stderr and prefer --html-file, rather than hard-erroring.
The command accepts both HTML and text together, so "exactly one of" was inaccurate.
125bfca to
e1576ae
Compare
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.
Adds the --text-file option and allows piping files to both --html-file and --text-file options.
echo "<h1>Hello</h1>" | resend emails send --from you@domain.com --to user@example.com --subject "Hi" --html-file -cat body.txt | resend emails send --from you@domain.com --to user@example.com --subject "Hi" --text-file -echo '[{"from":"you@domain.com","to":["user@example.com"],"subject":"Hi","text":"Hello"}]' | resend emails batch --file -Decided to move on with separate
--textand--text-fileoptions because otherwise we'd need a file indicator that would be less explicit (i.e. cURL uses@to indicate files). Opted for the more explicit approach for now.