From 899c166f3440abc12f67e66a709c5c697d29a9bf Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Tue, 24 Mar 2026 09:42:00 -0700 Subject: [PATCH 1/3] Fix chat post JSON output contract and upload error handling The --attach addition (cbaf4c5) changed chat post to return a synthetic map instead of the CampfireLine object, breaking --json consumers and mismatching the chat_line schema presenter. Restore CampfireLine return for text-only posts (the common case). Use a composite map only when uploads are involved, without the chat_line entity hint that doesn't fit the map shape. Also: route upload errors through convertSDKError for proper error code extraction, and rename contentType shadow to mimeType. --- internal/commands/chat.go | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/internal/commands/chat.go b/internal/commands/chat.go index b9e7f1dab..6e7a467a4 100644 --- a/internal/commands/chat.go +++ b/internal/commands/chat.go @@ -386,7 +386,7 @@ func runChatPost(cmd *cobra.Command, app *appctx.App, chatID, project, content, } // Post message using SDK - var lineID int64 + var line *basecamp.CampfireLine var uploadIDs []int64 // Post text message if there's content @@ -395,11 +395,11 @@ func runChatPost(cmd *cobra.Command, app *appctx.App, chatID, project, content, if contentType != "" { opts = &basecamp.CreateLineOptions{ContentType: contentType} } - line, err := app.Account().Campfires().CreateLine(cmd.Context(), chatIDInt, content, opts) + var err error + line, err = app.Account().Campfires().CreateLine(cmd.Context(), chatIDInt, content, opts) if err != nil { return err } - lineID = line.ID } // Upload attachments using CreateUpload @@ -409,7 +409,7 @@ func runChatPost(cmd *cobra.Command, app *appctx.App, chatID, project, content, return fmt.Errorf("%s: %w", filePath, err) } - contentType := richtext.DetectMIME(normalized) + mimeType := richtext.DetectMIME(normalized) filename := filepath.Base(normalized) f, err := os.Open(normalized) @@ -417,20 +417,20 @@ func runChatPost(cmd *cobra.Command, app *appctx.App, chatID, project, content, return fmt.Errorf("%s: %w", filePath, err) } - uploadLine, err := app.Account().Campfires().CreateUpload(cmd.Context(), chatIDInt, filename, contentType, f) + uploadLine, err := app.Account().Campfires().CreateUpload(cmd.Context(), chatIDInt, filename, mimeType, f) f.Close() if err != nil { - return fmt.Errorf("failed to upload %s: %w", filePath, err) + return convertSDKError(err) } uploadIDs = append(uploadIDs, uploadLine.ID) } // Build summary var summary string - if lineID != 0 && len(uploadIDs) > 0 { - summary = fmt.Sprintf("Posted message #%d with %d attachment(s)", lineID, len(uploadIDs)) - } else if lineID != 0 { - summary = fmt.Sprintf("Posted message #%d", lineID) + if line != nil && len(uploadIDs) > 0 { + summary = fmt.Sprintf("Posted message #%d with %d attachment(s)", line.ID, len(uploadIDs)) + } else if line != nil { + summary = fmt.Sprintf("Posted message #%d", line.ID) } else if len(uploadIDs) > 0 { summary = fmt.Sprintf("Posted %d attachment(s)", len(uploadIDs)) } @@ -467,17 +467,25 @@ func runChatPost(cmd *cobra.Command, app *appctx.App, chatID, project, content, respOpts := []output.ResponseOption{ output.WithSummary(summary), - output.WithEntity("chat_line"), output.WithBreadcrumbs(breadcrumbs...), } if mentionNotice != "" { respOpts = append(respOpts, output.WithDiagnostic(mentionNotice)) } - // Return result with all created IDs - result := map[string]interface{}{ - "message_id": lineID, - "upload_ids": uploadIDs, + // Text-only: return the Line object directly (preserves JSON contract) + if line != nil && len(uploadIDs) == 0 { + respOpts = append(respOpts, output.WithEntity("chat_line")) + return app.OK(line, respOpts...) + } + + // Uploads involved: return composite result + result := map[string]any{} + if line != nil { + result["message_id"] = line.ID + } + if len(uploadIDs) > 0 { + result["upload_ids"] = uploadIDs } return app.OK(result, respOpts...) } From 945c1a1d775c65123a273c514d6a3adec37b858c Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Tue, 24 Mar 2026 09:48:46 -0700 Subject: [PATCH 2/3] Address review feedback: error context, display data, SDK error routing - Preserve filePath context in upload errors: wrap with fmt.Errorf before convertSDKError so users know which file failed - Add WithDisplayData(chatLineDisplayData(line)) for text-only posts so styled/markdown output renders consistently with other chat commands - Route CreateLine errors through convertSDKError for structured error codes/hints --- internal/commands/chat.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/internal/commands/chat.go b/internal/commands/chat.go index 6e7a467a4..0368f7c65 100644 --- a/internal/commands/chat.go +++ b/internal/commands/chat.go @@ -398,7 +398,7 @@ func runChatPost(cmd *cobra.Command, app *appctx.App, chatID, project, content, var err error line, err = app.Account().Campfires().CreateLine(cmd.Context(), chatIDInt, content, opts) if err != nil { - return err + return convertSDKError(err) } } @@ -420,7 +420,7 @@ func runChatPost(cmd *cobra.Command, app *appctx.App, chatID, project, content, uploadLine, err := app.Account().Campfires().CreateUpload(cmd.Context(), chatIDInt, filename, mimeType, f) f.Close() if err != nil { - return convertSDKError(err) + return convertSDKError(fmt.Errorf("%s: %w", filePath, err)) } uploadIDs = append(uploadIDs, uploadLine.ID) } @@ -475,7 +475,10 @@ func runChatPost(cmd *cobra.Command, app *appctx.App, chatID, project, content, // Text-only: return the Line object directly (preserves JSON contract) if line != nil && len(uploadIDs) == 0 { - respOpts = append(respOpts, output.WithEntity("chat_line")) + respOpts = append(respOpts, + output.WithEntity("chat_line"), + output.WithDisplayData(chatLineDisplayData(line)), + ) return app.OK(line, respOpts...) } From 49eb2fc96c44db58e557761d8d437620b7cec0e3 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Tue, 24 Mar 2026 10:11:07 -0700 Subject: [PATCH 3/3] Preserve filePath in structured SDK upload errors convertSDKError unwraps to find *basecamp.Error and reconstructs output.Error from its fields, discarding any fmt.Errorf wrapper context. Prefix filePath into the converted error's Message so users know which file failed in multi-attachment uploads. --- internal/commands/chat.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/internal/commands/chat.go b/internal/commands/chat.go index 0368f7c65..d6d4a9366 100644 --- a/internal/commands/chat.go +++ b/internal/commands/chat.go @@ -420,7 +420,12 @@ func runChatPost(cmd *cobra.Command, app *appctx.App, chatID, project, content, uploadLine, err := app.Account().Campfires().CreateUpload(cmd.Context(), chatIDInt, filename, mimeType, f) f.Close() if err != nil { - return convertSDKError(fmt.Errorf("%s: %w", filePath, err)) + sdkErr := convertSDKError(err) + if outErr, ok := sdkErr.(*output.Error); ok { + outErr.Message = fmt.Sprintf("%s: %s", filePath, outErr.Message) + return outErr + } + return fmt.Errorf("%s: %w", filePath, err) } uploadIDs = append(uploadIDs, uploadLine.ID) }