Skip to content

Bump SDK to v0.10.0 - #584

Open
jeremy wants to merge 3 commits into
mainfrom
sdk-0.10.0
Open

Bump SDK to v0.10.0#584
jeremy wants to merge 3 commits into
mainfrom
sdk-0.10.0

Conversation

@jeremy

@jeremy jeremy commented Jul 29, 2026

Copy link
Copy Markdown
Member

Moves the SDK pin from bb363c84 to the v0.10.0 tag, ahead of cutting release N.

Breaking Go surface, and how it was adapted

UpdateCardRequest fields 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.Content and .Description became *string and always arrive null (basecamp/basecamp-sdk#487). The server's highlighted excerpt moved to PlainTextContent/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. The search command itself never read those fields (it emits id/title/type/project/created), so CLI output is unaffected.

Behavior riding along

EverythingService is deliberately not wired

v0.10.0 adds EverythingService via AccountClient.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.md records 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 — everything overlaps conceptually with the existing recordings, assignments, and search groups, so it deserves its own shaping rather than a mechanical 17-command translation.

Verification

bin/ci green: 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/go v0.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

    • Update to v0.10.0; adds EverythingService (17 methods). Not surfaced in the CLI; documented in API-COVERAGE.md.
    • Model: UpdateCardRequest fields are now *string for merge-safe partial updates; updated cards update and workspace TUI.
    • Search: switched to PlainTextContent/PlainTextDescription for excerpts.
    • Transport: HTTP 400 maps to validation; retries honor per-op max, replay bodies, and respect retry_on.
  • Migration

    • Exit codes: 400 now exits with 9 (was 7).

Written for commit 84efb6a. Summary will update on new commits.

Review in cubic

jeremy added 2 commits July 28, 2026 23:34
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.
Copilot AI review requested due to automatic review settings July 29, 2026 06:44
@github-actions github-actions Bot added commands CLI command implementations tui Terminal UI sdk SDK wrapper and provenance docs deps labels Jul 29, 2026
@github-actions github-actions Bot added the bug Something isn't working label Jul 29, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/go dependency to v0.10.0 (go.mod/go.sum) and refresh SDK provenance.
  • Adapt card update call sites to the SDK’s *string UpdateCardRequest fields to preserve “only send what was asked for” semantics.
  • Update TUI search excerpt extraction to use PlainTextContent / PlainTextDescription per SDK change, and document the new coverage notes in API-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.

Comment thread internal/tui/workspace/views/search.go Outdated
Comment on lines +518 to +519
// Content and Description always arrive null; the plain-text variants carry
// the server's highlighted excerpt.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread API-COVERAGE.md
Comment on lines +24 to +26
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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
Copilot AI review requested due to automatic review settings July 29, 2026 06:49

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated no new comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working commands CLI command implementations deps docs sdk SDK wrapper and provenance tui Terminal UI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants