Fix chat post JSON output contract and upload error handling#365
Fix chat post JSON output contract and upload error handling#365
Conversation
github-actions
bot
added
commands
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![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: 0ab2135353
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
This PR restores the basecamp chat post --json output contract by returning a full CampfireLine object for text-only posts, while returning a composite {message_id, upload_ids} result only when attachments are involved. It also improves attachment-upload error handling to preserve structured SDK error codes/hints, and removes a local variable shadow in the upload loop.
Changes:
- Return
*basecamp.CampfireLinefor text-only chat posts (and only use the composite ID map when uploads are present). - Route
CreateUploaderrors throughconvertSDKErrorfor consistent structured CLI errors. - Rename the inner
contentTypeshadow tomimeTypein the upload loop.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
jeremy
force-pushed
the
fix-chat-post-json-contract
branch
from
e54979b to
c9aa7bd
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c9aa7bd47e
ℹ️ 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".
Review carefully before merging. Consider a major version bump. |
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.
- 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
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.
jeremy
force-pushed
the
fix-chat-post-json-contract
branch
from
c55b77d to
49eb2fc
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Cherry-picked fixes from pending main PRs: - Fix --assignee filter ignored with --list (#369) - Fix chat post JSON output contract and upload error handling (#365) - Fix schedule update silently dropping --attach without --description (#364) - Fix blank-line-after-list suppressing paragraph break (#363) - Fix campfire messages --limit being ignored (#367)
…accounts (#373) * Bump basecamp-sdk to v0.7.1 Fixes Person.Id deserialization (normalizeJSON pre-unmarshal for notification responses) and Account.Logo type (string → AccountLogo object). Adapts two breaking changes: Cards().Move() and Todos().Reposition() now take optional params. Adds AVIF/HEIC to richtext MIME extension map for logo upload support. * Merge upstream fixes (#363, #364, #365, #367, #369) Also fixes: chat upload error now wraps converted SDK error instead of raw error; schedule update --attach without --description now fetches current description to append rather than replace. * Add gauges command for project progress tracking Subcommands: list (account-wide), needles/needle (project-scoped), create/update/delete, enable/disable. Create validates --position (0-100), --color, --notify (requires --subscriptions for custom mode). * Add assignments command for viewing my assignments Subcommands: list, completed, due (with scope filter). Shortcut pattern — bare invocation shows priorities + non-priorities. * Add notifications command, extend accounts, register all new commands Notifications: list (paginated with dynamic next-page breadcrumb) and read (resolves IDs to SGIDs from --page). Accounts: show, update, logo upload (5MB + image type validation) and remove. Registers all new commands in root.go, catalog, and test harness. * Update docs and surface for v0.7.1 commands API-COVERAGE.md: 42 sections, 155 endpoints, SDK v0.7.1. SKILL.md: gauges, assignments, notifications, accounts sections. .surface: 148 commands, 422 flags.
Summary
--attachaddition (cbaf4c5) replaced theCampfireLinereturn with a synthetic{"message_id", "upload_ids"}map, breaking--jsonconsumers expecting Line fields and mismatching thechat_lineschema presenter*basecamp.CampfireLinereturn for text-only posts (preserves backward compatibility); uses composite map only when uploads are involved, without thechat_lineentity hintconvertSDKError(was using rawfmt.Errorf, losing SDK error code/hint extraction)contentTypevariable shadow tomimeTypein the upload loopTest plan
make test— all passmake lint— cleanbasecamp chat post "hello" --in <project> --jsonshould return full CampfireLine JSONbasecamp chat post "hello" --attach <file> --in <project>should return composite resultSummary by cubic
Restores the chat post JSON contract. Text-only posts return the full
*basecamp.CampfireLinewith display data; uploads return a compact map withmessage_idand/orupload_ids. Improves error handling and keeps SDK error codes, including file path context for failed uploads.*basecamp.CampfireLinefor text-only posts; set entity tochat_lineand addWithDisplayData(chatLineDisplayData(line)).chat_lineentity; includemessage_idwhen present andupload_idsfor attachments.CreateLineandCreateUploaderrors throughconvertSDKError; preserve the failing file path in structured SDK errors; rename shadowedcontentTypetomimeType.Written for commit 49eb2fc. Summary will update on new commits.