Project-scope messagetypes on the bucket-scoped SDK; absorb bump-range surface (Fixes #567) - #570
Merged
Conversation
…SDK (Fixes #567) Message types (categories) are bucket-scoped in the BC3 API, but the SDK modeled them account-scoped, so every messagetypes command 404ed in production. Bump the SDK to bb363c847b92 (basecamp/basecamp-sdk#393), which moves MessageTypesService onto /buckets/{id}/categories…, and give all five commands --in/--project scoping per the gauges convention. Validation stays ahead of project resolution (resolution fetches /projects.json even for numeric IDs), and every breadcrumb carries the resolved --in. Unit tests pin the exact method + path for all five operations with a recording stub transport that rejects unmatched routes, plus validation-order coverage. e2e gains the sibling-standard missing-project and empty-flag tests; smoke passes -p on both messagetypes invocations.
…range Two new SDK capabilities land in the 3d4ec395922c..bb363c847b92 range beyond the messagetypes reshape: - Bubble Ups successor surface (basecamp/basecamp-sdk#426): 'notifications bubbleups' pages through all current and scheduled bubble-ups via GET /my/readings/bubble_ups.json, and 'notifications list --limit-bubble-ups' caps the inline feed at 2 while surfacing the uncapped counts in the summary. - Create-time client visibility on Tools.Create (basecamp/basecamp-sdk#419): 'tools create --visible-to-clients' passes the tri-state pointer, allocating the options struct whenever the flag is explicitly set so an explicit false reaches the wire. Honored only by chat and kanban_board tool types. Unit tests cover the dedicated endpoint, the limit_bubble_ups query param, and all three visible_to_clients wire states.
Rewrite the SDK version summary for v0.9.1-0.20260727173625-bb363c847b92, note the bucket-scoped categories API on the message_types row, add a new BC5 Bubble Ups row (per the BC5-split rule) with reconciled summary totals, and note create-time visible_to_clients on dock_tools. The skill documents 'notifications bubbleups', '--limit-bubble-ups', and 'tools create --visible-to-clients'.
github-actions
Bot
added
commands
![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: ad89acb82b
ℹ️ 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".
…dcrumb Following the next breadcrumb from a --limit-bubble-ups listing silently dropped the cap, restoring scheduled bubble-ups on subsequent pages. Carry the flag through and pin it with a breadcrumb test.
There was a problem hiding this comment.
Pull request overview
This PR updates the CLI to match newly bucket-scoped SDK/API behavior for message types by making messagetypes project-scoped (fixing production 404s), and absorbs additional SDK surface by adding Bubble Ups notifications support and create-time client visibility for tool creation.
Changes:
- Bump
github.com/basecamp/basecamp-sdk/goand update SDK provenance to the new revision/API sync. - Scope
messagetypesoperations to--in/--projectand add unit + e2e coverage for the bucket-scoped routes. - Add
notifications bubbleupsplusnotifications list --limit-bubble-ups, and addtools create --visible-to-clients(tri-state wire behavior) with tests and docs/surface updates.
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 17 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| skills/basecamp/SKILL.md | Documents new Bubble Ups command/flag and tools create --visible-to-clients behavior. |
| internal/version/sdk-provenance.json | Updates pinned SDK/API provenance metadata for the bump. |
| internal/commands/tools.go | Adds --visible-to-clients to tool creation and ensures explicit false can be sent. |
| internal/commands/tools_test.go | Adds unit tests covering tri-state visible_to_clients wire contract. |
| internal/commands/notifications.go | Adds Bubble Ups subcommand and --limit-bubble-ups option path + summaries/breadcrumbs. |
| internal/commands/notifications_test.go | Adds recording-transport based tests for the Bubble Ups endpoint and query param behavior. |
| internal/commands/messagetypes.go | Makes message types project-scoped and forwards resolved bucket ID into SDK calls. |
| internal/commands/messagetypes_test.go | Adds recording-transport tests pinning the 5-operation bucket-scoped route matrix + validation order. |
| internal/commands/commands.go | Updates command catalog to include notifications bubbleups. |
| go.sum | Updates dependency checksums for the SDK bump. |
| go.mod | Bumps the Basecamp SDK module version. |
| e2e/tools.bats | Adds e2e validation coverage for tools create --visible-to-clients missing --type. |
| e2e/smoke/smoke_notifications.bats | Adds smoke coverage for notifications bubbleups (allowlisted when endpoint unavailable). |
| e2e/smoke/smoke_communication.bats | Updates messagetypes smoke tests to pass project scoping. |
| e2e/smoke/.qa-allowlist | Updates allowlist notes for per-project messagetypes and adds Bubble Ups allowlist entry. |
| e2e/messagetypes.bats | Adds e2e tests for missing-project and empty --project value cases. |
| API-COVERAGE.md | Updates coverage matrix and SDK summary for bucket-scoped message types and Bubble Ups/tools visibility. |
| .surface | Updates CLI surface snapshot for new commands/flags. |
💡 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: 9ca3b29d3c
ℹ️ 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".
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
… extract shared recording test harness notifications read resolves IDs from the notification feed (/my/readings.json), not the dedicated Bubble Ups endpoint, so the read breadcrumb on bubbleups failed for any bubble-up outside the feed's first page. Remove it and pin the absence with a breadcrumb test. Move the recording-transport helpers out of messagetypes_test.go into a dedicated recording_test.go with its own token provider, making the notifications tests' dependency explicit and dropping the cross-file coupling on messages_test.go. Also align the messagetypes --in help text with the accepted inputs (ID or name).
Review carefully before merging. Consider a major version bump. |
The headline notification total was computed from the capped bubble_ups array and the omitted scheduled_bubble_ups array, undercounting relative to the non-limited mode. Use BubbleUpsCount/ScheduledBubbleUpsCount for the total when the cap is active, and pin the arithmetic in the breadcrumb test.
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
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.
Problem
Message types (categories) are bucket-scoped in the BC3 API, but the SDK modeled them account-scoped — all five `bc messagetypes` operations 404 in production today (#567).
Changes
Commit 1 — SDK bump + messagetypes scoping (Fixes #567)
Commit 2 — absorb additive SDK surface in the bump range
Commit 3 — docs
Verification
Fixes #567
Summary by cubic
Scopes
messagetypesto projects to match the bucket-scoped API, fixing production 404s (Fixes #567). Adds Bubble Ups listing and a compact notifications option; tools create can set client visibility, and notification totals are correct when the compact option is used.New Features
notifications bubbleupslists all Bubble Ups (paginated).notifications list --limit-bubble-upscaps inline Bubble Ups at 2 and shows uncapped counts.tools create --visible-to-clientssets client visibility at create time (chat and kanban_board only).github.com/basecamp/basecamp-sdk/gotov0.9.1for bucket-scoped MessageTypes, Bubble Ups, and tool visibility.Bug Fixes
messagetypesare now project-scoped. All list/show/create/update/delete commands require--in/--projectand call/buckets/{id}/categories…, resolving 404s.--limit-bubble-ups; kept--limit-bubble-upson the next-page breadcrumb; removed the unusablereadbreadcrumb fromnotifications bubbleups.Written for commit 62e3ed5. Summary will update on new commits.