Expose client visibility at create time (Fixes #457) - #554
Conversation
Basecamp (bc3 master) accepts a top-level visible_to_clients param at create time, but the Go SDK's create-request types don't carry it, so the CLI can't pass it without an out-of-lane SDK change. Record the required SDK change (Smithy create inputs -> generated -> wrapper -> mapping) and the CLI wiring that follows in SDK-GAP-457.md.
There was a problem hiding this comment.
Pull request overview
Adds an SDK gap communiqué documenting what the Basecamp Go SDK must expose to let the CLI set visible_to_clients at recording create time (blocking CLI issue #457), so the eventual --visible-to-clients flag can be implemented atomically.
Changes:
- Add
SDK-GAP-457.mddescribing server behavior, in-scope recording types, and the specific SDK shape/wrapper/mapping changes needed. - Document the intended follow-on CLI wiring steps after an SDK bump.
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.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
All reported issues were addressed across 1 file
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…cs/uploads Address review of SDK-GAP-457.md: - Fix message create contract: POST /message_boards/:board_id/messages.json with a flat body; the visible_to_clients key is top-level. - Require *bool tri-state (nil=inherit, true, false) end-to-end instead of bool,omitempty, which drops explicit false; cite the AllDay precedent. - Defer docs/uploads: they accept arbitrary folder IDs and nested vaults inherit folder visibility (silent no-op), so gate or defer before wiring. - Scope the verified gap to SDK v0.8.0; correct the lane-policy wording. - Link SDK tracking issue basecamp/basecamp-sdk#395 as the unblocker.
…ink follow-ups - Reference 'this PR' instead of a hard-coded PR number (drift-prone). - Point at function/struct identifiers instead of line numbers (files.go, schedules.go). - Link deferred docs/uploads to CLI follow-up #556; note the SDK should still expose those inputs for completeness (#395 covers all six) while the CLI withholds only the flag. - Clarify #457 closure sequencing (four-together vs messages-first).
… locked #457 boundary - Requested SDK change now targets all six inputs in the implementation steps (message, todolist, question, schedule entry, document, upload), matching the surrounding completeness note; no more four-now/two-later contradiction. - Gap evidence lists the six actual target wrappers (CreateMessageRequest, CreateTodolistRequest, CreateQuestionRequest, CreateScheduleEntryRequest, CreateDocumentRequest, CreateUploadRequest), not the unrelated card/todo/ comment structs. - Lock #457 completion boundary to the four initial commands; messages-first stays Refs #457 until all four are wired.
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Advances the basecamp-sdk pin from v0.8.0 to v0.8.1-0.20260724184307-e2c1abea4aea (basecamp-sdk#401), which adds a create-time visible_to_clients field to the six create-request inputs and repins bc3 provenance. The same bundle reshaped SearchMetadata: the Projects list was replaced by RecordingSearchTypes / FileSearchTypes filter options. Adapt runSearchMetadata's empty-check and summary to the new shape.
Wire the SDK's create-time visible_to_clients field into the four create commands where Basecamp accepts a top-level client-visibility param: messages, todolists, check-in questions, and schedule entries. The flag is tri-state — set req.VisibleToClients only when --visible-to-clients was provided (gate on Flags().Changed, matching --subscribe), so omitting it preserves the server's default and an explicit --visible-to-clients=false reaches the wire. Single atomic create, no follow-up toggle. Parent-inherited types (cards, todos, comments) are excluded — they ignore the create param and 403 on the toggle. Docs/uploads are deferred to #556 (nested-vault folder inheritance needs gating). Tests assert the create body for each command carries visible_to_clients for the unset/true/false cases. Regenerate .surface and document the flag in SKILL.md. Fixes #457
There was a problem hiding this comment.
All reported issues were addressed across 15 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Address review: - Client-visibility help was unsafe for client-authenticated callers. The four flag descriptions, inline comments, and SKILL.md claimed omission means team-only, but the server forces client visibility for client callers (even an explicit false is overridden). Document the context-dependent rule instead: team-only when posting as a team member; client callers always client-visible. - The SDK-bump adaptation left 'search metadata' describing 'available projects' though it now returns recording/file filter types. Fix the command Short/Long and add focused runSearchMetadata tests (filter-type summary; empty-response usage error), which the search tests previously never exercised.
There was a problem hiding this comment.
All reported issues were addressed across 7 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Address review: - schedule create: thread visibleToClients as a parameter into runScheduleCreate (matching the existing allDay pattern) instead of re-reading the flag via GetBool and dropping its error. Consistent with the other three create commands. - search metadata: parenthesize the nil-or-both-empty guard for clarity. - search_test: the transport's body field is a string, not a pointer — say 'empty body', not 'nil body'.
Bound the default search result set and wire the full filter surface into basecamp search. #470: --limit defaulted to 0, which the SDK treats as 'fetch every page' and follows Link-header pagination unbounded, hanging 90s+ on a broad query. Apply a default cap of 20 (--all restores unbounded; --all + --limit is a usage error). #546: sort vocabulary and the previously-ignored project/type/metadata filters. --sort normalizes to relevance (best_match) or recency. New filter flags: --project/--in scope to a project (resolved to bucket_ids[]) --type filter by content type (todo, card, ping, chat, ...) --creator filter by creator (name, email, ID, or me) --since bound to a time range (BC5-only) --file-type filter attachments (image, audio, video, pdf) --exclude-chat drop chat/campfire results Search uses a different type vocabulary than recordings (upload -> Attachment, ping -> Circle, check-in -> Question, event -> Schedule::Entry, folder -> Vault, chat -> Chat::Transcript, forward -> Inbox::Forward), so --type resolves through a dedicated table and rejects unknown values rather than passing them through: BC3 silently discards an unrecognized type and returns unfiltered results. --file-type normalizes casing to BC3's case-sensitive Blob::TYPES membership so 'image' does not silently disable the filter. Each filter is sent in both the plural (BucketIds/TypeNames/CreatorIds) and deprecated singular forms so BC5 honors the plural while older clients fall back to the singular. search metadata now presents the real recording/file search types and default labels; the summary counts only selectable (non-default) options — the key:null 'Everything'/'All files' default is excluded. Empty metadata is a graceful success, not an error. The SearchOptions/SearchMetadata types come from the SDK already pinned on main (e2c1abea4aea, bumped by #554); no further SDK change is needed here. Update .surface, SKILL.md, API-COVERAGE.md, and the search-metadata smoke test.
Bound the default search result set and wire the full filter surface into basecamp search. #470: --limit defaulted to 0, which the SDK treats as 'fetch every page' and follows Link-header pagination unbounded, hanging 90s+ on a broad query. Apply a default cap of 20 (--all restores unbounded; --all + --limit is a usage error). #546: sort vocabulary and the previously-ignored project/type/metadata filters. --sort normalizes to relevance (best_match) or recency. New filter flags: --project/--in scope to a project (resolved to bucket_ids[]) --type filter by content type (todo, card, ping, chat, ...) --creator filter by creator (name, email, ID, or me) --since bound to a time range (BC5-only) --file-type filter attachments (image, audio, video, pdf) --exclude-chat drop chat/campfire results Search uses a different type vocabulary than recordings (upload -> Attachment, ping -> Circle, check-in -> Question, event -> Schedule::Entry, folder -> Vault, chat -> Chat::Transcript, forward -> Inbox::Forward), so --type resolves through a dedicated table and rejects unknown values rather than passing them through: BC3 silently discards an unrecognized type and returns unfiltered results. --file-type normalizes casing to BC3's case-sensitive Blob::TYPES membership so 'image' does not silently disable the filter. Each filter is sent in both the plural (BucketIds/TypeNames/CreatorIds) and deprecated singular forms so BC5 honors the plural while older clients fall back to the singular. search metadata now presents the real recording/file search types and default labels; the summary counts only selectable (non-default) options — the key:null 'Everything'/'All files' default is excluded. Empty metadata is a graceful success, not an error. The SearchOptions/SearchMetadata types come from the SDK already pinned on main (e2c1abea4aea, bumped by #554); no further SDK change is needed here. Update .surface, SKILL.md, API-COVERAGE.md, and the search-metadata smoke test.
Bound the default search result set and wire the full filter surface into basecamp search. #470: --limit defaulted to 0, which the SDK treats as 'fetch every page' and follows Link-header pagination unbounded, hanging 90s+ on a broad query. Apply a default cap of 20 (--all restores unbounded; --all + --limit is a usage error). #546: sort vocabulary and the previously-ignored project/type/metadata filters. --sort normalizes to relevance (best_match) or recency. New filter flags: --project/--in scope to a project (resolved to bucket_ids[]) --type filter by content type (todo, card, ping, chat, ...) --creator filter by creator (name, email, ID, or me) --since bound to a time range (BC5-only) --file-type filter attachments (image, audio, video, pdf) --exclude-chat drop chat/campfire results Search uses a different type vocabulary than recordings (upload -> Attachment, ping -> Circle, check-in -> Question, event -> Schedule::Entry, folder -> Vault, chat -> Chat::Transcript, forward -> Inbox::Forward), so --type resolves through a dedicated table and rejects unknown values rather than passing them through: BC3 silently discards an unrecognized type and returns unfiltered results. --file-type normalizes casing to BC3's case-sensitive Blob::TYPES membership so 'image' does not silently disable the filter. Each filter is sent in both the plural (BucketIds/TypeNames/CreatorIds) and deprecated singular forms so BC5 honors the plural while older clients fall back to the singular. search metadata now presents the real recording/file search types and default labels; the summary counts only selectable (non-default) options — the key:null 'Everything'/'All files' default is excluded. Empty metadata is a graceful success, not an error. The SearchOptions/SearchMetadata types come from the SDK already pinned on main (e2c1abea4aea, bumped by #554); no further SDK change is needed here. Update .surface, SKILL.md, API-COVERAGE.md, and the search-metadata smoke test.
Rebasing onto main (#554) advanced the SDK pin to e2c1abea, five commits past the 93ace8d5 this branch first pinned. Refresh the API-COVERAGE SDK line to the merged pin (which additionally carries create-time visible_to_clients support) and regenerate the CLI surface so it reflects both todolists position and main's todolists create --visible-to-clients.
* Add `todolists position` to reorder to-do lists (#484) Adds `basecamp todolists position <id|url>...` to reorder a to-do list within its todoset — the one repositioning gap in the to-do family (`todos` and `todolistgroups` already had it). Single-list mode sets one explicit position (`--to N`, 1-based). Bulk mode accepts several incomplete lists from the same todoset and sets their visible order top-to-bottom; it is restricted to position 1 because reverse-iterating onto any other slot cannot reconstruct the typed order once loose to-dos or hidden completed lists shift the base. Bulk preflight enforces a shared todoset/bucket and rejects completed lists before issuing any PUT, and application stops at the first failure (a half-applied reorder is a wrong order), reporting applied-vs-attempted. Uses the dedicated `PUT /todosets/todolists/{id}/position.json` route, not the generic `/recordings/{id}/position.json` the issue's workaround used — the generic one skips the to-do-list position math and mis-places lists when loose to-dos or hidden completed lists exist. Naming follows the to-do family (`position`, aliases `move`/`reorder`, `--to` leading with `--position` accepted); a to-do list has no destination container, so `--to N` is unambiguously an index. SDK bumped to 93ace8d5 (basecamp-sdk#398), which also absorbs #396 (Templates.CreateProject envelope fix, signature unchanged) and #392 (SDK tests only). Closes #484 * Preserve error classification on partial reposition failure The bulk-apply failure path reclassified every non-*output.Error (raw transport/runtime failures, context cancellation) as code=usage via ErrUsageHint, overwrote the SDK's structured hint with the rerun hint, and claimed the todoset was left in an intermediate order even when the failing PUT was the first/only one and nothing had been mutated. Now: when zero PUTs have landed (always true in single-list mode, and when a bulk run's first PUT fails), surface the underlying error unchanged — its code, HTTP status, and hint intact, no false intermediate-order claim. Only once a partial reorder has actually landed does it add the applied-count accounting and rerun hint, and even then it preserves the underlying error's classification (appending the rerun text to the original hint rather than replacing it) and wraps raw errors with %w instead of reclassifying them as usage. Adds regression coverage for Code/HTTPStatus preservation, original-hint preservation, and the zero-applied single-list and bulk-first-PUT paths. * Reject wrong-type URL args to todolists position A pasted Basecamp URL for a different recording type (a todo, a card, or a todolists *collection* URL) still yields a trailing numeric ID via ExtractIDs, which — because this command mutates — would silently reposition an unrelated todolist that happens to share that ID. Validate any URL argument up front: it must be a non-collection `todolists` URL, mirroring the guard todos position already applies to its --list destination URL. Bare numeric IDs are unaffected. * Warn of partial reorder when a transport failure interrupts a bulk run The raw-error branch of the partial-failure path wrapped the cause with %w and the applied-count prefix, but dropped the recovery hint entirely. After one successful PUT followed by a transport failure (a dropped connection, a canceled context), the operator saw the count but was never told the todoset was left partially reordered and needs a rerun. Return a non-usage structured error that carries the applied-count message, the rerun/intermediate-order hint, and the raw cause (via Cause, so errors.Is/As still unwrap). Code is api_error, never usage. Adds a regression test that drops the connection on the second applied PUT and asserts the non-usage code, the accounting message, the rerun hint, the preserved cause, and that the third list is never touched. * Reconcile SDK pin references and surface after rebase onto e2c1abea Rebasing onto main (#554) advanced the SDK pin to e2c1abea, five commits past the 93ace8d5 this branch first pinned. Refresh the API-COVERAGE SDK line to the merged pin (which additionally carries create-time visible_to_clients support) and regenerate the CLI surface so it reflects both todolists position and main's todolists create --visible-to-clients.
) (#563) Wire create-time client visibility for docs create and uploads create (plus the top-level upload shortcut), closing the create-side gap left by #554. The server only honors visible_to_clients in the project's docked/root Docs & Files vault; for a nested folder it silently inherits the folder's visibility, and the visibility endpoint 403s for nested docs/uploads afterward. A shared resolveVaultClientVisibility helper runs before any mutating request: with no --vault/--folder the target is the root vault (no fetch), and an explicit folder is inspected via Vaults().Get — a non-nil Parent (nested) is a hard error raised before anything is staged. The tri-state *bool preserves the server default when the flag is omitted and sends an explicit false when given. Docs updated (SKILL.md, API-COVERAGE.md) and .surface regenerated.
Add TestTodosShowJSONSurfacesSDKFields, a regression guard for #449: the fields that report (due_on, starts_on, comments_count, boosts_count, completion_subscribers, description_attachments) must all surface in the --json output of `todos show`. The implementation already landed on main via the SDK model — SDK #375 added completion_subscribers/comments_count, SDK #400 added description_attachments, absorbed by CLI #552/#554's SDK bumps; due_on, starts_on and boosts_count already existed with omitempty. `todos show` emits the SDK Todo object directly, so the fields surface today. This is coverage, not a fix (hence Refs, not Fixes). The test serves a fully-populated todo and asserts both the presence and the values of all six fields, keeping the omitempty date/count fields exercised with nonzero values so a future SDK bump can't silently drop them.
Add TestTodosShowJSONSurfacesSDKFields, a regression guard for #449: the fields that report (due_on, starts_on, comments_count, boosts_count, completion_subscribers, description_attachments) must all surface in the --json output of `todos show`. The implementation already landed on main via the SDK model — SDK #375 added completion_subscribers/comments_count, SDK #400 added description_attachments, absorbed by CLI #552/#554's SDK bumps; due_on, starts_on and boosts_count already existed with omitempty. `todos show` emits the SDK Todo object directly, so the fields surface today. This is coverage, not a fix (hence Refs, not Fixes). The test serves a fully-populated todo and asserts both the presence and the values of all six fields, keeping the omitempty date/count fields exercised with nonzero values so a future SDK bump can't silently drop them.
…564) Add TestTodosShowJSONSurfacesSDKFields, a regression guard for #449: the fields that report (due_on, starts_on, comments_count, boosts_count, completion_subscribers, description_attachments) must all surface in the --json output of `todos show`. The implementation already landed on main via the SDK model — SDK #375 added completion_subscribers/comments_count, SDK #400 added description_attachments, absorbed by CLI #552/#554's SDK bumps; due_on, starts_on and boosts_count already existed with omitempty. `todos show` emits the SDK Todo object directly, so the fields surface today. This is coverage, not a fix (hence Refs, not Fixes). The test serves a fully-populated todo and asserts both the presence and the values of all six fields, keeping the omitempty date/count fields exercised with nonzero values so a future SDK bump can't silently drop them.
Adds a
--visible-to-clientsflag to the create commands where Basecamp accepts a top-level client-visibility param, so an agent can post a client-visible recording in a single atomic create instead of a post-hocrecordings visibilitytoggle. Fixes #457.What changed
v0.8.0 → v0.8.1-0.20260724184307-e2c1abea4aea(Add create-time visible_to_clients across the SDKs (+ bundled bc3 repin) basecamp-sdk#401), which adds create-timevisible_to_clientsto the six create-request inputs (tri-state*bool) and repins bc3 provenance.messages create,todolists create,checkins question create,schedule createeach gain--visible-to-clients. The request field is set only when the flag was provided (Flags().Changed, matching--subscribe), so:falseis overridden server-side for client callers);--visible-to-clients→ sendstrue;--visible-to-clients=false→ sends an explicitfalse(tri-state*boolguarantees it isn't dropped).visible_to_clientsfor the unset / true / false cases (16 tests total, incl. flag presence)..surfaceregenerated and the flag documented inskills/basecamp/SKILL.md.SDK-GAP-457.mddeleted — the handoff design doc is realized by this change.Scope
visible_to_clientsunconditionally.docs create/uploads create— accept arbitrary folder IDs; a nested vault inherits folder visibility (silent no-op), so they need docked/root-vault gating before carrying the flag. The SDK already exposes their inputs; only the CLI flag waits.cards,todos,comments— parent-inherited (create ignores the param; the toggle 403s).Collateral
The SDK bundle (#401) also reshaped
SearchMetadata(theProjectslist was replaced byRecordingSearchTypes/FileSearchTypesfilter options).runSearchMetadata's empty-check and summary are adapted to the new shape in a separate commit.Verification
bin/cigreen (build, vet, lint, unit + e2e, surface snapshot, skill drift, SDK provenance, go mod tidy). Manual check against a client-invited project, posting as a team member: a message created with--visible-to-clientsis client-visible; omitting the flag leaves it team-only. (Client-authenticated callers are always forced client-visible — see the flag help and SKILL.md note.)