From a780bf1f3e006a970842c85d4c83b72fd314cb00 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Wed, 22 Jul 2026 13:02:31 -0700 Subject: [PATCH 1/8] Document SDK gap blocking create-time client visibility (#457) 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. --- SDK-GAP-457.md | 110 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+) create mode 100644 SDK-GAP-457.md diff --git a/SDK-GAP-457.md b/SDK-GAP-457.md new file mode 100644 index 000000000..259636b9b --- /dev/null +++ b/SDK-GAP-457.md @@ -0,0 +1,110 @@ +# SDK Gap: create-time `visible_to_clients` (CLI issue #457) + +**Status:** blocks basecamp-cli #457 — "Expose client visibility on +recording-creating commands." + +**Lane note:** filed from the CLI repo per the CLI/SDK boundary. The SDK change +itself must be made in [`basecamp/basecamp-sdk`](https://github.com/basecamp/basecamp-sdk); +this file is the communiqué describing what the CLI needs. + +## What the CLI needs + +A way to set a recording's client visibility **at create time**, in the same +POST that creates the record, via the Go SDK's create-request types — so +`basecamp messages create --visible-to-clients` is a single atomic call rather +than a create-then-toggle follow-up. + +## Server already supports it + +Basecamp (bc3 **master**) accepts client visibility at create time as a +**top-level** boolean POST param `visible_to_clients`, a sibling of the +recordable payload: + +```json +POST /buckets/:bucket/messages.json +{ "message": { "subject": "…", "content": "…" }, "visible_to_clients": true } +``` + +Implemented by the `Recording::VisibleToClientsParam` controller concern +(`app/controllers/concerns/recording/visible_to_clients_param.rb`). Semantics: +- Omitted → inherits the parent recording's visibility (falls back to `false`). +- Client users are always forced `true`. +- Documented public API section (`doc/api/sections/client_visibility.md`) only + covers the separate toggle endpoint; the create-time param is implemented but + undocumented there. + +### Recording types that accept it at create (in scope) + +These docked/top-level recordings accept `visible_to_clients` at create and have +a corresponding CLI create command: + +| Recording | Controller | SDK create input | +|----------------|----------------------------------------------|-------------------------| +| Message | `messages_controller.rb` | `CreateMessageInput` | +| Todolist | `todolists_controller.rb` | `CreateTodolistInput` | +| Document | `documents_controller.rb` | `CreateDocumentInput` | +| Check-in question | `questions_controller.rb` | `CreateQuestionInput` | +| Upload | `uploads_controller.rb` / `vaults/uploads_controller.rb` | `CreateUploadInput` | +| Schedule entry | `schedules/entries_controller.rb` | `CreateScheduleEntryInput` | + +(Server also accepts it for cloud files, google documents, and doors — out of +scope for #457 unless the CLI grows create commands for them.) + +### Types that do NOT accept it (must stay unsupported) + +Parent-inherited recordings — the create endpoints ignore the param and the +toggle endpoint 403s: **Kanban cards** (inherit the card table/board), +**individual todos** (inherit the to-do list), **comments** (inherit the parent +recording). The CLI will not offer the flag on `cards create`, `todos create`, +or `comments create`. + +## Current SDK state (gap) + +As of `github.com/basecamp/basecamp-sdk/go@v0.8.0` (and every branch/commit +checked), no create-request type carries a client-visibility field: + +- Wrapper structs `CreateMessageRequest`, `CreateCardRequest`, + `CreateTodoRequest`, `CreateCommentRequest` (`pkg/basecamp/*.go`) — none have + `VisibleToClients`. +- Generated `Create*RequestContent` bodies (`pkg/generated/client.gen.go`) — + none have `visible_to_clients`. +- Smithy `Create*Input` shapes (`spec/basecamp.smithy`) — none declare it. + +Only `SetClientVisibility` exists (`RecordingsService.SetClientVisibility`, +`PUT …/recordings/:id/client_visibility.json`) — a separate call against an +already-created recording. `visible_to_clients` currently appears only on the +**response** types (`Message`, `Card`, `Todo`, …), never on create inputs. + +## Requested SDK change + +For each in-scope create input, add a `visible_to_clients` boolean (optional / +`omitempty` so omitting it preserves the server's inherit-from-parent default — +do **not** send `false` by default): + +1. **Smithy** (`spec/basecamp.smithy`): add an optional `visible_to_clients: + Boolean` member to `CreateMessageInput`, `CreateTodolistInput`, + `CreateDocumentInput`, `CreateQuestionInput`, `CreateUploadInput`, + `CreateScheduleEntryInput`. It is a top-level body field (a sibling of the + nested recordable payload — Rails ParamsWrapper keeps the top-level key + readable, matching how the server reads `params[:visible_to_clients]`). +2. **Regenerate** → adds `VisibleToClients *bool` (or `bool,omitempty`) to the + corresponding `Create*RequestContent` types in `pkg/generated`. +3. **Wrapper structs** (`pkg/basecamp/*.go`): add `VisibleToClients bool + \`json:"visible_to_clients,omitempty"\`` to `CreateMessageRequest` and the + other in-scope `Create*Request` structs. +4. **Mapping**: copy the field through in each `Create` method where the wrapper + is mapped into `generated.Create*JSONRequestBody{…}`. + +Messages is the priority (the issue's core use case); the other five can land in +the same SDK change or follow. + +## CLI wiring that follows (after SDK bump) + +Once the SDK exposes the field and the CLI bumps to it (`make bump-sdk`), each +in-scope create command adds an optional `--visible-to-clients` bool that sets +`req.VisibleToClients` **only when** `cmd.Flags().Changed("visible-to-clients")` +(mirrors the existing `--draft` / `--subscribe` optional-flag pattern in +`internal/commands/messages.go`). No follow-up call, no partial-success path, no +403 handling — the unsupported types simply don't get the flag. Then: tests +asserting the create body carries `visible_to_clients`, regenerate `.surface`, +update `skills/basecamp/SKILL.md`, `bin/ci` green. From b3d543191ad7bdf912d95b5aebf4a43ccdef4908 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Wed, 22 Jul 2026 13:41:24 -0700 Subject: [PATCH 2/8] Correct SDK gap handoff: flat message wire, tri-state *bool, defer docs/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. --- SDK-GAP-457.md | 198 ++++++++++++++++++++++++++++++------------------- 1 file changed, 121 insertions(+), 77 deletions(-) diff --git a/SDK-GAP-457.md b/SDK-GAP-457.md index 259636b9b..4ec9154a6 100644 --- a/SDK-GAP-457.md +++ b/SDK-GAP-457.md @@ -3,108 +3,152 @@ **Status:** blocks basecamp-cli #457 — "Expose client visibility on recording-creating commands." -**Lane note:** filed from the CLI repo per the CLI/SDK boundary. The SDK change -itself must be made in [`basecamp/basecamp-sdk`](https://github.com/basecamp/basecamp-sdk); -this file is the communiqué describing what the CLI needs. +**Lane note:** filed from the CLI repo. Repo policy (`AGENTS.md`) forbids +bypassing the SDK wrappers with raw generated-client calls and directs SDK +blockers to an SDK issue; it does not categorically forbid SDK-repo changes. This +document is the design communiqué behind that SDK issue. + +**SDK tracking issue (the unblocker):** +[basecamp/basecamp-sdk#395](https://github.com/basecamp/basecamp-sdk/issues/395). +That issue, not this file, is the durable record. + +**Lifecycle:** this file is a transient handoff artifact. It is deleted in the +same commit that wires the feature once the SDK unblocks; the SDK issue and PR +#554 remain the durable record. ## What the CLI needs A way to set a recording's client visibility **at create time**, in the same -POST that creates the record, via the Go SDK's create-request types — so -`basecamp messages create --visible-to-clients` is a single atomic call rather -than a create-then-toggle follow-up. +POST that creates the record, through the Go SDK's create-request types — so +`basecamp messages create --visible-to-clients` is one atomic call rather than a +create-then-toggle follow-up. ## Server already supports it -Basecamp (bc3 **master**) accepts client visibility at create time as a -**top-level** boolean POST param `visible_to_clients`, a sibling of the -recordable payload: +Basecamp (bc3 **master**) accepts a top-level boolean `visible_to_clients` on the +create request. The field is a sibling of the recordable fields in the POST body. +For messages the request is: + +``` +POST /message_boards/:board_id/messages.json +``` ```json -POST /buckets/:bucket/messages.json -{ "message": { "subject": "…", "content": "…" }, "visible_to_clients": true } +{ "subject": "…", "content": "…", "visible_to_clients": true } ``` +(The message fields are flat in the SDK/wire body — Rails ParamsWrapper wraps +them into `params[:message]` server-side while `visible_to_clients` stays a +top-level key, which is what the controller reads. A legacy route +`/buckets/:bucket/message_boards/:board_id/messages.json` also exists.) + Implemented by the `Recording::VisibleToClientsParam` controller concern (`app/controllers/concerns/recording/visible_to_clients_param.rb`). Semantics: -- Omitted → inherits the parent recording's visibility (falls back to `false`). +- **Omitted → inherit** the parent recording's visibility (falls back to + `false`). This is why the SDK field must be tri-state (see below): absent must + mean "inherit", explicit `false` must mean "team-only". - Client users are always forced `true`. -- Documented public API section (`doc/api/sections/client_visibility.md`) only - covers the separate toggle endpoint; the create-time param is implemented but - undocumented there. - -### Recording types that accept it at create (in scope) - -These docked/top-level recordings accept `visible_to_clients` at create and have -a corresponding CLI create command: - -| Recording | Controller | SDK create input | -|----------------|----------------------------------------------|-------------------------| -| Message | `messages_controller.rb` | `CreateMessageInput` | -| Todolist | `todolists_controller.rb` | `CreateTodolistInput` | -| Document | `documents_controller.rb` | `CreateDocumentInput` | -| Check-in question | `questions_controller.rb` | `CreateQuestionInput` | -| Upload | `uploads_controller.rb` / `vaults/uploads_controller.rb` | `CreateUploadInput` | -| Schedule entry | `schedules/entries_controller.rb` | `CreateScheduleEntryInput` | - -(Server also accepts it for cloud files, google documents, and doors — out of -scope for #457 unless the CLI grows create commands for them.) - -### Types that do NOT accept it (must stay unsupported) - -Parent-inherited recordings — the create endpoints ignore the param and the -toggle endpoint 403s: **Kanban cards** (inherit the card table/board), -**individual todos** (inherit the to-do list), **comments** (inherit the parent -recording). The CLI will not offer the flag on `cards create`, `todos create`, -or `comments create`. - -## Current SDK state (gap) - -As of `github.com/basecamp/basecamp-sdk/go@v0.8.0` (and every branch/commit -checked), no create-request type carries a client-visibility field: - -- Wrapper structs `CreateMessageRequest`, `CreateCardRequest`, - `CreateTodoRequest`, `CreateCommentRequest` (`pkg/basecamp/*.go`) — none have - `VisibleToClients`. -- Generated `Create*RequestContent` bodies (`pkg/generated/client.gen.go`) — - none have `visible_to_clients`. -- Smithy `Create*Input` shapes (`spec/basecamp.smithy`) — none declare it. - -Only `SetClientVisibility` exists (`RecordingsService.SetClientVisibility`, -`PUT …/recordings/:id/client_visibility.json`) — a separate call against an -already-created recording. `visible_to_clients` currently appears only on the +- The documented public API section (`doc/api/sections/client_visibility.md`) + only covers the separate toggle endpoint; the create-time param is implemented + but undocumented there. + +### Recording types that accept it at create + +Docked/top-level recordings whose create controller includes the concern **and** +that have a CLI create command: + +| Recording | Controller | SDK create input | Initial scope | +|-------------------|-----------------------------------|----------------------------|---------------| +| Message | `messages_controller.rb` | `CreateMessageInput` | yes (priority) | +| Todolist | `todolists_controller.rb` | `CreateTodolistInput` | yes | +| Check-in question | `questions_controller.rb` | `CreateQuestionInput` | yes | +| Schedule entry | `schedules/entries_controller.rb` | `CreateScheduleEntryInput` | yes | +| Document | `documents_controller.rb` | `CreateDocumentInput` | **deferred** (see below) | +| Upload | `uploads_controller.rb`, `vaults/uploads_controller.rb` | `CreateUploadInput` | **deferred** (see below) | + +**Documents and uploads are deferred, not silently included.** The CLI's `docs +create` and `uploads create` accept an arbitrary target folder via +`--vault`/`--folder` (`internal/commands/files.go:623`, +`internal/commands/files.go:923`). For a **nested** vault, BC3 ignores the +explicit `visible_to_clients` param and inherits the folder's visibility — so the +flag would be a silent no-op there, which is unacceptable. Before these two +commands carry the flag, the CLI must first verify the target is the docked/root +vault (or reject the flag for nested folders). Tracked as follow-up. + +(The server also accepts the param for cloud files, google documents, and doors — +out of scope for #457; no CLI create commands.) + +### Types that must stay unsupported + +Parent-inherited recordings — create ignores the param and the toggle endpoint +403s: **Kanban cards** (inherit the card table/board), **individual todos** +(inherit the to-do list), **comments** (inherit the parent recording). The CLI +will not offer the flag on `cards create`, `todos create`, or `comments create`. + +## Current SDK state (the gap) + +Verified against the pinned revision **`github.com/basecamp/basecamp-sdk/go +v0.8.0`** (the version this CLI builds against). No create-request path carries a +client-visibility field: + +- Wrapper structs `CreateMessageRequest`, `CreateTodoRequest`, + `CreateCardRequest`, `CreateCommentRequest` (`pkg/basecamp/*.go`). +- Generated `Create*RequestContent` bodies (`pkg/generated/client.gen.go`). +- Smithy `Create*Input` shapes (`spec/basecamp.smithy`). + +Only `RecordingsService.SetClientVisibility` (`PUT +…/recordings/:id/client_visibility.json`) exists — a separate call against an +already-created recording. `visible_to_clients` otherwise appears only on the **response** types (`Message`, `Card`, `Todo`, …), never on create inputs. ## Requested SDK change -For each in-scope create input, add a `visible_to_clients` boolean (optional / -`omitempty` so omitting it preserves the server's inherit-from-parent default — -do **not** send `false` by default): +For each in-scope create input, add a **tri-state** client-visibility field. Use +`*bool` end-to-end (not `bool` with `omitempty`) so the SDK can distinguish three +states: absent (`nil` → inherit from parent), explicit `true`, and explicit +`false` (team-only). `bool,omitempty` cannot represent explicit `false` — it +would be dropped from the body and silently inherit. + +This mirrors the SDK's existing precedent for `AllDay` +(`pkg/basecamp/schedules.go:97,123` — `AllDay *bool +\`json:"all_day,omitempty"\``; see `TestSchedulesService_UpdateEntryAllDay`, +which asserts that setting it to `false` sends `false` rather than omitting it). 1. **Smithy** (`spec/basecamp.smithy`): add an optional `visible_to_clients: Boolean` member to `CreateMessageInput`, `CreateTodolistInput`, - `CreateDocumentInput`, `CreateQuestionInput`, `CreateUploadInput`, - `CreateScheduleEntryInput`. It is a top-level body field (a sibling of the - nested recordable payload — Rails ParamsWrapper keeps the top-level key - readable, matching how the server reads `params[:visible_to_clients]`). -2. **Regenerate** → adds `VisibleToClients *bool` (or `bool,omitempty`) to the - corresponding `Create*RequestContent` types in `pkg/generated`. -3. **Wrapper structs** (`pkg/basecamp/*.go`): add `VisibleToClients bool + `CreateQuestionInput`, `CreateScheduleEntryInput` (and later + `CreateDocumentInput`, `CreateUploadInput` once the CLI handles nested + vaults). Top-level body field. +2. **Regenerate** → `*bool` `VisibleToClients` on the corresponding + `Create*RequestContent` types in `pkg/generated`. +3. **Wrapper structs** (`pkg/basecamp/*.go`): add `VisibleToClients *bool \`json:"visible_to_clients,omitempty"\`` to `CreateMessageRequest` and the other in-scope `Create*Request` structs. -4. **Mapping**: copy the field through in each `Create` method where the wrapper - is mapped into `generated.Create*JSONRequestBody{…}`. +4. **Mapping**: pass the pointer through in each `Create` method where the + wrapper is mapped into `generated.Create*JSONRequestBody{…}`. +5. **Tests**: cover `nil` (field omitted from body), `true` (`"visible_to_clients": + true`), and `false` (`"visible_to_clients": false` — present, not dropped). -Messages is the priority (the issue's core use case); the other five can land in -the same SDK change or follow. +Messages is the priority (the issue's core use case); todolists, check-in +questions, and schedule entries can land in the same SDK change or follow. ## CLI wiring that follows (after SDK bump) Once the SDK exposes the field and the CLI bumps to it (`make bump-sdk`), each -in-scope create command adds an optional `--visible-to-clients` bool that sets -`req.VisibleToClients` **only when** `cmd.Flags().Changed("visible-to-clients")` -(mirrors the existing `--draft` / `--subscribe` optional-flag pattern in -`internal/commands/messages.go`). No follow-up call, no partial-success path, no -403 handling — the unsupported types simply don't get the flag. Then: tests -asserting the create body carries `visible_to_clients`, regenerate `.surface`, -update `skills/basecamp/SKILL.md`, `bin/ci` green. +in-scope create command gains an optional `--visible-to-clients` bool. Set the +pointer **only when the flag was provided**, so the default stays +inherit-from-parent: + +```go +if cmd.Flags().Changed("visible-to-clients") { + req.VisibleToClients = &visibleToClients +} +``` + +(The gate-on-`Changed` pattern matches `--subscribe` in +`internal/commands/messages.go`; the `*bool` tri-state matches `AllDay`.) A +single atomic create call — no follow-up, no partial-success path, no 403 +handling, since the unsupported types don't get the flag. Then: tests asserting +the create body carries `visible_to_clients` for nil/true/false, regenerate +`.surface`, update `skills/basecamp/SKILL.md`, `bin/ci` green. Delete this file in +that commit and flip PR #554 to ready (`Refs #457` → `Fixes #457`). From 32f6daadbe332868656869cec95a46bbc454bcbe Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Wed, 22 Jul 2026 14:02:46 -0700 Subject: [PATCH 3/8] Refine SDK gap handoff: de-reference PR number, stable identifiers, link 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). --- SDK-GAP-457.md | 40 +++++++++++++++++++++++++++------------- 1 file changed, 27 insertions(+), 13 deletions(-) diff --git a/SDK-GAP-457.md b/SDK-GAP-457.md index 4ec9154a6..1bbd2096b 100644 --- a/SDK-GAP-457.md +++ b/SDK-GAP-457.md @@ -13,8 +13,8 @@ document is the design communiqué behind that SDK issue. That issue, not this file, is the durable record. **Lifecycle:** this file is a transient handoff artifact. It is deleted in the -same commit that wires the feature once the SDK unblocks; the SDK issue and PR -#554 remain the durable record. +same commit that wires the feature once the SDK unblocks; the SDK issue and this +PR remain the durable record. ## What the CLI needs @@ -66,14 +66,19 @@ that have a CLI create command: | Document | `documents_controller.rb` | `CreateDocumentInput` | **deferred** (see below) | | Upload | `uploads_controller.rb`, `vaults/uploads_controller.rb` | `CreateUploadInput` | **deferred** (see below) | -**Documents and uploads are deferred, not silently included.** The CLI's `docs -create` and `uploads create` accept an arbitrary target folder via -`--vault`/`--folder` (`internal/commands/files.go:623`, -`internal/commands/files.go:923`). For a **nested** vault, BC3 ignores the -explicit `visible_to_clients` param and inherits the folder's visibility — so the -flag would be a silent no-op there, which is unacceptable. Before these two -commands carry the flag, the CLI must first verify the target is the docked/root -vault (or reject the flag for nested folders). Tracked as follow-up. +**Documents and uploads are deferred at the CLI layer, not silently included.** +The CLI's `docs create` and `uploads create` (`newDocsCreateCmd` / +`newUploadsCreateCmd` in `internal/commands/files.go`) accept an arbitrary target +folder via `--vault`/`--folder`. For a **nested** vault, BC3 ignores the explicit +`visible_to_clients` param and inherits the folder's visibility — so the flag +would be a silent no-op there, which is unacceptable. Before these two commands +carry the flag, the CLI must first verify the target is the docked/root vault (or +reject the flag for nested folders). Tracked in basecamp-cli **#556**. + +Note this is a **CLI-UX** constraint, not an SDK one: the SDK should still expose +`visible_to_clients` on `CreateDocumentInput` / `CreateUploadInput` for +completeness (the server accepts it), and #395 covers all six inputs. The CLI +just withholds the *flag* on those two commands until #556 adds the gating. (The server also accepts the param for cloud files, google documents, and doors — out of scope for #457; no CLI create commands.) @@ -109,8 +114,8 @@ states: absent (`nil` → inherit from parent), explicit `true`, and explicit `false` (team-only). `bool,omitempty` cannot represent explicit `false` — it would be dropped from the body and silently inherit. -This mirrors the SDK's existing precedent for `AllDay` -(`pkg/basecamp/schedules.go:97,123` — `AllDay *bool +This mirrors the SDK's existing precedent for `AllDay` on the schedule-entry +request structs in `pkg/basecamp/schedules.go` (`AllDay *bool \`json:"all_day,omitempty"\``; see `TestSchedulesService_UpdateEntryAllDay`, which asserts that setting it to `false` sends `false` rather than omitting it). @@ -131,6 +136,15 @@ which asserts that setting it to `false` sends `false` rather than omitting it). Messages is the priority (the issue's core use case); todolists, check-in questions, and schedule entries can land in the same SDK change or follow. +Documents and uploads are in scope for the SDK (completeness) even though the CLI +defers their flag — see #556. + +**#457 closure:** the intent is a single CLI PR wiring the four initial commands +(messages, todolists, check-in questions, schedule) once their SDK inputs land, +closing #457; docs/uploads follow via #556. Because #395 prioritizes messages, if +the message input lands well ahead of the others we may instead ship messages +first and close #457, moving the remaining three to a follow-up — a sequencing +call to make when the SDK unblocks, not now. ## CLI wiring that follows (after SDK bump) @@ -151,4 +165,4 @@ single atomic create call — no follow-up, no partial-success path, no 403 handling, since the unsupported types don't get the flag. Then: tests asserting the create body carries `visible_to_clients` for nil/true/false, regenerate `.surface`, update `skills/basecamp/SKILL.md`, `bin/ci` green. Delete this file in -that commit and flip PR #554 to ready (`Refs #457` → `Fixes #457`). +that commit and flip this PR to ready (`Refs #457` → `Fixes #457`). From 0624823f5efc41d5782c12f635d262093a62fd50 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Wed, 22 Jul 2026 14:09:28 -0700 Subject: [PATCH 4/8] Make SDK gap internally consistent: all six inputs, correct wrappers, 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. --- SDK-GAP-457.md | 36 +++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/SDK-GAP-457.md b/SDK-GAP-457.md index 1bbd2096b..4f34a04f4 100644 --- a/SDK-GAP-457.md +++ b/SDK-GAP-457.md @@ -93,11 +93,12 @@ will not offer the flag on `cards create`, `todos create`, or `comments create`. ## Current SDK state (the gap) Verified against the pinned revision **`github.com/basecamp/basecamp-sdk/go -v0.8.0`** (the version this CLI builds against). No create-request path carries a -client-visibility field: +v0.8.0`** (the version this CLI builds against). None of the six target +create-request paths carry a client-visibility field: -- Wrapper structs `CreateMessageRequest`, `CreateTodoRequest`, - `CreateCardRequest`, `CreateCommentRequest` (`pkg/basecamp/*.go`). +- Wrapper structs `CreateMessageRequest`, `CreateTodolistRequest`, + `CreateQuestionRequest`, `CreateScheduleEntryRequest`, `CreateDocumentRequest`, + `CreateUploadRequest` (`pkg/basecamp/*.go`) — all exist, none has the field. - Generated `Create*RequestContent` bodies (`pkg/generated/client.gen.go`). - Smithy `Create*Input` shapes (`spec/basecamp.smithy`). @@ -119,16 +120,20 @@ request structs in `pkg/basecamp/schedules.go` (`AllDay *bool \`json:"all_day,omitempty"\``; see `TestSchedulesService_UpdateEntryAllDay`, which asserts that setting it to `false` sends `false` rather than omitting it). +Apply to **all six** server-supported inputs (SDK completeness — the CLI's +deferral of the document/upload *flag* per #556 is a CLI-UX concern that must not +narrow the SDK): + 1. **Smithy** (`spec/basecamp.smithy`): add an optional `visible_to_clients: Boolean` member to `CreateMessageInput`, `CreateTodolistInput`, - `CreateQuestionInput`, `CreateScheduleEntryInput` (and later - `CreateDocumentInput`, `CreateUploadInput` once the CLI handles nested - vaults). Top-level body field. + `CreateQuestionInput`, `CreateScheduleEntryInput`, `CreateDocumentInput`, and + `CreateUploadInput`. Top-level body field. 2. **Regenerate** → `*bool` `VisibleToClients` on the corresponding `Create*RequestContent` types in `pkg/generated`. 3. **Wrapper structs** (`pkg/basecamp/*.go`): add `VisibleToClients *bool - \`json:"visible_to_clients,omitempty"\`` to `CreateMessageRequest` and the - other in-scope `Create*Request` structs. + \`json:"visible_to_clients,omitempty"\`` to `CreateMessageRequest`, + `CreateTodolistRequest`, `CreateQuestionRequest`, `CreateScheduleEntryRequest`, + `CreateDocumentRequest`, and `CreateUploadRequest`. 4. **Mapping**: pass the pointer through in each `Create` method where the wrapper is mapped into `generated.Create*JSONRequestBody{…}`. 5. **Tests**: cover `nil` (field omitted from body), `true` (`"visible_to_clients": @@ -139,12 +144,13 @@ questions, and schedule entries can land in the same SDK change or follow. Documents and uploads are in scope for the SDK (completeness) even though the CLI defers their flag — see #556. -**#457 closure:** the intent is a single CLI PR wiring the four initial commands -(messages, todolists, check-in questions, schedule) once their SDK inputs land, -closing #457; docs/uploads follow via #556. Because #395 prioritizes messages, if -the message input lands well ahead of the others we may instead ship messages -first and close #457, moving the remaining three to a follow-up — a sequencing -call to make when the SDK unblocks, not now. +**#457 closure (locked):** #457 completes only when all four initial commands — +messages, todolists, check-in questions, schedule entries — are wired. If the +messages SDK input lands first, shipping a messages-only PR is fine, but it keeps +`Refs #457` and leaves #457 **open**; the switch to `Fixes #457` happens only once +all four are wired. This fixes a stable completion boundary rather than deciding +it under merge pressure. Docs/uploads remain a separate follow-up (#556) and are +not part of the #457 boundary. ## CLI wiring that follows (after SDK bump) From 759d6c48f36dcb771b36df7f096a145e4769c204 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Fri, 24 Jul 2026 12:04:58 -0700 Subject: [PATCH 5/8] Bump SDK to e2c1abea for create-time visible_to_clients 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. --- go.mod | 2 +- go.sum | 4 ++-- internal/commands/search.go | 7 ++++--- internal/version/sdk-provenance.json | 10 +++++----- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/go.mod b/go.mod index 0e7c5fe29..c3a63cb9a 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( charm.land/bubbles/v2 v2.1.1 charm.land/bubbletea/v2 v2.0.8 charm.land/lipgloss/v2 v2.0.5 - github.com/basecamp/basecamp-sdk/go v0.8.0 + github.com/basecamp/basecamp-sdk/go v0.8.1-0.20260724184307-e2c1abea4aea github.com/basecamp/cli v0.2.1 github.com/charmbracelet/bubbles v1.0.0 github.com/charmbracelet/glamour v1.0.0 diff --git a/go.sum b/go.sum index 6bf9c4bc9..14c85fd96 100644 --- a/go.sum +++ b/go.sum @@ -23,8 +23,8 @@ github.com/aymanbagabas/go-udiff v0.4.1 h1:OEIrQ8maEeDBXQDoGCbbTTXYJMYRCRO1fnodZ github.com/aymanbagabas/go-udiff v0.4.1/go.mod h1:0L9PGwj20lrtmEMeyw4WKJ/TMyDtvAoK9bf2u/mNo3w= github.com/aymerick/douceur v0.2.0 h1:Mv+mAeH1Q+n9Fr+oyamOlAkUNPWPlA8PPGR0QAaYuPk= github.com/aymerick/douceur v0.2.0/go.mod h1:wlT5vV2O3h55X9m7iVYN0TBM0NH/MmbLnd30/FjWUq4= -github.com/basecamp/basecamp-sdk/go v0.8.0 h1:ZxrNTyGTMcGocGE582ZMmpjyodvODD2+fZMjISokTYY= -github.com/basecamp/basecamp-sdk/go v0.8.0/go.mod h1:eX5mEKCdtxSfEL4P/n5AwOl21JVA/K+gRPic/Hd8W/Y= +github.com/basecamp/basecamp-sdk/go v0.8.1-0.20260724184307-e2c1abea4aea h1:dRwhvhnzbyXxitGbb3uGFaUIJHazztD9UgWC0XzM870= +github.com/basecamp/basecamp-sdk/go v0.8.1-0.20260724184307-e2c1abea4aea/go.mod h1:eX5mEKCdtxSfEL4P/n5AwOl21JVA/K+gRPic/Hd8W/Y= github.com/basecamp/cli v0.2.1 h1:8GyehPVtsTXla0oOPu4QgXRjwwzJ99prlByvyi+0HRQ= github.com/basecamp/cli v0.2.1/go.mod h1:p8tt/DatJ2LAzWO6N6tNfV8x3gF5T3IxDTo+U8FfWPo= github.com/bmatcuk/doublestar v1.1.1/go.mod h1:UD6OnuiIn0yFxxA2le/rnRU1G4RaI4UvFv1sNto9p6w= diff --git a/internal/commands/search.go b/internal/commands/search.go index 9c1cb478a..ef9a67810 100644 --- a/internal/commands/search.go +++ b/internal/commands/search.go @@ -202,14 +202,15 @@ func runSearchMetadata(cmd *cobra.Command, app *appctx.App) error { } // Handle empty response - if metadata == nil || len(metadata.Projects) == 0 { + if metadata == nil || len(metadata.RecordingSearchTypes) == 0 && len(metadata.FileSearchTypes) == 0 { return output.ErrUsageHint( "Search metadata not available", - "No projects available for search filtering", + "No search filters available", ) } - summary := fmt.Sprintf("Available projects: %d", len(metadata.Projects)) + summary := fmt.Sprintf("Search filters: %d recording types, %d file types", + len(metadata.RecordingSearchTypes), len(metadata.FileSearchTypes)) return app.OK(metadata, output.WithSummary(summary), diff --git a/internal/version/sdk-provenance.json b/internal/version/sdk-provenance.json index 42fd0755a..01255b7e0 100644 --- a/internal/version/sdk-provenance.json +++ b/internal/version/sdk-provenance.json @@ -1,13 +1,13 @@ { "sdk": { "module": "github.com/basecamp/basecamp-sdk/go", - "version": "v0.8.0", - "revision": "f55a6b51a9be", - "updated_at": "2026-07-22T08:57:07Z" + "version": "v0.8.1-0.20260724184307-e2c1abea4aea", + "revision": "e2c1abea4aea", + "updated_at": "2026-07-24T18:43:07Z" }, "api": { "repo": "basecamp/bc3", - "revision": "ba105ba7d7e48bd97afdc98305e9fb8a63a88beb", - "synced_at": "2026-07-22" + "revision": "ca1d34bcc40d2e2403a08bce9ead0d4b0276b1fa", + "synced_at": "2026-07-24" } } From 927bbca6c4c2d4feb018c16d880feacc893dbdff Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Fri, 24 Jul 2026 12:05:07 -0700 Subject: [PATCH 6/8] feat(clients): add --visible-to-clients to create commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .surface | 7 ++ SDK-GAP-457.md | 174 ---------------------------- internal/commands/checkins.go | 8 ++ internal/commands/checkins_test.go | 89 ++++++++++++++ internal/commands/messages.go | 8 ++ internal/commands/messages_test.go | 66 +++++++++++ internal/commands/schedule.go | 9 ++ internal/commands/schedule_test.go | 65 +++++++++++ internal/commands/todolists.go | 8 ++ internal/commands/todolists_test.go | 112 ++++++++++++++++++ skills/basecamp/SKILL.md | 14 ++- 11 files changed, 384 insertions(+), 176 deletions(-) delete mode 100644 SDK-GAP-457.md create mode 100644 internal/commands/todolists_test.go diff --git a/.surface b/.surface index 84f0339bb..c2f965d2e 100644 --- a/.surface +++ b/.surface @@ -3503,6 +3503,7 @@ FLAG basecamp checkin question create --styled type=bool FLAG basecamp checkin question create --time type=string FLAG basecamp checkin question create --todolist type=string FLAG basecamp checkin question create --verbose type=count +FLAG basecamp checkin question create --visible-to-clients type=bool FLAG basecamp checkin question show --account type=string FLAG basecamp checkin question show --agent type=bool FLAG basecamp checkin question show --all-comments type=bool @@ -3772,6 +3773,7 @@ FLAG basecamp checkins question create --styled type=bool FLAG basecamp checkins question create --time type=string FLAG basecamp checkins question create --todolist type=string FLAG basecamp checkins question create --verbose type=count +FLAG basecamp checkins question create --visible-to-clients type=bool FLAG basecamp checkins question show --account type=string FLAG basecamp checkins question show --agent type=bool FLAG basecamp checkins question show --all-comments type=bool @@ -9567,6 +9569,7 @@ FLAG basecamp messages create --styled type=bool FLAG basecamp messages create --subscribe type=string FLAG basecamp messages create --todolist type=string FLAG basecamp messages create --verbose type=count +FLAG basecamp messages create --visible-to-clients type=bool FLAG basecamp messages list --account type=string FLAG basecamp messages list --agent type=bool FLAG basecamp messages list --all type=bool @@ -9977,6 +9980,7 @@ FLAG basecamp msgs create --styled type=bool FLAG basecamp msgs create --subscribe type=string FLAG basecamp msgs create --todolist type=string FLAG basecamp msgs create --verbose type=count +FLAG basecamp msgs create --visible-to-clients type=bool FLAG basecamp msgs list --account type=string FLAG basecamp msgs list --agent type=bool FLAG basecamp msgs list --all type=bool @@ -11205,6 +11209,7 @@ FLAG basecamp schedule create --summary type=string FLAG basecamp schedule create --title type=string FLAG basecamp schedule create --todolist type=string FLAG basecamp schedule create --verbose type=count +FLAG basecamp schedule create --visible-to-clients type=bool FLAG basecamp schedule entries --account type=string FLAG basecamp schedule entries --agent type=bool FLAG basecamp schedule entries --all type=bool @@ -12426,6 +12431,7 @@ FLAG basecamp todolist create --styled type=bool FLAG basecamp todolist create --todolist type=string FLAG basecamp todolist create --todoset type=string FLAG basecamp todolist create --verbose type=count +FLAG basecamp todolist create --visible-to-clients type=bool FLAG basecamp todolist list --account type=string FLAG basecamp todolist list --agent type=bool FLAG basecamp todolist list --all type=bool @@ -12968,6 +12974,7 @@ FLAG basecamp todolists create --styled type=bool FLAG basecamp todolists create --todolist type=string FLAG basecamp todolists create --todoset type=string FLAG basecamp todolists create --verbose type=count +FLAG basecamp todolists create --visible-to-clients type=bool FLAG basecamp todolists list --account type=string FLAG basecamp todolists list --agent type=bool FLAG basecamp todolists list --all type=bool diff --git a/SDK-GAP-457.md b/SDK-GAP-457.md deleted file mode 100644 index 4f34a04f4..000000000 --- a/SDK-GAP-457.md +++ /dev/null @@ -1,174 +0,0 @@ -# SDK Gap: create-time `visible_to_clients` (CLI issue #457) - -**Status:** blocks basecamp-cli #457 — "Expose client visibility on -recording-creating commands." - -**Lane note:** filed from the CLI repo. Repo policy (`AGENTS.md`) forbids -bypassing the SDK wrappers with raw generated-client calls and directs SDK -blockers to an SDK issue; it does not categorically forbid SDK-repo changes. This -document is the design communiqué behind that SDK issue. - -**SDK tracking issue (the unblocker):** -[basecamp/basecamp-sdk#395](https://github.com/basecamp/basecamp-sdk/issues/395). -That issue, not this file, is the durable record. - -**Lifecycle:** this file is a transient handoff artifact. It is deleted in the -same commit that wires the feature once the SDK unblocks; the SDK issue and this -PR remain the durable record. - -## What the CLI needs - -A way to set a recording's client visibility **at create time**, in the same -POST that creates the record, through the Go SDK's create-request types — so -`basecamp messages create --visible-to-clients` is one atomic call rather than a -create-then-toggle follow-up. - -## Server already supports it - -Basecamp (bc3 **master**) accepts a top-level boolean `visible_to_clients` on the -create request. The field is a sibling of the recordable fields in the POST body. -For messages the request is: - -``` -POST /message_boards/:board_id/messages.json -``` - -```json -{ "subject": "…", "content": "…", "visible_to_clients": true } -``` - -(The message fields are flat in the SDK/wire body — Rails ParamsWrapper wraps -them into `params[:message]` server-side while `visible_to_clients` stays a -top-level key, which is what the controller reads. A legacy route -`/buckets/:bucket/message_boards/:board_id/messages.json` also exists.) - -Implemented by the `Recording::VisibleToClientsParam` controller concern -(`app/controllers/concerns/recording/visible_to_clients_param.rb`). Semantics: -- **Omitted → inherit** the parent recording's visibility (falls back to - `false`). This is why the SDK field must be tri-state (see below): absent must - mean "inherit", explicit `false` must mean "team-only". -- Client users are always forced `true`. -- The documented public API section (`doc/api/sections/client_visibility.md`) - only covers the separate toggle endpoint; the create-time param is implemented - but undocumented there. - -### Recording types that accept it at create - -Docked/top-level recordings whose create controller includes the concern **and** -that have a CLI create command: - -| Recording | Controller | SDK create input | Initial scope | -|-------------------|-----------------------------------|----------------------------|---------------| -| Message | `messages_controller.rb` | `CreateMessageInput` | yes (priority) | -| Todolist | `todolists_controller.rb` | `CreateTodolistInput` | yes | -| Check-in question | `questions_controller.rb` | `CreateQuestionInput` | yes | -| Schedule entry | `schedules/entries_controller.rb` | `CreateScheduleEntryInput` | yes | -| Document | `documents_controller.rb` | `CreateDocumentInput` | **deferred** (see below) | -| Upload | `uploads_controller.rb`, `vaults/uploads_controller.rb` | `CreateUploadInput` | **deferred** (see below) | - -**Documents and uploads are deferred at the CLI layer, not silently included.** -The CLI's `docs create` and `uploads create` (`newDocsCreateCmd` / -`newUploadsCreateCmd` in `internal/commands/files.go`) accept an arbitrary target -folder via `--vault`/`--folder`. For a **nested** vault, BC3 ignores the explicit -`visible_to_clients` param and inherits the folder's visibility — so the flag -would be a silent no-op there, which is unacceptable. Before these two commands -carry the flag, the CLI must first verify the target is the docked/root vault (or -reject the flag for nested folders). Tracked in basecamp-cli **#556**. - -Note this is a **CLI-UX** constraint, not an SDK one: the SDK should still expose -`visible_to_clients` on `CreateDocumentInput` / `CreateUploadInput` for -completeness (the server accepts it), and #395 covers all six inputs. The CLI -just withholds the *flag* on those two commands until #556 adds the gating. - -(The server also accepts the param for cloud files, google documents, and doors — -out of scope for #457; no CLI create commands.) - -### Types that must stay unsupported - -Parent-inherited recordings — create ignores the param and the toggle endpoint -403s: **Kanban cards** (inherit the card table/board), **individual todos** -(inherit the to-do list), **comments** (inherit the parent recording). The CLI -will not offer the flag on `cards create`, `todos create`, or `comments create`. - -## Current SDK state (the gap) - -Verified against the pinned revision **`github.com/basecamp/basecamp-sdk/go -v0.8.0`** (the version this CLI builds against). None of the six target -create-request paths carry a client-visibility field: - -- Wrapper structs `CreateMessageRequest`, `CreateTodolistRequest`, - `CreateQuestionRequest`, `CreateScheduleEntryRequest`, `CreateDocumentRequest`, - `CreateUploadRequest` (`pkg/basecamp/*.go`) — all exist, none has the field. -- Generated `Create*RequestContent` bodies (`pkg/generated/client.gen.go`). -- Smithy `Create*Input` shapes (`spec/basecamp.smithy`). - -Only `RecordingsService.SetClientVisibility` (`PUT -…/recordings/:id/client_visibility.json`) exists — a separate call against an -already-created recording. `visible_to_clients` otherwise appears only on the -**response** types (`Message`, `Card`, `Todo`, …), never on create inputs. - -## Requested SDK change - -For each in-scope create input, add a **tri-state** client-visibility field. Use -`*bool` end-to-end (not `bool` with `omitempty`) so the SDK can distinguish three -states: absent (`nil` → inherit from parent), explicit `true`, and explicit -`false` (team-only). `bool,omitempty` cannot represent explicit `false` — it -would be dropped from the body and silently inherit. - -This mirrors the SDK's existing precedent for `AllDay` on the schedule-entry -request structs in `pkg/basecamp/schedules.go` (`AllDay *bool -\`json:"all_day,omitempty"\``; see `TestSchedulesService_UpdateEntryAllDay`, -which asserts that setting it to `false` sends `false` rather than omitting it). - -Apply to **all six** server-supported inputs (SDK completeness — the CLI's -deferral of the document/upload *flag* per #556 is a CLI-UX concern that must not -narrow the SDK): - -1. **Smithy** (`spec/basecamp.smithy`): add an optional `visible_to_clients: - Boolean` member to `CreateMessageInput`, `CreateTodolistInput`, - `CreateQuestionInput`, `CreateScheduleEntryInput`, `CreateDocumentInput`, and - `CreateUploadInput`. Top-level body field. -2. **Regenerate** → `*bool` `VisibleToClients` on the corresponding - `Create*RequestContent` types in `pkg/generated`. -3. **Wrapper structs** (`pkg/basecamp/*.go`): add `VisibleToClients *bool - \`json:"visible_to_clients,omitempty"\`` to `CreateMessageRequest`, - `CreateTodolistRequest`, `CreateQuestionRequest`, `CreateScheduleEntryRequest`, - `CreateDocumentRequest`, and `CreateUploadRequest`. -4. **Mapping**: pass the pointer through in each `Create` method where the - wrapper is mapped into `generated.Create*JSONRequestBody{…}`. -5. **Tests**: cover `nil` (field omitted from body), `true` (`"visible_to_clients": - true`), and `false` (`"visible_to_clients": false` — present, not dropped). - -Messages is the priority (the issue's core use case); todolists, check-in -questions, and schedule entries can land in the same SDK change or follow. -Documents and uploads are in scope for the SDK (completeness) even though the CLI -defers their flag — see #556. - -**#457 closure (locked):** #457 completes only when all four initial commands — -messages, todolists, check-in questions, schedule entries — are wired. If the -messages SDK input lands first, shipping a messages-only PR is fine, but it keeps -`Refs #457` and leaves #457 **open**; the switch to `Fixes #457` happens only once -all four are wired. This fixes a stable completion boundary rather than deciding -it under merge pressure. Docs/uploads remain a separate follow-up (#556) and are -not part of the #457 boundary. - -## CLI wiring that follows (after SDK bump) - -Once the SDK exposes the field and the CLI bumps to it (`make bump-sdk`), each -in-scope create command gains an optional `--visible-to-clients` bool. Set the -pointer **only when the flag was provided**, so the default stays -inherit-from-parent: - -```go -if cmd.Flags().Changed("visible-to-clients") { - req.VisibleToClients = &visibleToClients -} -``` - -(The gate-on-`Changed` pattern matches `--subscribe` in -`internal/commands/messages.go`; the `*bool` tri-state matches `AllDay`.) A -single atomic create call — no follow-up, no partial-success path, no 403 -handling, since the unsupported types don't get the flag. Then: tests asserting -the create body carries `visible_to_clients` for nil/true/false, regenerate -`.surface`, update `skills/basecamp/SKILL.md`, `bin/ci` green. Delete this file in -that commit and flip this PR to ready (`Refs #457` → `Fixes #457`). diff --git a/internal/commands/checkins.go b/internal/commands/checkins.go index f46f2a349..70436c553 100644 --- a/internal/commands/checkins.go +++ b/internal/commands/checkins.go @@ -268,6 +268,7 @@ func newCheckinsQuestionCreateCmd(project *string) *cobra.Command { var frequency string var timeOfDay string var days string + var visibleToClients bool cmd := &cobra.Command{ Use: "create ", @@ -366,6 +367,12 @@ Days format: comma-separated (0=Sun, 1=Mon, 2=Tue, 3=Wed, 4=Thu, 5=Fri, 6=Sat)`, }, } + // Set client visibility only when the flag was provided; omitting it + // leaves the server's default (team-only for a top-level question). + if cmd.Flags().Changed("visible-to-clients") { + req.VisibleToClients = &visibleToClients + } + question, err := app.Account().Checkins().CreateQuestion(cmd.Context(), qID, req) if err != nil { return convertSDKError(err) @@ -393,6 +400,7 @@ Days format: comma-separated (0=Sun, 1=Mon, 2=Tue, 3=Wed, 4=Thu, 5=Fri, 6=Sat)`, cmd.Flags().StringVarP(&frequency, "frequency", "f", "", "Schedule frequency (default: every_day)") cmd.Flags().StringVar(&timeOfDay, "time", "", "Time to ask (default: 5:00pm)") cmd.Flags().StringVarP(&days, "days", "d", "", "Days to ask, comma-separated (default: 1,2,3,4,5)") + cmd.Flags().BoolVar(&visibleToClients, "visible-to-clients", false, "Make the question visible to clients on the project (default: team-only)") return cmd } diff --git a/internal/commands/checkins_test.go b/internal/commands/checkins_test.go index 844822e34..fadc5c3e4 100644 --- a/internal/commands/checkins_test.go +++ b/internal/commands/checkins_test.go @@ -146,6 +146,95 @@ func (m *mockCheckinsAnswerCreateTransport) RoundTrip(req *http.Request) (*http. } } +// mockCheckinsQuestionCreateTransport resolves the questionnaire via the project +// dock and captures the POST body sent to create a question. +type mockCheckinsQuestionCreateTransport struct { + recordedBody map[string]any +} + +func (m *mockCheckinsQuestionCreateTransport) RoundTrip(req *http.Request) (*http.Response, error) { + header := make(http.Header) + header.Set("Content-Type", "application/json") + + switch { + case req.Method == "GET" && strings.Contains(req.URL.Path, "/projects.json"): + return &http.Response{ + StatusCode: 200, + Body: io.NopCloser(strings.NewReader(`[{"id":123,"name":"Test Project"}]`)), + Header: header, + }, nil + case req.Method == "GET" && strings.Contains(req.URL.Path, "/projects/"): + return &http.Response{ + StatusCode: 200, + Body: io.NopCloser(strings.NewReader(`{"id":123,"dock":[{"name":"questionnaire","id":555,"enabled":true}]}`)), + Header: header, + }, nil + case req.Method == "POST" && strings.Contains(req.URL.Path, "/questions.json"): + if req.Body != nil { + defer req.Body.Close() + } + body, err := io.ReadAll(req.Body) + if err != nil { + return nil, err + } + if err := json.Unmarshal(body, &m.recordedBody); err != nil { + return nil, err + } + return &http.Response{ + StatusCode: 201, + Body: io.NopCloser(strings.NewReader(`{"id":789,"title":"How are you?","type":"Question"}`)), + Header: header, + }, nil + default: + return &http.Response{ + StatusCode: 404, + Body: io.NopCloser(strings.NewReader(`{"error":"Not Found"}`)), + Header: header, + }, nil + } +} + +func runCheckinsQuestionCreate(t *testing.T, args ...string) *mockCheckinsQuestionCreateTransport { + t.Helper() + transport := &mockCheckinsQuestionCreateTransport{} + app, _ := newTestAppWithTransport(t, transport) + app.Config.ProjectID = "123" + + project := "" + cmd := newCheckinsQuestionCreateCmd(&project) + + err := executeCommand(cmd, app, args...) + require.NoError(t, err) + require.NotNil(t, transport.recordedBody, "expected request body to be captured") + return transport +} + +func TestCheckinsQuestionCreateHasVisibleToClientsFlag(t *testing.T) { + project := "" + cmd := newCheckinsQuestionCreateCmd(&project) + + flag := cmd.Flags().Lookup("visible-to-clients") + require.NotNil(t, flag, "expected --visible-to-clients flag on check-in question create") +} + +func TestCheckinsQuestionCreateDefaultOmitsVisibleToClients(t *testing.T) { + transport := runCheckinsQuestionCreate(t, "How are you?") + _, ok := transport.recordedBody["visible_to_clients"] + assert.False(t, ok, "expected visible_to_clients to be omitted when flag is not set") +} + +func TestCheckinsQuestionCreateVisibleToClientsTrue(t *testing.T) { + transport := runCheckinsQuestionCreate(t, "How are you?", "--visible-to-clients") + assert.Equal(t, true, transport.recordedBody["visible_to_clients"]) +} + +func TestCheckinsQuestionCreateVisibleToClientsFalse(t *testing.T) { + transport := runCheckinsQuestionCreate(t, "How are you?", "--visible-to-clients=false") + val, ok := transport.recordedBody["visible_to_clients"] + require.True(t, ok, "expected visible_to_clients present for explicit --visible-to-clients=false") + assert.Equal(t, false, val) +} + func TestCheckinsAnswerCreateDefaultsDateToToday(t *testing.T) { originalNow := checkinsNow checkinsNow = func() time.Time { diff --git a/internal/commands/messages.go b/internal/commands/messages.go index e782db7c6..6c24424c6 100644 --- a/internal/commands/messages.go +++ b/internal/commands/messages.go @@ -263,6 +263,7 @@ func newMessagesCreateCmd(project *string, messageBoard *string) *cobra.Command var subscribe string var noSubscribe bool var attachFiles []string + var visibleToClients bool cmd := &cobra.Command{ Use: "create <title> [body]", @@ -384,6 +385,12 @@ func newMessagesCreateCmd(project *string, messageBoard *string) *cobra.Command req.Status = "active" } + // Set client visibility only when the flag was provided; omitting it + // leaves the server's default (team-only for a top-level message). + if cmd.Flags().Changed("visible-to-clients") { + req.VisibleToClients = &visibleToClients + } + message, err := app.Account().Messages().Create(cmd.Context(), boardID, req) if err != nil { return convertSDKError(err) @@ -417,6 +424,7 @@ func newMessagesCreateCmd(project *string, messageBoard *string) *cobra.Command cmd.Flags().StringVar(&subscribe, "subscribe", "", "Subscribe specific people (comma-separated names, emails, IDs, or \"me\")") cmd.Flags().BoolVar(&noSubscribe, "no-subscribe", false, "Don't subscribe anyone else (silent, no notifications)") cmd.Flags().StringArrayVar(&attachFiles, "attach", nil, "Attach file (repeatable)") + cmd.Flags().BoolVar(&visibleToClients, "visible-to-clients", false, "Make the message visible to clients on the project (default: team-only)") return cmd } diff --git a/internal/commands/messages_test.go b/internal/commands/messages_test.go index ec0ba65f2..3d3d7fb5b 100644 --- a/internal/commands/messages_test.go +++ b/internal/commands/messages_test.go @@ -301,6 +301,72 @@ func TestMessagesCreateSubscribeEmptyIsError(t *testing.T) { assert.Contains(t, e.Message, "at least one person") } +// TestMessagesCreateHasVisibleToClientsFlag tests that messages create has the --visible-to-clients flag. +func TestMessagesCreateHasVisibleToClientsFlag(t *testing.T) { + cmd := NewMessagesCmd() + createCmd, _, err := cmd.Find([]string{"create"}) + require.NoError(t, err) + + flag := createCmd.Flags().Lookup("visible-to-clients") + require.NotNil(t, flag, "expected --visible-to-clients flag on messages create") +} + +// TestMessagesCreateDefaultOmitsVisibleToClients verifies that without the flag, +// visible_to_clients is omitted so the server applies its own default. +func TestMessagesCreateDefaultOmitsVisibleToClients(t *testing.T) { + transport := &mockMessageCreateTransport{} + app, _ := setupMessagesMockApp(t, transport) + + cmd := NewMessagesCmd() + + err := executeMessagesCommand(cmd, app, "create", "Normal post") + require.NoError(t, err) + require.NotEmpty(t, transport.capturedBody) + + var body map[string]any + require.NoError(t, json.Unmarshal(transport.capturedBody, &body)) + + _, ok := body["visible_to_clients"] + assert.False(t, ok, "expected visible_to_clients to be omitted when flag is not set") +} + +// TestMessagesCreateVisibleToClientsTrue verifies --visible-to-clients sends true. +func TestMessagesCreateVisibleToClientsTrue(t *testing.T) { + transport := &mockMessageCreateTransport{} + app, _ := setupMessagesMockApp(t, transport) + + cmd := NewMessagesCmd() + + err := executeMessagesCommand(cmd, app, "create", "Client post", "--visible-to-clients") + require.NoError(t, err) + require.NotEmpty(t, transport.capturedBody) + + var body map[string]any + require.NoError(t, json.Unmarshal(transport.capturedBody, &body)) + + assert.Equal(t, true, body["visible_to_clients"]) +} + +// TestMessagesCreateVisibleToClientsFalse verifies --visible-to-clients=false +// sends an explicit false rather than dropping the field. +func TestMessagesCreateVisibleToClientsFalse(t *testing.T) { + transport := &mockMessageCreateTransport{} + app, _ := setupMessagesMockApp(t, transport) + + cmd := NewMessagesCmd() + + err := executeMessagesCommand(cmd, app, "create", "Team post", "--visible-to-clients=false") + require.NoError(t, err) + require.NotEmpty(t, transport.capturedBody) + + var body map[string]any + require.NoError(t, json.Unmarshal(transport.capturedBody, &body)) + + val, ok := body["visible_to_clients"] + require.True(t, ok, "expected visible_to_clients present for explicit --visible-to-clients=false") + assert.Equal(t, false, val) +} + // mockMessageUpdateTransport handles PUT requests and captures the body. type mockMessageUpdateTransport struct { capturedBody []byte diff --git a/internal/commands/schedule.go b/internal/commands/schedule.go index 104f112dd..9f10d2e21 100644 --- a/internal/commands/schedule.go +++ b/internal/commands/schedule.go @@ -409,6 +409,7 @@ func newScheduleCreateCmd(project, scheduleID *string) *cobra.Command { var subscribe string var noSubscribe bool var attachFiles []string + var visibleToClients bool cmd := &cobra.Command{ Use: "create <summary>", @@ -457,6 +458,7 @@ func newScheduleCreateCmd(project, scheduleID *string) *cobra.Command { cmd.Flags().StringVar(&subscribe, "subscribe", "", "Subscribe specific people (comma-separated names, emails, IDs, or \"me\")") cmd.Flags().BoolVar(&noSubscribe, "no-subscribe", false, "Don't subscribe anyone else (silent, no notifications)") cmd.Flags().StringArrayVar(&attachFiles, "attach", nil, "Attach file (repeatable)") + cmd.Flags().BoolVar(&visibleToClients, "visible-to-clients", false, "Make the schedule entry visible to clients on the project (default: team-only)") return cmd } @@ -534,6 +536,13 @@ func runScheduleCreate(cmd *cobra.Command, app *appctx.App, project, scheduleID, Subscriptions: subs, } + // Set client visibility only when the flag was provided; omitting it leaves + // the server's default (team-only for a top-level schedule entry). + if cmd.Flags().Changed("visible-to-clients") { + visibleToClients, _ := cmd.Flags().GetBool("visible-to-clients") + req.VisibleToClients = &visibleToClients + } + if participants != "" { var ids []int64 for idStr := range strings.SplitSeq(participants, ",") { diff --git a/internal/commands/schedule_test.go b/internal/commands/schedule_test.go index 1fdbc5e94..5477bb238 100644 --- a/internal/commands/schedule_test.go +++ b/internal/commands/schedule_test.go @@ -153,6 +153,71 @@ func TestScheduleUpdateDescriptionIsHTML(t *testing.T) { assert.Contains(t, desc, "<strong>details</strong>") } +func TestScheduleCreateHasVisibleToClientsFlag(t *testing.T) { + cmd := NewScheduleCmd() + createCmd, _, err := cmd.Find([]string{"create"}) + require.NoError(t, err) + + flag := createCmd.Flags().Lookup("visible-to-clients") + require.NotNil(t, flag, "expected --visible-to-clients flag on schedule create") +} + +func TestScheduleCreateDefaultOmitsVisibleToClients(t *testing.T) { + transport := &mockScheduleCreateTransport{} + app, _ := setupMessagesMockApp(t, transport) + + cmd := NewScheduleCmd() + err := executeMessagesCommand(cmd, app, "create", "Event", + "--starts-at", "2026-03-04T09:00:00Z", + "--ends-at", "2026-03-04T09:30:00Z") + require.NoError(t, err) + require.NotEmpty(t, transport.capturedBody) + + var body map[string]any + require.NoError(t, json.Unmarshal(transport.capturedBody, &body)) + + _, ok := body["visible_to_clients"] + assert.False(t, ok, "expected visible_to_clients to be omitted when flag is not set") +} + +func TestScheduleCreateVisibleToClientsTrue(t *testing.T) { + transport := &mockScheduleCreateTransport{} + app, _ := setupMessagesMockApp(t, transport) + + cmd := NewScheduleCmd() + err := executeMessagesCommand(cmd, app, "create", "Event", + "--starts-at", "2026-03-04T09:00:00Z", + "--ends-at", "2026-03-04T09:30:00Z", + "--visible-to-clients") + require.NoError(t, err) + require.NotEmpty(t, transport.capturedBody) + + var body map[string]any + require.NoError(t, json.Unmarshal(transport.capturedBody, &body)) + + assert.Equal(t, true, body["visible_to_clients"]) +} + +func TestScheduleCreateVisibleToClientsFalse(t *testing.T) { + transport := &mockScheduleCreateTransport{} + app, _ := setupMessagesMockApp(t, transport) + + cmd := NewScheduleCmd() + err := executeMessagesCommand(cmd, app, "create", "Event", + "--starts-at", "2026-03-04T09:00:00Z", + "--ends-at", "2026-03-04T09:30:00Z", + "--visible-to-clients=false") + require.NoError(t, err) + require.NotEmpty(t, transport.capturedBody) + + var body map[string]any + require.NoError(t, json.Unmarshal(transport.capturedBody, &body)) + + val, ok := body["visible_to_clients"] + require.True(t, ok, "expected visible_to_clients present for explicit --visible-to-clients=false") + assert.Equal(t, false, val) +} + func TestScheduleCreateLocalImageErrors(t *testing.T) { transport := &mockScheduleCreateTransport{} app, _ := setupMessagesMockApp(t, transport) diff --git a/internal/commands/todolists.go b/internal/commands/todolists.go index 04d16e3e7..10651504b 100644 --- a/internal/commands/todolists.go +++ b/internal/commands/todolists.go @@ -265,6 +265,7 @@ You can pass either a todolist ID or a Basecamp URL: func newTodolistsCreateCmd(project, todosetID *string) *cobra.Command { var description string + var visibleToClients bool cmd := &cobra.Command{ Use: "create <name>", @@ -325,6 +326,12 @@ func newTodolistsCreateCmd(project, todosetID *string) *cobra.Command { Description: description, } + // Set client visibility only when the flag was provided; omitting it + // leaves the server's default (team-only for a top-level todolist). + if cmd.Flags().Changed("visible-to-clients") { + req.VisibleToClients = &visibleToClients + } + // Create todolist via SDK todolist, err := app.Account().Todolists().Create(cmd.Context(), tsID, req) if err != nil { @@ -354,6 +361,7 @@ func newTodolistsCreateCmd(project, todosetID *string) *cobra.Command { cmd.Flags().StringVarP(todosetID, "todoset", "t", "", "Todoset ID (for projects with multiple todosets)") cmd.Flags().StringVarP(&description, "description", "d", "", "Todolist description") + cmd.Flags().BoolVar(&visibleToClients, "visible-to-clients", false, "Make the todolist visible to clients on the project (default: team-only)") return cmd } diff --git a/internal/commands/todolists_test.go b/internal/commands/todolists_test.go new file mode 100644 index 000000000..95b9e6db1 --- /dev/null +++ b/internal/commands/todolists_test.go @@ -0,0 +1,112 @@ +package commands + +import ( + "encoding/json" + "errors" + "io" + "net/http" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// mockTodolistCreateTransport resolves the todoset via the project dock and +// captures the POST body sent to create a todolist. +type mockTodolistCreateTransport struct { + capturedBody []byte +} + +func (t *mockTodolistCreateTransport) RoundTrip(req *http.Request) (*http.Response, error) { + header := make(http.Header) + header.Set("Content-Type", "application/json") + + if req.Method == "GET" { + var body string + if strings.Contains(req.URL.Path, "/projects.json") { + body = `[{"id": 123, "name": "Test Project"}]` + } else if strings.Contains(req.URL.Path, "/projects/") { + body = `{"id": 123, "dock": [{"name": "todoset", "id": 777, "enabled": true}]}` + } else { + body = `{}` + } + return &http.Response{ + StatusCode: 200, + Body: io.NopCloser(strings.NewReader(body)), + Header: header, + }, nil + } + + if req.Method == "POST" && strings.Contains(req.URL.Path, "/todolists.json") { + if req.Body != nil { + body, _ := io.ReadAll(req.Body) + t.capturedBody = body + req.Body.Close() + } + return &http.Response{ + StatusCode: 201, + Body: io.NopCloser(strings.NewReader(`{"id": 999, "name": "My list"}`)), + Header: header, + }, nil + } + + return nil, errors.New("unexpected request") +} + +func TestTodolistsCreateHasVisibleToClientsFlag(t *testing.T) { + cmd := NewTodolistsCmd() + createCmd, _, err := cmd.Find([]string{"create"}) + require.NoError(t, err) + + flag := createCmd.Flags().Lookup("visible-to-clients") + require.NotNil(t, flag, "expected --visible-to-clients flag on todolists create") +} + +func TestTodolistsCreateDefaultOmitsVisibleToClients(t *testing.T) { + transport := &mockTodolistCreateTransport{} + app, _ := setupMessagesMockApp(t, transport) + + cmd := NewTodolistsCmd() + err := executeMessagesCommand(cmd, app, "create", "My list") + require.NoError(t, err) + require.NotEmpty(t, transport.capturedBody) + + var body map[string]any + require.NoError(t, json.Unmarshal(transport.capturedBody, &body)) + + _, ok := body["visible_to_clients"] + assert.False(t, ok, "expected visible_to_clients to be omitted when flag is not set") +} + +func TestTodolistsCreateVisibleToClientsTrue(t *testing.T) { + transport := &mockTodolistCreateTransport{} + app, _ := setupMessagesMockApp(t, transport) + + cmd := NewTodolistsCmd() + err := executeMessagesCommand(cmd, app, "create", "My list", "--visible-to-clients") + require.NoError(t, err) + require.NotEmpty(t, transport.capturedBody) + + var body map[string]any + require.NoError(t, json.Unmarshal(transport.capturedBody, &body)) + + assert.Equal(t, true, body["visible_to_clients"]) +} + +func TestTodolistsCreateVisibleToClientsFalse(t *testing.T) { + transport := &mockTodolistCreateTransport{} + app, _ := setupMessagesMockApp(t, transport) + + cmd := NewTodolistsCmd() + err := executeMessagesCommand(cmd, app, "create", "My list", "--visible-to-clients=false") + require.NoError(t, err) + require.NotEmpty(t, transport.capturedBody) + + var body map[string]any + require.NoError(t, json.Unmarshal(transport.capturedBody, &body)) + + val, ok := body["visible_to_clients"] + require.True(t, ok, "expected visible_to_clients present for explicit --visible-to-clients=false") + assert.Equal(t, false, val) +} diff --git a/skills/basecamp/SKILL.md b/skills/basecamp/SKILL.md index 36f3e696d..35dc4b5fb 100644 --- a/skills/basecamp/SKILL.md +++ b/skills/basecamp/SKILL.md @@ -550,6 +550,7 @@ basecamp todolists list --in <project> --json # List todolists basecamp todolists show <id> --in <project> # Show details basecamp todolists create "Name" --in <project> --json # Create basecamp todolists create "Name" --description "Desc" --in <project> +basecamp todolists create "Name" --visible-to-clients --in <project> # Visible to clients basecamp todolists update <id> --name "New" --in <project> # Update ``` @@ -613,13 +614,20 @@ basecamp messages unpin <id> # Unpin **Archived/trashed messages:** `messages list` only returns active messages. For archived or trashed messages, use `basecamp recordings messages --status archived --in <project>` or `--status trashed`. -**Flags:** `--draft` (create as draft), `--no-subscribe` (silent, no notifications), `--subscribe "people"` (comma-separated names, emails, IDs, or "me"; mutually exclusive with `--no-subscribe`), `--message-board <id>` (if multiple boards) +**Flags:** `--draft` (create as draft), `--no-subscribe` (silent, no notifications), `--subscribe "people"` (comma-separated names, emails, IDs, or "me"; mutually exclusive with `--no-subscribe`), `--message-board <id>` (if multiple boards), `--visible-to-clients` (make visible to clients on the project; omit for team-only) ```bash basecamp messages create "Bot update" "Done" --no-subscribe --in <project> basecamp messages create "FYI" "Note" --subscribe "Alice,bob@x.com" --in <project> +basecamp messages create "For the client" "..." --visible-to-clients --in <project> ``` +**Client visibility at create time:** `messages create`, `todolists create`, +`schedule create`, and `checkins question create` accept `--visible-to-clients` +to post a client-visible recording in one call. Omitting the flag leaves the +server default (team-only for these top-level posts). To change visibility on an +already-created recording, use `recordings visibility <id> --visible`. + ### Comments ```bash @@ -669,7 +677,7 @@ basecamp schedule update <id> --summary "New title" --starts-at "..." basecamp schedule settings --include-due --in <project> # Include todos/cards due dates ``` -**Flags:** `--all-day`, `--notify`, `--participants <ids>`, `--no-subscribe`, `--subscribe "people"` (mutually exclusive), `--status` (active/archived/trashed) +**Flags:** `--all-day`, `--notify`, `--participants <ids>`, `--no-subscribe`, `--subscribe "people"` (mutually exclusive), `--status` (active/archived/trashed), `--visible-to-clients` (make visible to clients; omit for team-only) ### Check-ins @@ -689,6 +697,8 @@ basecamp checkins answer update <id> "Updated" --in <project> **Schedule options:** `--frequency` (every_day, every_week, every_other_week, every_month, on_certain_days), `--days 1,2,3,4,5` (0=Sun), `--time "5:00pm"` +**Client visibility:** `checkins question create` accepts `--visible-to-clients` to make the question visible to clients (omit for team-only). + ### Timeline ```bash From 4774ff29276932ade9bb58ff58e15995754dc1fe Mon Sep 17 00:00:00 2001 From: Jeremy Daer <jeremy@37signals.com> Date: Fri, 24 Jul 2026 12:32:46 -0700 Subject: [PATCH 7/8] Correct client-visibility help and search metadata text 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. --- internal/commands/checkins.go | 8 ++-- internal/commands/messages.go | 8 ++-- internal/commands/schedule.go | 8 ++-- internal/commands/search.go | 4 +- internal/commands/search_test.go | 71 ++++++++++++++++++++++++++++++++ internal/commands/todolists.go | 8 ++-- skills/basecamp/SKILL.md | 16 ++++--- 7 files changed, 103 insertions(+), 20 deletions(-) diff --git a/internal/commands/checkins.go b/internal/commands/checkins.go index 70436c553..37f28ae88 100644 --- a/internal/commands/checkins.go +++ b/internal/commands/checkins.go @@ -367,8 +367,10 @@ Days format: comma-separated (0=Sun, 1=Mon, 2=Tue, 3=Wed, 4=Thu, 5=Fri, 6=Sat)`, }, } - // Set client visibility only when the flag was provided; omitting it - // leaves the server's default (team-only for a top-level question). + // Set client visibility only when the flag was provided. Omitting it + // uses the server's default: team-only when posting as a team member, + // but a client-authenticated caller always creates client-visible + // records (an explicit false is overridden server-side). if cmd.Flags().Changed("visible-to-clients") { req.VisibleToClients = &visibleToClients } @@ -400,7 +402,7 @@ Days format: comma-separated (0=Sun, 1=Mon, 2=Tue, 3=Wed, 4=Thu, 5=Fri, 6=Sat)`, cmd.Flags().StringVarP(&frequency, "frequency", "f", "", "Schedule frequency (default: every_day)") cmd.Flags().StringVar(&timeOfDay, "time", "", "Time to ask (default: 5:00pm)") cmd.Flags().StringVarP(&days, "days", "d", "", "Days to ask, comma-separated (default: 1,2,3,4,5)") - cmd.Flags().BoolVar(&visibleToClients, "visible-to-clients", false, "Make the question visible to clients on the project (default: team-only)") + cmd.Flags().BoolVar(&visibleToClients, "visible-to-clients", false, "Make the question visible to clients on the project (omit for the server default; client-authenticated callers always post client-visible)") return cmd } diff --git a/internal/commands/messages.go b/internal/commands/messages.go index 6c24424c6..aa9b11474 100644 --- a/internal/commands/messages.go +++ b/internal/commands/messages.go @@ -385,8 +385,10 @@ func newMessagesCreateCmd(project *string, messageBoard *string) *cobra.Command req.Status = "active" } - // Set client visibility only when the flag was provided; omitting it - // leaves the server's default (team-only for a top-level message). + // Set client visibility only when the flag was provided. Omitting it + // uses the server's default: team-only when posting as a team member, + // but a client-authenticated caller always creates client-visible + // records (an explicit false is overridden server-side). if cmd.Flags().Changed("visible-to-clients") { req.VisibleToClients = &visibleToClients } @@ -424,7 +426,7 @@ func newMessagesCreateCmd(project *string, messageBoard *string) *cobra.Command cmd.Flags().StringVar(&subscribe, "subscribe", "", "Subscribe specific people (comma-separated names, emails, IDs, or \"me\")") cmd.Flags().BoolVar(&noSubscribe, "no-subscribe", false, "Don't subscribe anyone else (silent, no notifications)") cmd.Flags().StringArrayVar(&attachFiles, "attach", nil, "Attach file (repeatable)") - cmd.Flags().BoolVar(&visibleToClients, "visible-to-clients", false, "Make the message visible to clients on the project (default: team-only)") + cmd.Flags().BoolVar(&visibleToClients, "visible-to-clients", false, "Make the message visible to clients on the project (omit for the server default; client-authenticated callers always post client-visible)") return cmd } diff --git a/internal/commands/schedule.go b/internal/commands/schedule.go index 9f10d2e21..a0c5a48e1 100644 --- a/internal/commands/schedule.go +++ b/internal/commands/schedule.go @@ -458,7 +458,7 @@ func newScheduleCreateCmd(project, scheduleID *string) *cobra.Command { cmd.Flags().StringVar(&subscribe, "subscribe", "", "Subscribe specific people (comma-separated names, emails, IDs, or \"me\")") cmd.Flags().BoolVar(&noSubscribe, "no-subscribe", false, "Don't subscribe anyone else (silent, no notifications)") cmd.Flags().StringArrayVar(&attachFiles, "attach", nil, "Attach file (repeatable)") - cmd.Flags().BoolVar(&visibleToClients, "visible-to-clients", false, "Make the schedule entry visible to clients on the project (default: team-only)") + cmd.Flags().BoolVar(&visibleToClients, "visible-to-clients", false, "Make the schedule entry visible to clients on the project (omit for the server default; client-authenticated callers always post client-visible)") return cmd } @@ -536,8 +536,10 @@ func runScheduleCreate(cmd *cobra.Command, app *appctx.App, project, scheduleID, Subscriptions: subs, } - // Set client visibility only when the flag was provided; omitting it leaves - // the server's default (team-only for a top-level schedule entry). + // Set client visibility only when the flag was provided. Omitting it uses the + // server's default: team-only when posting as a team member, but a + // client-authenticated caller always creates client-visible records (an + // explicit false is overridden server-side). if cmd.Flags().Changed("visible-to-clients") { visibleToClients, _ := cmd.Flags().GetBool("visible-to-clients") req.VisibleToClients = &visibleToClients diff --git a/internal/commands/search.go b/internal/commands/search.go index ef9a67810..7d7d7729a 100644 --- a/internal/commands/search.go +++ b/internal/commands/search.go @@ -111,8 +111,8 @@ func newSearchMetadataCmd() *cobra.Command { return &cobra.Command{ Use: "metadata", Aliases: []string{"types"}, - Short: "Show available search scopes", - Long: "Display available projects for search scope filtering.", + Short: "Show available search filters", + Long: "Display the available recording-type and file-type filters for scoping a search.", RunE: func(cmd *cobra.Command, args []string) error { app := appctx.FromContext(cmd.Context()) return runSearchMetadata(cmd, app) diff --git a/internal/commands/search_test.go b/internal/commands/search_test.go index 85c4b89d2..197517fe8 100644 --- a/internal/commands/search_test.go +++ b/internal/commands/search_test.go @@ -109,6 +109,77 @@ func executeSearchCommand(cmd *cobra.Command, app *appctx.App, args ...string) e return cmd.Execute() } +// searchMetadataTransport serves the search metadata endpoint. A nil body +// simulates an empty (no-filters) response. +type searchMetadataTransport struct { + body string +} + +func (s searchMetadataTransport) RoundTrip(req *http.Request) (*http.Response, error) { + header := make(http.Header) + header.Set("Content-Type", "application/json") + + if !strings.Contains(req.URL.Path, "/searches/metadata") { + return nil, errors.New("unexpected request: " + req.URL.Path) + } + body := s.body + if body == "" { + body = `{"recording_search_types":[],"file_search_types":[]}` + } + return &http.Response{ + StatusCode: 200, + Body: io.NopCloser(strings.NewReader(body)), + Header: header, + Request: req, + }, nil +} + +const searchMetadataBody = `{ + "recording_search_types": [ + {"key": null, "value": "Everything"}, + {"key": "Message", "value": "Messages"}, + {"key": "Todo", "value": "To-dos"} + ], + "file_search_types": [ + {"key": null, "value": "All files"}, + {"key": "image", "value": "Images"} + ], + "default_creator_label": "Anyone", + "default_bucket_label": "All projects", + "default_circle_label": "All pings", + "default_file_type_label": "All files", + "default_type_label": "Everything" +}` + +// TestSearchMetadataReturnsFilterTypes verifies the metadata command reports the +// recording/file filter counts from the reshaped SearchMetadata response. +func TestSearchMetadataReturnsFilterTypes(t *testing.T) { + app, buf := setupSearchTestApp(t, searchMetadataTransport{body: searchMetadataBody}) + + cmd := NewSearchCmd() + err := executeSearchCommand(cmd, app, "metadata") + require.NoError(t, err) + + var envelope output.Response + require.NoError(t, json.Unmarshal(buf.Bytes(), &envelope)) + assert.Contains(t, envelope.Summary, "3 recording types") + assert.Contains(t, envelope.Summary, "2 file types") +} + +// TestSearchMetadataEmptyIsUsageError verifies an empty metadata response +// surfaces a usage hint rather than an empty success envelope. +func TestSearchMetadataEmptyIsUsageError(t *testing.T) { + app, _ := setupSearchTestApp(t, searchMetadataTransport{}) + + cmd := NewSearchCmd() + err := executeSearchCommand(cmd, app, "metadata") + require.Error(t, err) + + var e *output.Error + require.True(t, errors.As(err, &e), "expected *output.Error, got %T: %v", err, err) + assert.Equal(t, "Search metadata not available", e.Message) +} + func TestSearchTruncationNoticePresent(t *testing.T) { app, buf := setupSearchTestApp(t, searchTransport{resultCount: 5, totalCount: 20}) diff --git a/internal/commands/todolists.go b/internal/commands/todolists.go index 10651504b..acab87915 100644 --- a/internal/commands/todolists.go +++ b/internal/commands/todolists.go @@ -326,8 +326,10 @@ func newTodolistsCreateCmd(project, todosetID *string) *cobra.Command { Description: description, } - // Set client visibility only when the flag was provided; omitting it - // leaves the server's default (team-only for a top-level todolist). + // Set client visibility only when the flag was provided. Omitting it + // uses the server's default: team-only when posting as a team member, + // but a client-authenticated caller always creates client-visible + // records (an explicit false is overridden server-side). if cmd.Flags().Changed("visible-to-clients") { req.VisibleToClients = &visibleToClients } @@ -361,7 +363,7 @@ func newTodolistsCreateCmd(project, todosetID *string) *cobra.Command { cmd.Flags().StringVarP(todosetID, "todoset", "t", "", "Todoset ID (for projects with multiple todosets)") cmd.Flags().StringVarP(&description, "description", "d", "", "Todolist description") - cmd.Flags().BoolVar(&visibleToClients, "visible-to-clients", false, "Make the todolist visible to clients on the project (default: team-only)") + cmd.Flags().BoolVar(&visibleToClients, "visible-to-clients", false, "Make the todolist visible to clients on the project (omit for the server default; client-authenticated callers always post client-visible)") return cmd } diff --git a/skills/basecamp/SKILL.md b/skills/basecamp/SKILL.md index 35dc4b5fb..8c3741d93 100644 --- a/skills/basecamp/SKILL.md +++ b/skills/basecamp/SKILL.md @@ -614,7 +614,7 @@ basecamp messages unpin <id> # Unpin **Archived/trashed messages:** `messages list` only returns active messages. For archived or trashed messages, use `basecamp recordings messages --status archived --in <project>` or `--status trashed`. -**Flags:** `--draft` (create as draft), `--no-subscribe` (silent, no notifications), `--subscribe "people"` (comma-separated names, emails, IDs, or "me"; mutually exclusive with `--no-subscribe`), `--message-board <id>` (if multiple boards), `--visible-to-clients` (make visible to clients on the project; omit for team-only) +**Flags:** `--draft` (create as draft), `--no-subscribe` (silent, no notifications), `--subscribe "people"` (comma-separated names, emails, IDs, or "me"; mutually exclusive with `--no-subscribe`), `--message-board <id>` (if multiple boards), `--visible-to-clients` (make visible to clients on the project; omit for the server default) ```bash basecamp messages create "Bot update" "Done" --no-subscribe --in <project> @@ -624,9 +624,13 @@ basecamp messages create "For the client" "..." --visible-to-clients --in <proje **Client visibility at create time:** `messages create`, `todolists create`, `schedule create`, and `checkins question create` accept `--visible-to-clients` -to post a client-visible recording in one call. Omitting the flag leaves the -server default (team-only for these top-level posts). To change visibility on an -already-created recording, use `recordings visibility <id> --visible`. +to post a client-visible recording in one call. Omitting the flag uses the +server default, which is context-dependent: **team-only when you post as a team +member**, but a **client-authenticated caller always creates client-visible +records** (an explicit `--visible-to-clients=false` is overridden server-side for +client callers). Passing `--visible-to-clients` posts client-visible in every +case. To change visibility on an already-created recording, use +`recordings visibility <id> --visible`. ### Comments @@ -677,7 +681,7 @@ basecamp schedule update <id> --summary "New title" --starts-at "..." basecamp schedule settings --include-due --in <project> # Include todos/cards due dates ``` -**Flags:** `--all-day`, `--notify`, `--participants <ids>`, `--no-subscribe`, `--subscribe "people"` (mutually exclusive), `--status` (active/archived/trashed), `--visible-to-clients` (make visible to clients; omit for team-only) +**Flags:** `--all-day`, `--notify`, `--participants <ids>`, `--no-subscribe`, `--subscribe "people"` (mutually exclusive), `--status` (active/archived/trashed), `--visible-to-clients` (make visible to clients; omit for the server default) ### Check-ins @@ -697,7 +701,7 @@ basecamp checkins answer update <id> "Updated" --in <project> **Schedule options:** `--frequency` (every_day, every_week, every_other_week, every_month, on_certain_days), `--days 1,2,3,4,5` (0=Sun), `--time "5:00pm"` -**Client visibility:** `checkins question create` accepts `--visible-to-clients` to make the question visible to clients (omit for team-only). +**Client visibility:** `checkins question create` accepts `--visible-to-clients` to make the question visible to clients (omit for the server default; see the note under Messages for the context-dependent rule). ### Timeline From cb1c7e4b95e66dc82bd768bc9f01142c111898b2 Mon Sep 17 00:00:00 2001 From: Jeremy Daer <jeremy@37signals.com> Date: Fri, 24 Jul 2026 12:50:09 -0700 Subject: [PATCH 8/8] Refine schedule visibility wiring and search readability MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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'. --- internal/commands/schedule.go | 5 ++--- internal/commands/search.go | 2 +- internal/commands/search_test.go | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/internal/commands/schedule.go b/internal/commands/schedule.go index a0c5a48e1..df9d8f006 100644 --- a/internal/commands/schedule.go +++ b/internal/commands/schedule.go @@ -439,7 +439,7 @@ func newScheduleCreateCmd(project, scheduleID *string) *cobra.Command { return output.ErrUsage("--ends-at required (ISO 8601 datetime)") } - return runScheduleCreate(cmd, app, *project, *scheduleID, entrySummary, startsAt, endsAt, description, allDay, notify, participants, subscribe, noSubscribe, attachFiles) + return runScheduleCreate(cmd, app, *project, *scheduleID, entrySummary, startsAt, endsAt, description, allDay, notify, visibleToClients, participants, subscribe, noSubscribe, attachFiles) }, } @@ -463,7 +463,7 @@ func newScheduleCreateCmd(project, scheduleID *string) *cobra.Command { return cmd } -func runScheduleCreate(cmd *cobra.Command, app *appctx.App, project, scheduleID, summary, startsAt, endsAt, description string, allDay, notify bool, participants, subscribe string, noSubscribe bool, attachFiles []string) error { +func runScheduleCreate(cmd *cobra.Command, app *appctx.App, project, scheduleID, summary, startsAt, endsAt, description string, allDay, notify, visibleToClients bool, participants, subscribe string, noSubscribe bool, attachFiles []string) error { // Resolve subscription flags early (fail fast on bad input) subs, err := applySubscribeFlags(cmd.Context(), app.Names, subscribe, cmd.Flags().Changed("subscribe"), noSubscribe) if err != nil { @@ -541,7 +541,6 @@ func runScheduleCreate(cmd *cobra.Command, app *appctx.App, project, scheduleID, // client-authenticated caller always creates client-visible records (an // explicit false is overridden server-side). if cmd.Flags().Changed("visible-to-clients") { - visibleToClients, _ := cmd.Flags().GetBool("visible-to-clients") req.VisibleToClients = &visibleToClients } diff --git a/internal/commands/search.go b/internal/commands/search.go index 7d7d7729a..1e62f76c8 100644 --- a/internal/commands/search.go +++ b/internal/commands/search.go @@ -202,7 +202,7 @@ func runSearchMetadata(cmd *cobra.Command, app *appctx.App) error { } // Handle empty response - if metadata == nil || len(metadata.RecordingSearchTypes) == 0 && len(metadata.FileSearchTypes) == 0 { + if metadata == nil || (len(metadata.RecordingSearchTypes) == 0 && len(metadata.FileSearchTypes) == 0) { return output.ErrUsageHint( "Search metadata not available", "No search filters available", diff --git a/internal/commands/search_test.go b/internal/commands/search_test.go index 197517fe8..ce76ef5fe 100644 --- a/internal/commands/search_test.go +++ b/internal/commands/search_test.go @@ -109,7 +109,7 @@ func executeSearchCommand(cmd *cobra.Command, app *appctx.App, args ...string) e return cmd.Execute() } -// searchMetadataTransport serves the search metadata endpoint. A nil body +// searchMetadataTransport serves the search metadata endpoint. An empty body // simulates an empty (no-filters) response. type searchMetadataTransport struct { body string