Support attachments in hey compose#129
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds first-class attachment support to hey compose, enabling users to pass repeatable -a/--attachment flags that are uploaded via Rails Active Storage direct uploads and embedded into the message body as ActionText attachment markup (for both new messages and --thread-id replies).
Changes:
- Add repeatable
-a/--attachmentflag tohey compose, validate paths, upload files, and append ActionText markup before send. - Implement an SDK-backed Active Storage direct-upload creator + response parsing.
- Add unit tests for direct upload parsing/creation and attachment validation/markup behavior; update README and
.surfacehelp surface.
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Document compose attachment usage with realistic examples. |
| internal/cmd/compose.go | Wire --attachment into hey compose send flow via attachFiles. |
| internal/cmd/compose_attachments.go | Add Active Storage direct-upload creation and response parsing. |
| internal/cmd/compose_attachments_test.go | Add tests for parsing, creator request body, and compose attachment validation behavior. |
| internal/attachments/attachments.go | New attachments package: validate, upload (direct upload + PUT), and render ActionText markup. |
| internal/attachments/attachments_test.go | Test validation, upload flow, error handling, and markup escaping/append behavior. |
| .surface | Expose hey compose --attachment in surfaced command list. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ctx := cmd.Context() | ||
|
|
||
| message, err := c.attachFiles(ctx, message) | ||
| if err != nil { | ||
| return err | ||
| } |
| import ( | ||
| "context" | ||
| "encoding/json" | ||
| "fmt" | ||
| "net/http" | ||
|
|
||
| hey "github.com/basecamp/hey-sdk/go/pkg/hey" | ||
|
|
||
| "github.com/basecamp/hey-cli/internal/attachments" | ||
| "github.com/basecamp/hey-cli/internal/output" | ||
| ) | ||
|
|
||
| // directUploadsPath is HEY's Active Storage direct-upload endpoint. | ||
| const directUploadsPath = "/rails/active_storage/direct_uploads.json" | ||
|
|
||
| // newAttachmentUploader builds an uploader backed by the SDK client. Blob bytes | ||
| // are PUT to a self-authenticating Active Storage URL, so the transfer uses a | ||
| // plain HTTP client with no HEY credentials. | ||
| func newAttachmentUploader() *attachments.Uploader { | ||
| return attachments.NewUploader(&sdkDirectUploadCreator{client: sdk}, http.DefaultClient) | ||
| } |
| if payload.DirectUpload.URL == "" { | ||
| return nil, output.ErrAPI(0, "direct upload response missing upload URL") | ||
| } |
| data, err := os.ReadFile(path) //nolint:gosec // path is user-provided by design | ||
| if err != nil { | ||
| return nil, apierr.ErrUsage(fmt.Sprintf("cannot read attachment %s: %v", path, err)) | ||
| } |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f8688e0e33
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if upload.AttachableSGID != "" { | ||
| return upload.AttachableSGID | ||
| } | ||
| return upload.SignedID |
There was a problem hiding this comment.
For direct-upload responses that only contain the standard Active Storage signed_id, this fallback writes that blob signed ID into the sgid attribute generated by Markup(). ActionText resolves sgid as a SignedGlobalID, not an Active Storage signed blob ID, so those messages save with an attachment tag that cannot resolve/render and the uploaded blob remains unattached. Please treat a missing attachable_sgid as an API error or obtain the real attachable SGID instead of falling back.
Useful? React with 👍 / 👎.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
6 issues found across 7 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="internal/cmd/compose.go">
<violation number="1" location="internal/cmd/compose.go:131">
P2: Failed uploads and send failures leave orphaned Active Storage blobs with no cleanup mechanism.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| // upload or a sent message. | ||
| for _, path := range c.attachments { | ||
| if err := attachments.Validate(path); err != nil { | ||
| return "", err |
There was a problem hiding this comment.
P2: Failed uploads and send failures leave orphaned Active Storage blobs with no cleanup mechanism.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At internal/cmd/compose.go, line 131:
<comment>Failed uploads and send failures leave orphaned Active Storage blobs with no cleanup mechanism.</comment>
<file context>
@@ -105,6 +115,36 @@ func (c *composeCommand) run(cmd *cobra.Command, args []string) error {
+ // upload or a sent message.
+ for _, path := range c.attachments {
+ if err := attachments.Validate(path); err != nil {
+ return "", err
+ }
+ }
</file context>
|
Updated in
Verification run locally: mise exec -- make test
mise exec -- go build ./...
mise exec -- go vet ./...
git diff --check
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dbe51c9028
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| func (u *Uploader) put(ctx context.Context, upload *DirectUpload, body io.Reader) error { | ||
| req, err := http.NewRequestWithContext(ctx, http.MethodPut, upload.URL, body) |
There was a problem hiding this comment.
For S3-backed Active Storage direct-upload URLs, this builds the PUT from a generic io.Reader, so Go leaves the request length unknown and sends it chunked instead of with a fixed Content-Length. Presigned S3/S3-compatible PUT URLs reject that form even though the local httptest server accepts it, causing attachment uploads to fail before the message is sent. Please pass the blob/file size into put and set req.ContentLength before Do.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
2 issues found across 5 files (changes from recent commits).
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="internal/cmd/compose_attachments_test.go">
<violation number="1" location="internal/cmd/compose_attachments_test.go:150">
P1: t.Fatalf called from HTTP handler goroutine violates testing contract</violation>
</file>
<file name="internal/attachments/attachments.go">
<violation number="1" location="internal/attachments/attachments.go:156">
P1: Streaming upload via *os.File loses the explicit Content-Length guarantee. Go only auto-sets ContentLength for bytes.Buffer, bytes.Reader, and strings.Reader, not for *os.File. Active Storage S3 direct-upload URLs sign content_length, so chunked encoding can trigger a 403 signature mismatch.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| srv := httptest.NewServer(mux) | ||
| defer srv.Close() | ||
|
|
||
| mux.HandleFunc("/rails/active_storage/direct_uploads.json", func(w http.ResponseWriter, r *http.Request) { |
There was a problem hiding this comment.
P1: t.Fatalf called from HTTP handler goroutine violates testing contract
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At internal/cmd/compose_attachments_test.go, line 150:
<comment>t.Fatalf called from HTTP handler goroutine violates testing contract</comment>
<file context>
@@ -120,3 +136,129 @@ func TestAttachFilesDirectoryFails(t *testing.T) {
+ srv := httptest.NewServer(mux)
+ defer srv.Close()
+
+ mux.HandleFunc("/rails/active_storage/direct_uploads.json", func(w http.ResponseWriter, r *http.Request) {
+ var body struct {
+ Blob struct {
</file context>
| }, nil | ||
| } | ||
|
|
||
| func (u *Uploader) put(ctx context.Context, upload *DirectUpload, body io.Reader) error { |
There was a problem hiding this comment.
P1: Streaming upload via *os.File loses the explicit Content-Length guarantee. Go only auto-sets ContentLength for bytes.Buffer, bytes.Reader, and strings.Reader, not for *os.File. Active Storage S3 direct-upload URLs sign content_length, so chunked encoding can trigger a 403 signature mismatch.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At internal/attachments/attachments.go, line 156:
<comment>Streaming upload via *os.File loses the explicit Content-Length guarantee. Go only auto-sets ContentLength for bytes.Buffer, bytes.Reader, and strings.Reader, not for *os.File. Active Storage S3 direct-upload URLs sign content_length, so chunked encoding can trigger a 403 signature mismatch.</comment>
<file context>
@@ -153,12 +149,12 @@ func (u *Uploader) Upload(ctx context.Context, path string) (*Attachment, error)
-func (u *Uploader) put(ctx context.Context, upload *DirectUpload, data []byte) error {
- req, err := http.NewRequestWithContext(ctx, http.MethodPut, upload.URL, bytes.NewReader(data))
+func (u *Uploader) put(ctx context.Context, upload *DirectUpload, body io.Reader) error {
+ req, err := http.NewRequestWithContext(ctx, http.MethodPut, upload.URL, body)
if err != nil {
</file context>
Summary
-a, --attachmentsupport tohey compose--thread-idrepliesCloses #117.
Verification
mise exec -- make testmise exec -- go build ./...git diff --checkNotes
I smoke-tested the feature by sending a PDF from
geonizeli@hey.comtogeonizeli@hey.com; the CLI returnedok: trueand the message appeared in the Imbox.make lintwas not run locally becausegolangci-lintis not installed in this environment.Summary by cubic
Adds attachments to
hey composevia the repeatable-a/--attachmentflag. Files are direct-uploaded through Active Storage and embedded as ActionText attachments in new messages and--thread-idreplies.--thread-idto avoid partial sends..surface, and README with usage examples.Written for commit dbe51c9. Summary will update on new commits.