Bump SDK to v0.10.0 - #584
Conversation
Moves the pin from bb363c84 to the v0.10.0 tag. Two adaptations were forced by the new surface: UpdateCardRequest's Title, Content, and DueOn are now *string, where nil leaves the field unchanged. The call sites already guarded on non-empty values, so they now take addresses and preserve the same "only send what was asked for" behavior — with the SDK's merge-safe semantics underneath. SearchResult.Content and .Description became *string and always arrive null; the server's highlighted excerpt now lands in PlainTextContent and PlainTextDescription. The workspace search view reads those instead, so excerpts keep rendering rather than silently going blank. Also rides along from the SDK: HTTP 400 now maps to the validation error code rather than api_error, which moves the exit code for a 400 from 7 to 9. 422 was already validation, so the two agree now.
Notes the new EverythingService aggregate family as present in the SDK but not yet surfaced by any CLI command, so it stays untracked and out of the parity totals rather than being counted as covered. Also records the transport and model changes that ride along with the bump.
There was a problem hiding this comment.
Pull request overview
This PR bumps the Basecamp Go SDK dependency to v0.10.0 and updates Basecamp CLI call sites to match the SDK’s breaking surface changes (notably pointer fields on card updates and updated search excerpt fields), while recording the new SDK/API provenance and updating API coverage notes.
Changes:
- Update
github.com/basecamp/basecamp-sdk/godependency tov0.10.0(go.mod/go.sum) and refresh SDK provenance. - Adapt card update call sites to the SDK’s
*stringUpdateCardRequestfields to preserve “only send what was asked for” semantics. - Update TUI search excerpt extraction to use
PlainTextContent/PlainTextDescriptionper SDK change, and document the new coverage notes inAPI-COVERAGE.md.
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 6 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| internal/version/sdk-provenance.json | Records updated SDK version/revision and synced API revision. |
| internal/tui/workspace/views/search.go | Switches TUI search excerpt source to the new plain-text excerpt fields. |
| internal/tui/workspace/views/detail.go | Updates card due date/title edits to send pointer fields in UpdateCardRequest. |
| internal/commands/cards.go | Updates cards update request construction to use pointer fields and preserve partial update behavior. |
| go.sum | Updates dependency checksums for the bumped SDK version. |
| go.mod | Pins the Basecamp SDK dependency to v0.10.0. |
| API-COVERAGE.md | Documents SDK v0.10.0 behavior changes and notes EverythingService as present-but-unsurfaced. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Content and Description always arrive null; the plain-text variants carry | ||
| // the server's highlighted excerpt. |
There was a problem hiding this comment.
Good point — taken in 84efb6a. The comment now describes the SDK contract (fields became nullable, excerpt moved to the plain-text variants) rather than asserting what the server puts on the wire, so it stays accurate if that changes.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4c40a9c153
ℹ️ 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".
| the open/completed/unassigned/overdue/no-due-date todo and card rollups. **No CLI | ||
| commands surface it yet** — the family is untracked in the matrix below and | ||
| excluded from the parity totals, on the same footing as the other untracked BC5 |
There was a problem hiding this comment.
Add CLI coverage for the new Everything methods
The SDK bump introduces 17 callable EverythingService operations, but this change explicitly leaves every one inaccessible from the CLI and excludes them from the coverage matrix. Users needing these account-wide rollups therefore cannot use the newly adopted SDK surface, contrary to the repository's SDK completeness requirement; add the corresponding commands, catalog/registration coverage, tests, and matrix row as part of this bump.
AGENTS.md reference: AGENTS.md:L121-L125
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Declining this one deliberately, and the coverage matrix is written so it doesn't paper over the gap.
The completeness bar in AGENTS.md is real, but it isn't mechanically enforced — make test and the catalog-parity test pass with the service unwired, because they check CLI-internal consistency, not SDK-to-CLI coverage. So this is a scoping judgment, not a gate bypass.
The judgment: this bump sits on the critical path for release N, which exists to ship the keychain-hang fix train to users still on v0.7.2 from four months ago. EverythingService is 17 methods of new feature surface. Wiring it here would turn a pin bump into a feature release and delay the fix.
The alternative to silence is what's in this diff: API-COVERAGE.md records the family as present-but-unsurfaced and keeps it out of the parity totals, so the matrix does not claim coverage it doesn't have. Design work is tracked in #585.
On "add the corresponding commands" — a 1:1 translation would be the wrong shape regardless of timing. These aggregates overlap recordings (browse by type/status across projects), assignments (what's on my plate), and search. Whether they belong in a new everything group or as account-wide scope flags on the groups that already own those nouns is an open design question, and STYLE.md forbids a new top-level verb shadowing an existing group noun. That deserves shaping, not a mechanical port under release pressure.
…vior The absolute claim about nulls on the wire would go stale if the server changed; what is durable is that the SDK made the fields nullable and moved the excerpt to the plain-text variants.
Moves the SDK pin from
bb363c84to thev0.10.0tag, ahead of cutting release N.Breaking Go surface, and how it was adapted
UpdateCardRequestfields became*string(basecamp/basecamp-sdk#489), where nil now means "leave unchanged" instead of the field being merge-unsafe. The three call sites (cards update, plus the workspace TUI's due-date and title editors) already guarded on non-empty values, so they take addresses and preserve the same "only send what was asked for" behavior — now with merge-safe semantics underneath.SearchResult.Contentand.Descriptionbecame*stringand always arrive null (basecamp/basecamp-sdk#487). The server's highlighted excerpt moved toPlainTextContent/PlainTextDescription. The workspace search view now reads those. This one is worth a second look in review: fixing the compile error by dereferencing the old fields would have compiled cleanly and produced permanently blank excerpts. Thesearchcommand itself never read those fields (it emits id/title/type/project/created), so CLI output is unaffected.Behavior riding along
validationerror code rather thanapi_error(Go: map HTTP 400 to validation (not api_error) basecamp-sdk#482). SinceconvertSDKErrorpassessdkErr.Codestraight through, a 400's exit code moves from 7 to 9. 422 was already validation, so the two now agree. This is a user-visible contract change for anyone scripting against exit codes.retry.maxis honored as a ceiling (security: pin the gitleaks download with a SHA-256 checksum #483),*WithBodybodies replay across retries (security: close config trust-boundary gaps and gate completion loader #481), and the declaredretry_onstatus set is honored (Document multiple identity profiles #486). The CLI configures no retry policy of its own — its resilience layer is gating (circuit breaker/bulkhead) via hooks, not a retry loop — so nothing compounds.EverythingService is deliberately not wired
v0.10.0 adds
EverythingServiceviaAccountClient.Everything()(basecamp/basecamp-sdk#435, #438): 17 methods covering account-wide messages, comments, checkins, forwards, boosts, files, and the open/completed/unassigned/overdue/no-due-date todo and card rollups.No CLI commands surface it here. Wiring 17 methods is feature work, and this bump is on the release-N critical path for the keychain fix train.
API-COVERAGE.mdrecords the family as present-but-unsurfaced and keeps it out of the parity totals rather than counting it as covered. Worth a follow-up issue to design the command surface —everythingoverlaps conceptually with the existingrecordings,assignments, andsearchgroups, so it deserves its own shaping rather than a mechanical 17-command translation.Verification
bin/cigreen: build, vet, lint, unit + 346 e2e tests, naming, CLI surface snapshot up to date, skill drift, bare-group convention, leaf-command coverage (0 uncovered), SDK provenance OK at v0.10.0, go mod tidy.Summary by cubic
Bumps SDK to
github.com/basecamp/basecamp-sdk/gov0.10.0 and adapts the CLI to its breaking API changes. Card updates and search excerpts keep working; HTTP 400 now returns the validation exit code.Dependencies
v0.10.0; adds EverythingService (17 methods). Not surfaced in the CLI; documented inAPI-COVERAGE.md.UpdateCardRequestfields are now*stringfor merge-safe partial updates; updatedcards updateand workspace TUI.PlainTextContent/PlainTextDescriptionfor excerpts.validation; retries honor per-op max, replay bodies, and respectretry_on.Migration
Written for commit 84efb6a. Summary will update on new commits.