Merged
Conversation
stainless-app
bot
force-pushed
the
release-please--branches--main--changes--next
branch
from
4b58a72 to
1839ad6
Compare
Contributor
Author
|
🤖 Release is at https://github.com/agentmail-to/agentmail-cli/releases/tag/v0.0.2 🌻 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
14 issues found across 89 files
Note: This PR contains a large number of files. cubic only reviews up to 75 files per PR, so some files may not have been reviewed.
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="pkg/cmd/apikey.go">
<violation number="1" location="pkg/cmd/apikey.go:60">
P2: The delete subcommand reuses the global `--api-key` flag name for the API key ID, which conflicts with the global auth flag and makes it impossible to pass both values on the command line. Use a distinct name like `--api-key-id` to avoid deleting the auth key by mistake.</violation>
</file>
<file name="internal/mocktest/mocktest.go">
<violation number="1" location="internal/mocktest/mocktest.go:87">
P2: Avoid spawning the `head` binary in tests; it is not available on Windows by default and will cause these tests to fail on non-Unix environments. Implement a Go-based line-limited reader instead.</violation>
</file>
<file name="internal/autocomplete/autocomplete.go">
<violation number="1" location="internal/autocomplete/autocomplete.go:352">
P2: When the current arg ends with “:”, the loop breaks without appending the next token, so inputs like ["a:", "b"] never rebuild to "a:b". This defeats the stated goal of rejoining colon-separated args.</violation>
</file>
<file name="pkg/cmd/cmdutil_unix.go">
<violation number="1" location="pkg/cmd/cmdutil_unix.go:48">
P2: `wstatus.ExitStatus()` is checked without first verifying that `Wait4` succeeded. If `Wait4` returns an error, `wstatus` may be unreliable. Check `err` from `Wait4` before inspecting `wstatus`.</violation>
<violation number="2" location="pkg/cmd/cmdutil_unix.go:71">
P1: After `os.NewFile(uintptr(parentFd), ...)` on line 85, the `os.File` owns the fd. The error-path cleanups on lines 94 and 100 call `unix.Close(parentFd)` directly, bypassing `parentConn`. When `parentConn` is later GC'd, its finalizer will close the same fd number again, risking corruption of an unrelated fd. Use `parentConn.Close()` instead of `unix.Close(parentFd)` on these error paths.</violation>
</file>
<file name="scripts/utils/upload-artifact.sh">
<violation number="1" location="scripts/utils/upload-artifact.sh:2">
P1: Avoid `set -x` here because it will log the Authorization bearer token from `AUTH`, leaking credentials in CI logs. Disable xtrace globally or at least around the curl commands.</violation>
</file>
<file name="internal/autocomplete/shellscripts/bash_autocomplete.bash">
<violation number="1" location="internal/autocomplete/shellscripts/bash_autocomplete.bash:49">
P3: Handle unexpected completion exit codes by clearing COMPREPLY; otherwise stale completions can be shown after an error.</violation>
</file>
<file name="pkg/cmd/cmdutil.go">
<violation number="1" location="pkg/cmd/cmdutil.go:322">
P2: The yaml branch in formatJSON writes to the output directly and returns nil bytes, which breaks ShowJSONIterator’s pagination and buffering logic (line counts stay zero and the output is emitted before the pager decision). Return the YAML bytes instead so the caller can paginate consistently.</violation>
</file>
<file name="internal/requestflag/requestflag.go">
<violation number="1" location="internal/requestflag/requestflag.go:133">
P1: `reflect.TypeOf(f.value).Kind()` always returns `reflect.Ptr` because `f.value` is `*cliValue[T]`, so the string and bool checks on lines 133 and 140 are always false. Empty-string values from env vars or file sources are silently dropped for string flags, and bool flags won't be defaulted to `false`. Use `reflect.TypeOf(f.Default).Kind()` to inspect the actual type parameter `T`.</violation>
</file>
<file name="internal/jsonview/explorer.go">
<violation number="1" location="internal/jsonview/explorer.go:396">
P2: Guard against empty tables before indexing rowData; empty arrays/objects will panic when printing the selected value.</violation>
<violation number="2" location="internal/jsonview/explorer.go:465">
P2: Toggling raw mode rebuilds views from stale `GetData()` snapshots, so rows loaded via streaming disappear after pressing `r`. Update the table’s backing data when loading more rows or rebuild from `rowData` when toggling raw.</violation>
</file>
<file name="pkg/cmd/cmd.go">
<violation number="1" location="pkg/cmd/cmd.go:301">
P2: Return the MkdirAll error instead of ignoring it so manpage generation fails fast with a clear message.</violation>
</file>
<file name="internal/apiquery/encoder.go">
<violation number="1" location="internal/apiquery/encoder.go:138">
P2: `encodePrimitive` skips `reflect.Int8`/`reflect.Uint8`, so `int8`/`uint8` (including `byte` values) are silently omitted from query encoding. Include the missing kinds in the integer cases.</violation>
</file>
<file name="pkg/cmd/flagoptions_test.go">
<violation number="1" location="pkg/cmd/flagoptions_test.go:229">
P3: The io.Reader subtest opens files via EmbedIOReader and discards the returned *os.File handles without closing them, which can leak file descriptors during the test run. Capture the returned value and close any *os.File instances after the assertion.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| // Use small buffer sizes so we don't ask the server for more paginated | ||
| // values than we actually need. | ||
| if err := unix.SetsockoptInt(parentFd, unix.SOL_SOCKET, unix.SO_SNDBUF, 128); err != nil { | ||
| unix.Close(parentFd) |
There was a problem hiding this comment.
P1: After os.NewFile(uintptr(parentFd), ...) on line 85, the os.File owns the fd. The error-path cleanups on lines 94 and 100 call unix.Close(parentFd) directly, bypassing parentConn. When parentConn is later GC'd, its finalizer will close the same fd number again, risking corruption of an unrelated fd. Use parentConn.Close() instead of unix.Close(parentFd) on these error paths.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/cmd/cmdutil_unix.go, line 71:
<comment>After `os.NewFile(uintptr(parentFd), ...)` on line 85, the `os.File` owns the fd. The error-path cleanups on lines 94 and 100 call `unix.Close(parentFd)` directly, bypassing `parentConn`. When `parentConn` is later GC'd, its finalizer will close the same fd number again, risking corruption of an unrelated fd. Use `parentConn.Close()` instead of `unix.Close(parentFd)` on these error paths.</comment>
<file context>
@@ -0,0 +1,116 @@
+ // Use small buffer sizes so we don't ask the server for more paginated
+ // values than we actually need.
+ if err := unix.SetsockoptInt(parentFd, unix.SOL_SOCKET, unix.SO_SNDBUF, 128); err != nil {
+ unix.Close(parentFd)
+ return nil, 0, err
+ }
</file context>
| @@ -0,0 +1,59 @@ | |||
| #!/usr/bin/env bash | |||
| set -exuo pipefail | |||
There was a problem hiding this comment.
P1: Avoid set -x here because it will log the Authorization bearer token from AUTH, leaking credentials in CI logs. Disable xtrace globally or at least around the curl commands.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/utils/upload-artifact.sh, line 2:
<comment>Avoid `set -x` here because it will log the Authorization bearer token from `AUTH`, leaking credentials in CI logs. Disable xtrace globally or at least around the curl commands.</comment>
<file context>
@@ -0,0 +1,59 @@
+#!/usr/bin/env bash
+set -exuo pipefail
+
+BINARY_NAME="agentmail"
</file context>
| func (f *Flag[T]) PostParse() error { | ||
| if !f.hasBeenSet { | ||
| if val, source, found := f.Sources.LookupWithSource(); found { | ||
| if val != "" || reflect.TypeOf(f.value).Kind() == reflect.String { |
There was a problem hiding this comment.
P1: reflect.TypeOf(f.value).Kind() always returns reflect.Ptr because f.value is *cliValue[T], so the string and bool checks on lines 133 and 140 are always false. Empty-string values from env vars or file sources are silently dropped for string flags, and bool flags won't be defaulted to false. Use reflect.TypeOf(f.Default).Kind() to inspect the actual type parameter T.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At internal/requestflag/requestflag.go, line 133:
<comment>`reflect.TypeOf(f.value).Kind()` always returns `reflect.Ptr` because `f.value` is `*cliValue[T]`, so the string and bool checks on lines 133 and 140 are always false. Empty-string values from env vars or file sources are silently dropped for string flags, and bool flags won't be defaulted to `false`. Use `reflect.TypeOf(f.Default).Kind()` to inspect the actual type parameter `T`.</comment>
<file context>
@@ -0,0 +1,671 @@
+func (f *Flag[T]) PostParse() error {
+ if !f.hasBeenSet {
+ if val, source, found := f.Sources.LookupWithSource(); found {
+ if val != "" || reflect.TypeOf(f.value).Kind() == reflect.String {
+ if err := f.Set(f.Name, val); err != nil {
+ return fmt.Errorf(
</file context>
| Suggest: true, | ||
| Flags: []cli.Flag{ | ||
| &requestflag.Flag[string]{ | ||
| Name: "api-key", |
There was a problem hiding this comment.
P2: The delete subcommand reuses the global --api-key flag name for the API key ID, which conflicts with the global auth flag and makes it impossible to pass both values on the command line. Use a distinct name like --api-key-id to avoid deleting the auth key by mistake.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/cmd/apikey.go, line 60:
<comment>The delete subcommand reuses the global `--api-key` flag name for the API key ID, which conflicts with the global auth flag and makes it impossible to pass both values on the command line. Use a distinct name like `--api-key-id` to avoid deleting the auth key by mistake.</comment>
<file context>
@@ -0,0 +1,160 @@
+ Suggest: true,
+ Flags: []cli.Flag{
+ &requestflag.Flag[string]{
+ Name: "api-key",
+ Usage: "ID of api key.",
+ Required: true,
</file context>
| // paginated or streamed endpoints. 100 lines of output should be enough to | ||
| // test that the API endpoint worked, or report back a meaningful amount of | ||
| // data if something went wrong. | ||
| headCmd := exec.Command("head", "-n", "100") |
There was a problem hiding this comment.
P2: Avoid spawning the head binary in tests; it is not available on Windows by default and will cause these tests to fail on non-Unix environments. Implement a Go-based line-limited reader instead.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At internal/mocktest/mocktest.go, line 87:
<comment>Avoid spawning the `head` binary in tests; it is not available on Windows by default and will cause these tests to fail on non-Unix environments. Implement a Go-based line-limited reader instead.</comment>
<file context>
@@ -0,0 +1,127 @@
+ // paginated or streamed endpoints. 100 lines of output should be enough to
+ // test that the API endpoint worked, or report back a meaningful amount of
+ // data if something went wrong.
+ headCmd := exec.Command("head", "-n", "100")
+ pipe, err := cliCmd.StdoutPipe()
+ require.NoError(t, err, "Failed to create pipe for CLI command")
</file context>
| return v.current().GetData().Raw | ||
| } | ||
|
|
||
| selected := tableView.rowData[tableView.table.Cursor()] |
There was a problem hiding this comment.
P2: Guard against empty tables before indexing rowData; empty arrays/objects will panic when printing the selected value.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At internal/jsonview/explorer.go, line 396:
<comment>Guard against empty tables before indexing rowData; empty arrays/objects will panic when printing the selected value.</comment>
<file context>
@@ -0,0 +1,775 @@
+ return v.current().GetData().Raw
+ }
+
+ selected := tableView.rowData[tableView.table.Cursor()]
+ if selected.Type == gjson.String {
+ return selected.String()
</file context>
|
|
||
| func generateManpages(ctx context.Context, c *cli.Command) error { | ||
| manpage, err := docs.ToManWithSection(Command, 1) | ||
| if err != nil { |
There was a problem hiding this comment.
P2: Return the MkdirAll error instead of ignoring it so manpage generation fails fast with a clear message.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/cmd/cmd.go, line 301:
<comment>Return the MkdirAll error instead of ignoring it so manpage generation fails fast with a clear message.</comment>
<file context>
@@ -0,0 +1,334 @@
+
+func generateManpages(ctx context.Context, c *cli.Command) error {
+ manpage, err := docs.ToManWithSection(Command, 1)
+ if err != nil {
+ return err
+ }
</file context>
| } | ||
| return []Pair{{key, "false"}}, nil | ||
|
|
||
| case reflect.Int, reflect.Int16, reflect.Int32, reflect.Int64: |
There was a problem hiding this comment.
P2: encodePrimitive skips reflect.Int8/reflect.Uint8, so int8/uint8 (including byte values) are silently omitted from query encoding. Include the missing kinds in the integer cases.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At internal/apiquery/encoder.go, line 138:
<comment>`encodePrimitive` skips `reflect.Int8`/`reflect.Uint8`, so `int8`/`uint8` (including `byte` values) are silently omitted from query encoding. Include the missing kinds in the integer cases.</comment>
<file context>
@@ -0,0 +1,166 @@
+ }
+ return []Pair{{key, "false"}}, nil
+
+ case reflect.Int, reflect.Int16, reflect.Int32, reflect.Int64:
+ return []Pair{{key, strconv.FormatInt(value.Int(), 10)}}, nil
+
</file context>
| if [[ "$force_file_completion" == true ]]; then | ||
| mapfile -t COMPREPLY < <(compgen -f -- "$file_part" | sed "s|^|$prefix|") | ||
| else | ||
| case $exit_code in |
There was a problem hiding this comment.
P3: Handle unexpected completion exit codes by clearing COMPREPLY; otherwise stale completions can be shown after an error.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At internal/autocomplete/shellscripts/bash_autocomplete.bash, line 49:
<comment>Handle unexpected completion exit codes by clearing COMPREPLY; otherwise stale completions can be shown after an error.</comment>
<file context>
@@ -0,0 +1,59 @@
+ if [[ "$force_file_completion" == true ]]; then
+ mapfile -t COMPREPLY < <(compgen -f -- "$file_part" | sed "s|^|$prefix|")
+ else
+ case $exit_code in
+ 10) mapfile -t COMPREPLY < <(compgen -f -- "$cur") ;; # file completion
+ 11) COMPREPLY=() ;; # no completion
</file context>
| }) | ||
|
|
||
| t.Run(tt.name+" io.Reader", func(t *testing.T) { | ||
| _, err := embedFiles(tt.input, EmbedIOReader) |
There was a problem hiding this comment.
P3: The io.Reader subtest opens files via EmbedIOReader and discards the returned *os.File handles without closing them, which can leak file descriptors during the test run. Capture the returned value and close any *os.File instances after the assertion.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/cmd/flagoptions_test.go, line 229:
<comment>The io.Reader subtest opens files via EmbedIOReader and discards the returned *os.File handles without closing them, which can leak file descriptors during the test run. Capture the returned value and close any *os.File instances after the assertion.</comment>
<file context>
@@ -0,0 +1,244 @@
+ })
+
+ t.Run(tt.name+" io.Reader", func(t *testing.T) {
+ _, err := embedFiles(tt.input, EmbedIOReader)
+ if tt.wantErr {
+ assert.Error(t, err)
</file context>
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
1 participant
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.
Automated Release PR
0.0.2 (2026-03-02)
Full Changelog: v0.0.1...v0.0.2
Chores
This pull request is managed by Stainless's GitHub App.
The semver version number is based on included commit messages. Alternatively, you can manually set the version number in the title of this pull request.
For a better experience, it is recommended to use either rebase-merge or squash-merge when merging this pull request.
🔗 Stainless website
📚 Read the docs
🙋 Reach out for help or questions
Summary by cubic
Release v0.0.2 ships the first Agentmail CLI with core commands, rich output (including an interactive JSON explorer), shell completion, and CI-driven releases.
New Features
Release & CI
Written for commit 1839ad6. Summary will update on new commits.