Uh oh!
There was an error while loading. Please reload this page.
Fix chat post JSON output contract and upload error handling - #365
Conversation
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".
Uh oh!
There was an error while loading. Please reload this page.
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.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
e54979b to
c9aa7bdCompareThere 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.
Uh oh!
There was an error while loading. Please reload this page.
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".
Uh oh!
There was an error while loading. Please reload this page.
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.
c55b77d to
49eb2fcCompareThere 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.
Uh oh!
There was an error while loading. Please reload this page.
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.