Add Inbound Email API commands - #10
Merged
Merged
Conversation
📝 WalkthroughWalkthroughThe CLI adds inbound folders, inboxes, messages, and threads commands. It adds shared email address parsing, inbound webhook routing, related domain and email-log fields, integration tests, and documentation. ChangesInbound CLI support
Estimated code review effort: 4 (Complex) | ~60 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant CLI
participant InboundAPI
participant Output
CLI->>InboundAPI: Execute inbound command
InboundAPI-->>CLI: Return resource or send result
CLI->>Output: Render table or JSON
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
internal/commands/inbound/messages/send.go (1)
53-73: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider collapsing the repeated to/cc/bcc parsing blocks.
The
to,cc, andbccblocks repeat the same pattern: check length, callcmdutil.ParseEmailAddrs, wrap the error, and assign tobody. A small loop over the three fields removes the repetition without changing behavior.♻️ Proposed refactor
- if len(s.to) > 0 { - addrs, err := cmdutil.ParseEmailAddrs(s.to) - if err != nil { - return nil, fmt.Errorf("invalid --to address: %w", err) - } - body["to"] = addrs - } - if len(s.cc) > 0 { - addrs, err := cmdutil.ParseEmailAddrs(s.cc) - if err != nil { - return nil, fmt.Errorf("invalid --cc address: %w", err) - } - body["cc"] = addrs - } - if len(s.bcc) > 0 { - addrs, err := cmdutil.ParseEmailAddrs(s.bcc) - if err != nil { - return nil, fmt.Errorf("invalid --bcc address: %w", err) - } - body["bcc"] = addrs - } + for _, field := range []struct { + name string + values []string + }{{"to", s.to}, {"cc", s.cc}, {"bcc", s.bcc}} { + if len(field.values) == 0 { + continue + } + addrs, err := cmdutil.ParseEmailAddrs(field.values) + if err != nil { + return nil, fmt.Errorf("invalid --%s address: %w", field.name, err) + } + body[field.name] = addrs + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/commands/inbound/messages/send.go` around lines 53 - 73, Collapse the repeated address parsing in the send command into a loop over the to, cc, and bcc fields, preserving each field’s existing empty check, cmdutil.ParseEmailAddrs call, error context, and body assignment. Update the surrounding request-building method rather than changing parsing behavior.internal/commands/inbound/messages/messages_test.go (1)
126-158: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd test coverage for
reply-all.
NewCmdReplyAllposts to a distinct/reply_allendpoint but has no automated test and no manual test-plan row, unlikereplyandforward, which both have coverage in each artifact.
internal/commands/inbound/messages/messages_test.go#L126-L158: add aTestMessagesReplyAlltest mirroringTestMessagesReply, asserting the POST path ends in/reply_alland the request body and response are handled correctly.docs/TEST_PLAN.md#L338-L364: add a table row formailtrap inbound messages reply-all --inbox-id <INBOX_ID> --id <MESSAGE_ID> --text "..."between rows 22.13 (Reply) and 22.14 (Forward), consistent with the note on line 340 that already describes reply-all's behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/commands/inbound/messages/messages_test.go` around lines 126 - 158, Add TestMessagesReplyAll in internal/commands/inbound/messages/messages_test.go, mirroring TestMessagesReply while asserting the POST path ends with /reply_all and validating the request body, response handling, and output message ID. Add the corresponding mailtrap inbound messages reply-all command row in docs/TEST_PLAN.md between rows 22.13 and 22.14.internal/commands/inbound/messages/reply.go (1)
13-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared reply/reply-all/forward command boilerplate.
reply.go,reply_all.go, andforward.goeach implement the same command shape: validateinbox-idandid, build a client, callsend.body(), POST to an inbound endpoint, and print the result withsendResultColumns. The only differences are theUse/Shortstrings, the endpoint suffix, and the extra--tocheck inforward.go. A shared constructor removes this duplication and keeps future endpoint or flag changes in one place.
internal/commands/inbound/messages/reply.go#L13-L57: replace the body ofNewCmdReplywith a call to a shared helper, e.g.newSendActionCmd(f, "reply", "Reply to an inbound message (sends a real email to the original sender)", "reply", false).internal/commands/inbound/messages/reply_all.go#L13-L57: replace the body ofNewCmdReplyAllwith the same helper, passing"reply-all", the reply-allShorttext, and endpoint suffix"reply_all".internal/commands/inbound/messages/forward.go#L13-L60: replace the body ofNewCmdForwardwith the same helper, passing"forward", the forwardShorttext, endpoint suffix"forward", andrequireTo=trueto keep the--to is requiredcheck.♻️ Proposed shared helper (place in send.go)
func newSendActionCmd(f *cmdutil.Factory, use, short, endpointSuffix string, requireTo bool) *cobra.Command { var ( inboxID string messageID string send sendFlags ) cmd := &cobra.Command{ Use: use, Short: short, RunE: func(cmd *cobra.Command, args []string) error { if err := cmdutil.RequireFlag("inbox-id", inboxID); err != nil { return err } if err := cmdutil.RequireFlag("id", messageID); err != nil { return err } if requireTo && len(send.to) == 0 { return fmt.Errorf("--to is required") } c, err := f.NewClient() if err != nil { return err } body, err := send.body() if err != nil { return err } path := fmt.Sprintf("/api/inbound/inboxes/%s/messages/%s/%s", inboxID, messageID, endpointSuffix) var resp SendMessageResult if err := c.Post(context.Background(), client.BaseGeneral, path, body, &resp); err != nil { return err } return output.Print(f.IOStreams.Out, cmdutil.GetOutputFormat(), resp, sendResultColumns) }, } cmd.Flags().StringVar(&inboxID, "inbox-id", "", "Inbox ID (required)") cmd.Flags().StringVar(&messageID, "id", "", "Message ID (required)") send.register(cmd) return cmd }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/commands/inbound/messages/reply.go` around lines 13 - 57, Extract the duplicated command flow into a shared newSendActionCmd helper in send.go, preserving flag validation, client creation, body generation, POST, and output formatting. Update internal/commands/inbound/messages/reply.go lines 13-57 to call it with reply metadata and requireTo=false; update internal/commands/inbound/messages/reply_all.go lines 13-57 with reply-all metadata and endpoint suffix reply_all; update internal/commands/inbound/messages/forward.go lines 13-60 with forward metadata, endpoint suffix forward, and requireTo=true so the existing --to validation remains.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/cmdutil/address.go`:
- Around line 15-33: Update ParseEmailAddr to validate the extracted email with
net/mail.ParseAddress or equivalent before returning it, rejecting malformed
plain addresses and empty angle-bracket addresses while preserving supported
“Name <email>” parsing; add tests covering invalid values such as “not-an-email”
and “Name <>”.
In `@internal/commands/inbound/messages/list.go`:
- Around line 63-84: Both inbound list commands discard the pagination cursor,
so users cannot discover the next --last-id value. In
internal/commands/inbound/messages/list.go lines 63-84, update the messages list
command after output.Print to emit resp.LastID, using the established output
behavior for non-JSON formats; apply the same resp.LastID output change in
internal/commands/inbound/threads/list.go lines 69-90 for the threads list
command, while preserving the existing data output and JSON behavior.
In `@internal/commands/webhooks/create.go`:
- Around line 94-100: The webhook CLI help text in the flag definitions should
match the API contract: update the `--type` description to include `campaigns`,
and update the `--domain-id` description to indicate it applies to both
`email_sending` and `campaigns` webhooks. Modify only these descriptions in the
command flag setup.
---
Nitpick comments:
In `@internal/commands/inbound/messages/messages_test.go`:
- Around line 126-158: Add TestMessagesReplyAll in
internal/commands/inbound/messages/messages_test.go, mirroring TestMessagesReply
while asserting the POST path ends with /reply_all and validating the request
body, response handling, and output message ID. Add the corresponding mailtrap
inbound messages reply-all command row in docs/TEST_PLAN.md between rows 22.13
and 22.14.
In `@internal/commands/inbound/messages/reply.go`:
- Around line 13-57: Extract the duplicated command flow into a shared
newSendActionCmd helper in send.go, preserving flag validation, client creation,
body generation, POST, and output formatting. Update
internal/commands/inbound/messages/reply.go lines 13-57 to call it with reply
metadata and requireTo=false; update
internal/commands/inbound/messages/reply_all.go lines 13-57 with reply-all
metadata and endpoint suffix reply_all; update
internal/commands/inbound/messages/forward.go lines 13-60 with forward metadata,
endpoint suffix forward, and requireTo=true so the existing --to validation
remains.
In `@internal/commands/inbound/messages/send.go`:
- Around line 53-73: Collapse the repeated address parsing in the send command
into a loop over the to, cc, and bcc fields, preserving each field’s existing
empty check, cmdutil.ParseEmailAddrs call, error context, and body assignment.
Update the surrounding request-building method rather than changing parsing
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 91c9b5fc-15f5-430e-bf94-183ac5c03ad8
📒 Files selected for processing (43)
README.mdcmd/root.godocs/TEST_PLAN.mdinternal/cmdutil/address.gointernal/cmdutil/address_test.gointernal/commands/domains/list.gointernal/commands/email_logs/list.gointernal/commands/inbound/folders/create.gointernal/commands/inbound/folders/delete.gointernal/commands/inbound/folders/folders.gointernal/commands/inbound/folders/folders_test.gointernal/commands/inbound/folders/get.gointernal/commands/inbound/folders/list.gointernal/commands/inbound/folders/update.gointernal/commands/inbound/inbound.gointernal/commands/inbound/inboxes/create.gointernal/commands/inbound/inboxes/delete.gointernal/commands/inbound/inboxes/get.gointernal/commands/inbound/inboxes/inboxes.gointernal/commands/inbound/inboxes/inboxes_test.gointernal/commands/inbound/inboxes/list.gointernal/commands/inbound/inboxes/update.gointernal/commands/inbound/messages/delete.gointernal/commands/inbound/messages/forward.gointernal/commands/inbound/messages/get.gointernal/commands/inbound/messages/list.gointernal/commands/inbound/messages/messages.gointernal/commands/inbound/messages/messages_test.gointernal/commands/inbound/messages/reply.gointernal/commands/inbound/messages/reply_all.gointernal/commands/inbound/messages/send.gointernal/commands/inbound/threads/delete.gointernal/commands/inbound/threads/get.gointernal/commands/inbound/threads/list.gointernal/commands/inbound/threads/threads.gointernal/commands/inbound/threads/threads_test.gointernal/commands/send/send.gointernal/commands/send/transactional.gointernal/commands/webhooks/create.gointernal/commands/webhooks/list.gointernal/commands/webhooks/update.goskills/mailtrap-cli/SKILL.mdskills/mailtrap-cli/references/inbound.md
💤 Files with no reviewable changes (1)
- internal/commands/send/send.go
leonid-shevtsov
approved these changes
Aug 4, 2026
Add the inbound command group with folders and inboxes subcommands (list/get/create/update/delete). Inbound endpoints are token-scoped under /api/inbound, so paths are built directly rather than via the account path helper. Register the group in the root command.
Add the inbound messages group (list/get/delete plus reply, reply-all,
and forward) and the threads group (list/get/delete). Message and thread
list paginate via a manual --last-id cursor flag, matching the
email-logs command.
Extract the shared email-address parser ('Name <email>' / 'email') into
cmdutil and reuse it in the send command and for reply/reply-all/forward,
so inbound recipient parsing matches send. Wire both groups into the
inbound command.
Add --inbound-inbox-id to webhooks create/update and the inbound_receiving type; add inbound_enabled/inbound_verified to the domain output; add the inbound threading fields (rfc_message_id, in_reply_to, references, thread_id) to the email-log output.
Add tests for the inbound folders, inboxes, messages, and threads commands using an httptest server, asserting the request method, token-scoped /api/inbound path, request bodies, and rendered output.
Add the inbound command reference (skills/mailtrap-cli/references/inbound.md), a Commands-table row and usage examples in the README, an Inbound section in the test plan, and the inbound entry in the skill's command-group table.
Include the campaigns webhook type in --type, and correct --domain-id to note it scopes email_sending and campaigns (not email_sending only).
inbound messages/threads list and email-logs list now print the next page cursor (--last-id / --cursor) after the table so it is discoverable for the next request. JSON output is unchanged.
IgorDobryn
approved these changes
Aug 4, 2026
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
internal/commands/inbound/messages/messages_test.go (1)
129-161: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd an integration test for
reply-all.
TestMessagesReplytests onlyreply. The PR also addsreply-all, but this file has no test for that command. Add a test that verifies the expected request route, request body, and response output forreply-all.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/commands/inbound/messages/messages_test.go` around lines 129 - 161, Add a dedicated integration test alongside TestMessagesReply for the messages reply-all command, using the existing setupTest and command execution patterns. Verify it sends the expected POST route and request body, then assert Execute succeeds and the response message ID appears in command output.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/commands/email_logs/list.go`:
- Around line 119-125: Update the pagination notice write in the command’s
output flow to handle the error returned by fmt.Fprintf instead of ignoring it.
Return that write error when printing the “Next page” cursor, while preserving
the existing output.Print handling and nil-success behavior when no pagination
notice is emitted.
In `@internal/commands/inbound/inboxes/inboxes_test.go`:
- Around line 128-130: Check and handle errors from io.ReadAll and
json.Unmarshal in the create request block at
internal/commands/inbound/inboxes/inboxes_test.go:128-130, reporting each error
and returning before assertions. Apply the same error handling to the update
request block at internal/commands/inbound/inboxes/inboxes_test.go:237-239; both
sites require direct changes.
- Around line 56-59: Check each mock response json.Encoder.Encode call in
internal/commands/inbound/inboxes/inboxes_test.go at lines 56-59, 99-102,
139-141, 185-188, and 244-247, and report any returned error with t.Errorf so
encoding failures directly fail the corresponding test.
In `@internal/commands/inbound/messages/messages_test.go`:
- Around line 48-50: Replace suffix-based URL assertions with exact r.URL.Path
comparisons for the list, get, reply, forward, and delete endpoint tests in
internal/commands/inbound/messages/messages_test.go at lines 48-50, 105-107,
134-136, 165-167, and 220-222, preserving each test’s complete expected API
path.
---
Nitpick comments:
In `@internal/commands/inbound/messages/messages_test.go`:
- Around line 129-161: Add a dedicated integration test alongside
TestMessagesReply for the messages reply-all command, using the existing
setupTest and command execution patterns. Verify it sends the expected POST
route and request body, then assert Execute succeeds and the response message ID
appears in command output.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2c2d67b1-e2f3-459a-aac1-d821a8624b22
📒 Files selected for processing (44)
README.mdcmd/root.godocs/TEST_PLAN.mdinternal/cmdutil/address.gointernal/cmdutil/address_test.gointernal/commands/domains/list.gointernal/commands/email_logs/email_logs_test.gointernal/commands/email_logs/list.gointernal/commands/inbound/folders/create.gointernal/commands/inbound/folders/delete.gointernal/commands/inbound/folders/folders.gointernal/commands/inbound/folders/folders_test.gointernal/commands/inbound/folders/get.gointernal/commands/inbound/folders/list.gointernal/commands/inbound/folders/update.gointernal/commands/inbound/inbound.gointernal/commands/inbound/inboxes/create.gointernal/commands/inbound/inboxes/delete.gointernal/commands/inbound/inboxes/get.gointernal/commands/inbound/inboxes/inboxes.gointernal/commands/inbound/inboxes/inboxes_test.gointernal/commands/inbound/inboxes/list.gointernal/commands/inbound/inboxes/update.gointernal/commands/inbound/messages/delete.gointernal/commands/inbound/messages/forward.gointernal/commands/inbound/messages/get.gointernal/commands/inbound/messages/list.gointernal/commands/inbound/messages/messages.gointernal/commands/inbound/messages/messages_test.gointernal/commands/inbound/messages/reply.gointernal/commands/inbound/messages/reply_all.gointernal/commands/inbound/messages/send.gointernal/commands/inbound/threads/delete.gointernal/commands/inbound/threads/get.gointernal/commands/inbound/threads/list.gointernal/commands/inbound/threads/threads.gointernal/commands/inbound/threads/threads_test.gointernal/commands/send/send.gointernal/commands/send/transactional.gointernal/commands/webhooks/create.gointernal/commands/webhooks/list.gointernal/commands/webhooks/update.goskills/mailtrap-cli/SKILL.mdskills/mailtrap-cli/references/inbound.md
💤 Files with no reviewable changes (1)
- internal/commands/send/send.go
🚧 Files skipped from review as they are similar to previous changes (39)
- internal/commands/inbound/folders/create.go
- internal/commands/inbound/inboxes/inboxes.go
- internal/commands/inbound/messages/messages.go
- cmd/root.go
- internal/commands/inbound/inboxes/get.go
- internal/commands/inbound/folders/list.go
- internal/commands/domains/list.go
- internal/commands/webhooks/update.go
- internal/commands/inbound/inboxes/list.go
- internal/commands/inbound/folders/update.go
- README.md
- internal/commands/webhooks/create.go
- internal/commands/inbound/messages/list.go
- skills/mailtrap-cli/SKILL.md
- internal/commands/inbound/messages/delete.go
- internal/commands/inbound/threads/get.go
- internal/commands/inbound/messages/reply.go
- internal/commands/inbound/threads/threads.go
- internal/commands/inbound/inboxes/update.go
- internal/commands/webhooks/list.go
- internal/commands/inbound/messages/send.go
- internal/commands/inbound/messages/get.go
- internal/commands/inbound/inboxes/create.go
- internal/commands/inbound/threads/delete.go
- internal/commands/inbound/messages/forward.go
- skills/mailtrap-cli/references/inbound.md
- internal/commands/inbound/inbound.go
- internal/commands/inbound/threads/list.go
- internal/commands/inbound/messages/reply_all.go
- internal/cmdutil/address_test.go
- docs/TEST_PLAN.md
- internal/commands/inbound/folders/get.go
- internal/commands/inbound/folders/delete.go
- internal/commands/inbound/threads/threads_test.go
- internal/commands/send/transactional.go
- internal/commands/inbound/folders/folders_test.go
- internal/cmdutil/address.go
- internal/commands/inbound/folders/folders.go
- internal/commands/inbound/inboxes/delete.go
Comment on lines
+119
to
+125
| if err := output.Print(f.IOStreams.Out, format, resp.Messages, emailLogColumns); err != nil { | ||
| return err | ||
| } | ||
| if format != output.FormatJSON && resp.NextPageCursor != "" { | ||
| fmt.Fprintf(f.IOStreams.Out, "\nNext page: --cursor %s\n", resp.NextPageCursor) | ||
| } | ||
| return nil |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Propagate pagination output errors.
fmt.Fprintf can fail after output.Print succeeds, but the command ignores that error and returns success. Return the write error so broken pipes and other output failures are reported.
Proposed fix
- fmt.Fprintf(f.IOStreams.Out, "\nNext page: --cursor %s\n", resp.NextPageCursor)
+ if _, err := fmt.Fprintf(f.IOStreams.Out, "\nNext page: --cursor %s\n", resp.NextPageCursor); err != nil {
+ return err
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if err := output.Print(f.IOStreams.Out, format, resp.Messages, emailLogColumns); err != nil { | |
| return err | |
| } | |
| if format != output.FormatJSON && resp.NextPageCursor != "" { | |
| fmt.Fprintf(f.IOStreams.Out, "\nNext page: --cursor %s\n", resp.NextPageCursor) | |
| } | |
| return nil | |
| if err := output.Print(f.IOStreams.Out, format, resp.Messages, emailLogColumns); err != nil { | |
| return err | |
| } | |
| if format != output.FormatJSON && resp.NextPageCursor != "" { | |
| if _, err := fmt.Fprintf(f.IOStreams.Out, "\nNext page: --cursor %s\n", resp.NextPageCursor); err != nil { | |
| return err | |
| } | |
| } | |
| return nil |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/commands/email_logs/list.go` around lines 119 - 125, Update the
pagination notice write in the command’s output flow to handle the error
returned by fmt.Fprintf instead of ignoring it. Return that write error when
printing the “Next page” cursor, while preserving the existing output.Print
handling and nil-success behavior when no pagination notice is emitted.
Comment on lines
+56
to
+59
| json.NewEncoder(w).Encode([]map[string]interface{}{ | ||
| {"id": 201, "name": "Support inbox", "address": "support@inbound-mailtrap.io"}, | ||
| {"id": 202, "name": "Catch-all", "address": "catch-all@example.com", "domain_id": 6}, | ||
| }) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Check every mock response encoding error.
The unchecked Encode calls violate errcheck. Report the error through t.Errorf so a failed mock response has a direct test failure.
internal/commands/inbound/inboxes/inboxes_test.go#L56-L59: check the list response encoding error.internal/commands/inbound/inboxes/inboxes_test.go#L99-L102: check the get response encoding error.internal/commands/inbound/inboxes/inboxes_test.go#L139-L141: check the create response encoding error.internal/commands/inbound/inboxes/inboxes_test.go#L185-L188: check the JSON list response encoding error.internal/commands/inbound/inboxes/inboxes_test.go#L244-L247: check the update response encoding error.
Proposed fix pattern
- json.NewEncoder(w).Encode(response)
+ if err := json.NewEncoder(w).Encode(response); err != nil {
+ t.Errorf("encode response: %v", err)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| json.NewEncoder(w).Encode([]map[string]interface{}{ | |
| {"id": 201, "name": "Support inbox", "address": "support@inbound-mailtrap.io"}, | |
| {"id": 202, "name": "Catch-all", "address": "catch-all@example.com", "domain_id": 6}, | |
| }) | |
| if err := json.NewEncoder(w).Encode([]map[string]interface{}{ | |
| {"id": 201, "name": "Support inbox", "address": "support@inbound-mailtrap.io"}, | |
| {"id": 202, "name": "Catch-all", "address": "catch-all@example.com", "domain_id": 6}, | |
| }); err != nil { | |
| t.Errorf("encode response: %v", err) | |
| } |
🧰 Tools
🪛 golangci-lint (2.12.2)
[error] 56-56: Error return value of (*encoding/json.Encoder).Encode is not checked
(errcheck)
📍 Affects 1 file
internal/commands/inbound/inboxes/inboxes_test.go#L56-L59(this comment)internal/commands/inbound/inboxes/inboxes_test.go#L99-L102internal/commands/inbound/inboxes/inboxes_test.go#L139-L141internal/commands/inbound/inboxes/inboxes_test.go#L185-L188internal/commands/inbound/inboxes/inboxes_test.go#L244-L247
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/commands/inbound/inboxes/inboxes_test.go` around lines 56 - 59,
Check each mock response json.Encoder.Encode call in
internal/commands/inbound/inboxes/inboxes_test.go at lines 56-59, 99-102,
139-141, 185-188, and 244-247, and report any returned error with t.Errorf so
encoding failures directly fail the corresponding test.
Source: Linters/SAST tools
Comment on lines
+128
to
+130
| body, _ := io.ReadAll(r.Body) | ||
| var reqBody map[string]interface{} | ||
| json.Unmarshal(body, &reqBody) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Check request body read and JSON decode errors.
The handlers continue after request decoding fails. Report the error and return before assertions so the test identifies the failed operation.
internal/commands/inbound/inboxes/inboxes_test.go#L128-L130: checkio.ReadAllandjson.Unmarshalfor the create request.internal/commands/inbound/inboxes/inboxes_test.go#L237-L239: checkio.ReadAllandjson.Unmarshalfor the update request.
Proposed fix pattern
- body, _ := io.ReadAll(r.Body)
+ body, err := io.ReadAll(r.Body)
+ if err != nil {
+ t.Errorf("read request body: %v", err)
+ return
+ }
var reqBody map[string]interface{}
- json.Unmarshal(body, &reqBody)
+ if err := json.Unmarshal(body, &reqBody); err != nil {
+ t.Errorf("decode request body: %v", err)
+ return
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| body, _ := io.ReadAll(r.Body) | |
| var reqBody map[string]interface{} | |
| json.Unmarshal(body, &reqBody) | |
| body, err := io.ReadAll(r.Body) | |
| if err != nil { | |
| t.Errorf("read request body: %v", err) | |
| return | |
| } | |
| var reqBody map[string]interface{} | |
| if err := json.Unmarshal(body, &reqBody); err != nil { | |
| t.Errorf("decode request body: %v", err) | |
| return | |
| } |
🧰 Tools
🪛 golangci-lint (2.12.2)
[error] 130-130: Error return value of json.Unmarshal is not checked
(errcheck)
📍 Affects 1 file
internal/commands/inbound/inboxes/inboxes_test.go#L128-L130(this comment)internal/commands/inbound/inboxes/inboxes_test.go#L237-L239
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/commands/inbound/inboxes/inboxes_test.go` around lines 128 - 130,
Check and handle errors from io.ReadAll and json.Unmarshal in the create request
block at internal/commands/inbound/inboxes/inboxes_test.go:128-130, reporting
each error and returning before assertions. Apply the same error handling to the
update request block at
internal/commands/inbound/inboxes/inboxes_test.go:237-239; both sites require
direct changes.
Source: Linters/SAST tools
Comment on lines
+48
to
+50
| if !strings.HasSuffix(r.URL.Path, "/api/inbound/inboxes/201/messages") { | ||
| t.Errorf("unexpected path: %s", r.URL.Path) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Compare complete API paths in endpoint tests.
strings.HasSuffix accepts an unexpected path prefix. This can let an incorrect client route pass these tests. Compare r.URL.Path with the complete expected path.
internal/commands/inbound/messages/messages_test.go#L48-L50: compare the list path exactly.internal/commands/inbound/messages/messages_test.go#L105-L107: compare the get path exactly.internal/commands/inbound/messages/messages_test.go#L134-L136: compare the reply path exactly.internal/commands/inbound/messages/messages_test.go#L165-L167: compare the forward path exactly.internal/commands/inbound/messages/messages_test.go#L220-L222: compare the delete path exactly.
📍 Affects 1 file
internal/commands/inbound/messages/messages_test.go#L48-L50(this comment)internal/commands/inbound/messages/messages_test.go#L105-L107internal/commands/inbound/messages/messages_test.go#L134-L136internal/commands/inbound/messages/messages_test.go#L165-L167internal/commands/inbound/messages/messages_test.go#L220-L222
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/commands/inbound/messages/messages_test.go` around lines 48 - 50,
Replace suffix-based URL assertions with exact r.URL.Path comparisons for the
list, get, reply, forward, and delete endpoint tests in
internal/commands/inbound/messages/messages_test.go at lines 48-50, 105-107,
134-136, 165-167, and 220-222, preserving each test’s complete expected API
path.
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
3 participants
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.
Summary
Adds Inbound Email API support to the CLI, matching the released Node, Ruby, MCP, PHP, Python, Java, and .NET SDKs. Exposed as
mailtrap inbound …— the commands are not account-scoped (no--account-id); they go tohttps://mailtrap.io/api/inbound/....Also surfaces the other fields that ship with the Inbound Email API:
webhooks create/update: new--inbound-inbox-idflag and theinbound_receivingwebhook typedomains:inbound_enabled/inbound_verifiedemail-logs: inbound threading fields (rfc_message_id,in_reply_to,references,thread_id)Notes
/api/inbound/...onBaseGeneral(noAccountPath, noRequireAccountID). Inbox management is folder-scoped; messages and threads are accessed via the top-level inbox route.{data, total_count, last_id}(paginated via--last-id); reply/forward return{message_ids}.reply/reply-all/forwardreuse thesendcommand's address parsing (extracted intocmdutil), so recipients acceptName <email>oremail;forwardrequires--to. There is no--subject(the API derives it).sdk-dev/references/cli.md: subcommand factory,RequireFlag, PATCHChanged()-gating, per-spec response wrappers, single column slice, help-text style.Summary by CodeRabbit