diff --git a/.surface b/.surface index d4ca419c..e444710f 100644 --- a/.surface +++ b/.surface @@ -290,6 +290,9 @@ ARG basecamp msgs update 00 ARG basecamp notes set 00 [content] ARG basecamp notifications read 00 ... ARG basecamp people add 00 ... +ARG basecamp people clients add 00 ... +ARG basecamp people clients invite 00 ... +ARG basecamp people clients remove 00 ... ARG basecamp people ooo 00 [me] ARG basecamp people out-of-office 00 [me] ARG basecamp people remove 00 ... @@ -925,6 +928,13 @@ CMD basecamp notifications list CMD basecamp notifications read CMD basecamp people CMD basecamp people add +CMD basecamp people clients +CMD basecamp people clients add +CMD basecamp people clients disable +CMD basecamp people clients enable +CMD basecamp people clients invite +CMD basecamp people clients list +CMD basecamp people clients remove CMD basecamp people list CMD basecamp people ooo CMD basecamp people out-of-office @@ -11742,6 +11752,155 @@ FLAG basecamp people add --stats type=bool FLAG basecamp people add --styled type=bool FLAG basecamp people add --todolist type=string FLAG basecamp people add --verbose type=count +FLAG basecamp people clients --account type=string +FLAG basecamp people clients --agent type=bool +FLAG basecamp people clients --cache-dir type=string +FLAG basecamp people clients --count type=bool +FLAG basecamp people clients --help type=bool +FLAG basecamp people clients --hints type=bool +FLAG basecamp people clients --ids-only type=bool +FLAG basecamp people clients --in type=string +FLAG basecamp people clients --jq type=string +FLAG basecamp people clients --json type=bool +FLAG basecamp people clients --markdown type=bool +FLAG basecamp people clients --md type=bool +FLAG basecamp people clients --no-hints type=bool +FLAG basecamp people clients --no-stats type=bool +FLAG basecamp people clients --profile type=string +FLAG basecamp people clients --project type=string +FLAG basecamp people clients --quiet type=bool +FLAG basecamp people clients --stats type=bool +FLAG basecamp people clients --styled type=bool +FLAG basecamp people clients --todolist type=string +FLAG basecamp people clients --verbose type=count +FLAG basecamp people clients add --account type=string +FLAG basecamp people clients add --agent type=bool +FLAG basecamp people clients add --cache-dir type=string +FLAG basecamp people clients add --count type=bool +FLAG basecamp people clients add --help type=bool +FLAG basecamp people clients add --hints type=bool +FLAG basecamp people clients add --ids-only type=bool +FLAG basecamp people clients add --in type=string +FLAG basecamp people clients add --jq type=string +FLAG basecamp people clients add --json type=bool +FLAG basecamp people clients add --markdown type=bool +FLAG basecamp people clients add --md type=bool +FLAG basecamp people clients add --no-hints type=bool +FLAG basecamp people clients add --no-stats type=bool +FLAG basecamp people clients add --profile type=string +FLAG basecamp people clients add --project type=string +FLAG basecamp people clients add --quiet type=bool +FLAG basecamp people clients add --stats type=bool +FLAG basecamp people clients add --styled type=bool +FLAG basecamp people clients add --todolist type=string +FLAG basecamp people clients add --verbose type=count +FLAG basecamp people clients disable --account type=string +FLAG basecamp people clients disable --agent type=bool +FLAG basecamp people clients disable --cache-dir type=string +FLAG basecamp people clients disable --count type=bool +FLAG basecamp people clients disable --help type=bool +FLAG basecamp people clients disable --hints type=bool +FLAG basecamp people clients disable --ids-only type=bool +FLAG basecamp people clients disable --in type=string +FLAG basecamp people clients disable --jq type=string +FLAG basecamp people clients disable --json type=bool +FLAG basecamp people clients disable --markdown type=bool +FLAG basecamp people clients disable --md type=bool +FLAG basecamp people clients disable --no-hints type=bool +FLAG basecamp people clients disable --no-stats type=bool +FLAG basecamp people clients disable --profile type=string +FLAG basecamp people clients disable --project type=string +FLAG basecamp people clients disable --quiet type=bool +FLAG basecamp people clients disable --stats type=bool +FLAG basecamp people clients disable --styled type=bool +FLAG basecamp people clients disable --todolist type=string +FLAG basecamp people clients disable --verbose type=count +FLAG basecamp people clients enable --account type=string +FLAG basecamp people clients enable --agent type=bool +FLAG basecamp people clients enable --cache-dir type=string +FLAG basecamp people clients enable --count type=bool +FLAG basecamp people clients enable --help type=bool +FLAG basecamp people clients enable --hints type=bool +FLAG basecamp people clients enable --ids-only type=bool +FLAG basecamp people clients enable --in type=string +FLAG basecamp people clients enable --jq type=string +FLAG basecamp people clients enable --json type=bool +FLAG basecamp people clients enable --markdown type=bool +FLAG basecamp people clients enable --md type=bool +FLAG basecamp people clients enable --no-hints type=bool +FLAG basecamp people clients enable --no-stats type=bool +FLAG basecamp people clients enable --profile type=string +FLAG basecamp people clients enable --project type=string +FLAG basecamp people clients enable --quiet type=bool +FLAG basecamp people clients enable --stats type=bool +FLAG basecamp people clients enable --styled type=bool +FLAG basecamp people clients enable --todolist type=string +FLAG basecamp people clients enable --verbose type=count +FLAG basecamp people clients invite --account type=string +FLAG basecamp people clients invite --agent type=bool +FLAG basecamp people clients invite --cache-dir type=string +FLAG basecamp people clients invite --company type=string +FLAG basecamp people clients invite --count type=bool +FLAG basecamp people clients invite --help type=bool +FLAG basecamp people clients invite --hints type=bool +FLAG basecamp people clients invite --ids-only type=bool +FLAG basecamp people clients invite --in type=string +FLAG basecamp people clients invite --jq type=string +FLAG basecamp people clients invite --json type=bool +FLAG basecamp people clients invite --markdown type=bool +FLAG basecamp people clients invite --md type=bool +FLAG basecamp people clients invite --no-hints type=bool +FLAG basecamp people clients invite --no-stats type=bool +FLAG basecamp people clients invite --profile type=string +FLAG basecamp people clients invite --project type=string +FLAG basecamp people clients invite --quiet type=bool +FLAG basecamp people clients invite --stats type=bool +FLAG basecamp people clients invite --styled type=bool +FLAG basecamp people clients invite --title type=string +FLAG basecamp people clients invite --todolist type=string +FLAG basecamp people clients invite --verbose type=count +FLAG basecamp people clients list --account type=string +FLAG basecamp people clients list --agent type=bool +FLAG basecamp people clients list --cache-dir type=string +FLAG basecamp people clients list --count type=bool +FLAG basecamp people clients list --help type=bool +FLAG basecamp people clients list --hints type=bool +FLAG basecamp people clients list --ids-only type=bool +FLAG basecamp people clients list --in type=string +FLAG basecamp people clients list --jq type=string +FLAG basecamp people clients list --json type=bool +FLAG basecamp people clients list --markdown type=bool +FLAG basecamp people clients list --md type=bool +FLAG basecamp people clients list --no-hints type=bool +FLAG basecamp people clients list --no-stats type=bool +FLAG basecamp people clients list --profile type=string +FLAG basecamp people clients list --project type=string +FLAG basecamp people clients list --quiet type=bool +FLAG basecamp people clients list --stats type=bool +FLAG basecamp people clients list --styled type=bool +FLAG basecamp people clients list --todolist type=string +FLAG basecamp people clients list --verbose type=count +FLAG basecamp people clients remove --account type=string +FLAG basecamp people clients remove --agent type=bool +FLAG basecamp people clients remove --cache-dir type=string +FLAG basecamp people clients remove --count type=bool +FLAG basecamp people clients remove --help type=bool +FLAG basecamp people clients remove --hints type=bool +FLAG basecamp people clients remove --ids-only type=bool +FLAG basecamp people clients remove --in type=string +FLAG basecamp people clients remove --jq type=string +FLAG basecamp people clients remove --json type=bool +FLAG basecamp people clients remove --markdown type=bool +FLAG basecamp people clients remove --md type=bool +FLAG basecamp people clients remove --no-hints type=bool +FLAG basecamp people clients remove --no-stats type=bool +FLAG basecamp people clients remove --profile type=string +FLAG basecamp people clients remove --project type=string +FLAG basecamp people clients remove --quiet type=bool +FLAG basecamp people clients remove --stats type=bool +FLAG basecamp people clients remove --styled type=bool +FLAG basecamp people clients remove --todolist type=string +FLAG basecamp people clients remove --verbose type=count FLAG basecamp people list --account type=string FLAG basecamp people list --agent type=bool FLAG basecamp people list --all type=bool @@ -18435,6 +18594,13 @@ SUB basecamp notifications list SUB basecamp notifications read SUB basecamp people SUB basecamp people add +SUB basecamp people clients +SUB basecamp people clients add +SUB basecamp people clients disable +SUB basecamp people clients enable +SUB basecamp people clients invite +SUB basecamp people clients list +SUB basecamp people clients remove SUB basecamp people list SUB basecamp people ooo SUB basecamp people out-of-office diff --git a/API-COVERAGE.md b/API-COVERAGE.md index 6c3f334f..88656873 100644 --- a/API-COVERAGE.md +++ b/API-COVERAGE.md @@ -6,12 +6,15 @@ Coverage of Basecamp 3 API endpoints. Source: [bc3-api/sections](https://github. | Status | Sections | Endpoints | |--------|----------|-----------| -| ✅ Implemented | 50 | 189 | +| ✅ Implemented | 50 | 192 | | ⚠️ Blocked | 0 | 0 | | ⏭️ Out of scope | 4 | 12 | -| **Total tracked** | **54** | **201** | +| **Total tracked** | **54** | **204** | -**189 of 189 tracked in-scope endpoints.** SDK v0.16.0 adds the three to-do +**192 of 192 tracked in-scope endpoints.** The client-admission endpoints +basecamp/bc3#13098 added — `PUT /projects/:id/people/client_users.json` and +`POST`/`DELETE /projects/:id/client_enablement.json` — land as `people clients`. +SDK v0.16.0 adds the three to-do list template-library operations, available through `templates library`, `templates copy`, and `templates copy-status`. The previous last gap — `GET /uploads/:id/versions.json` — closed with the v0.14.0 SDK bump. The command @@ -211,7 +214,7 @@ cannot faithfully cover at least one endpoint for a reason outside the CLI. A | drafts | 1 | `drafts` | ✅ | BC5 | - | list unpublished drafts across projects (server caps at 250). Bounded like the account-wide listings; publishing happens through the command for the draft's type | | my_notes | 2 | `notes` | ✅ | BC5 | - | show, set. A singleton per person, so no id and no listing. Pre-first-write the record does not exist yet and renders as empty rather than 404. `set` writes Markdown as HTML; attachments are out of scope | | **People** | -| people | 12 | `people`, `me` | ✅ | BC4 | - | list, show, update (edit your own profile via `PUT /my/profile.json`), out-of-office show/set/clear (`GetOutOfOffice`/`EnableOutOfOffice`/`DisableOutOfOffice`), pingable, add, remove (BC5: `tagline` alias of `bio` on person output) | +| people | 15 | `people`, `me` | ✅ | BC4 | - | list, show, update (edit your own profile via `PUT /my/profile.json`), out-of-office show/set/clear (`GetOutOfOffice`/`EnableOutOfOffice`/`DisableOutOfOffice`), pingable, add, remove (BC5: `tagline` alias of `bio` on person output). `people clients` covers the client side: `add`/`remove`/`invite` through `PUT /projects/:id/people/client_users.json` (basecamp/bc3#13098), `enable`/`disable` through `POST`/`DELETE /projects/:id/client_enablement.json`; `list` is the project roster filtered to `client: true` | | **Search & Recordings** | | my_assignments | 6 | `assignments` | ✅ | BC4 | - | list (priorities/non-priorities), completed, due (with scope filter), prioritize, deprioritize, reorder. `list` surfaces `priority_recording_id`, which is the only way to address a prioritized card-table step — it appears in no URL | | search | 2 | `search` | ✅ | BC4 | - | Full-text search + metadata. Filters: `--project`/`--in`, `--type`, `--creator`, `--since` (BC5-only), `--file-type`, `--exclude-chat`. Metadata lists recording/file search types | diff --git a/e2e/people.bats b/e2e/people.bats new file mode 100644 index 00000000..c48f1fe4 --- /dev/null +++ b/e2e/people.bats @@ -0,0 +1,80 @@ +#!/usr/bin/env bats +# people.bats - people command error handling (offline: every case resolves +# locally, before any request) + +load test_helper + + +# Help + +@test "people clients without subcommand shows help" { + run basecamp people clients + assert_success + assert_output_contains "COMMANDS" +} + + +# Missing context errors + +@test "people clients list without project shows error" { + create_credentials + create_global_config '{"account_id": 99999}' + + run basecamp people clients list --json + assert_failure + assert_json_value '.code' 'usage' + assert_json_value '.error' '--project (or --in) is required' +} + +@test "people clients add without ids shows error" { + create_credentials + create_global_config '{"account_id": 99999, "project_id": 123}' + + run basecamp people clients add --json + assert_failure + assert_json_value '.code' 'usage' + assert_json_value '.error' '... required' +} + + +# Invitee parsing + +@test "people clients invite rejects a token that is not an address" { + create_credentials + create_global_config '{"account_id": 99999, "project_id": 123}' + + run basecamp people clients invite "Annie Bryan" --json + assert_failure + assert_json_value '.code' 'usage' + assert_json_value '.error | contains("Name ")' 'true' +} + +@test "people clients invite - with empty pipe is a usage error" { + create_credentials + create_global_config '{"account_id": 99999, "project_id": 123}' + + run bash -c "printf '' | basecamp people clients invite - --json" + assert_failure + assert_json_value '.code' 'usage' + assert_output_contains "empty" +} + +@test "people clients invite - mixed with other invitees is rejected" { + create_credentials + create_global_config '{"account_id": 99999, "project_id": 123}' + + run bash -c "printf 'a@example.com' | basecamp people clients invite - b@example.com --json" + assert_failure + assert_json_value '.code' 'usage' + assert_output_contains "cannot be combined" +} + +@test "people clients invite --title with several invitees is rejected" { + create_credentials + create_global_config '{"account_id": 99999, "project_id": 123}' + + run basecamp people clients invite a@example.com b@example.com --title Owner --json + assert_failure + assert_json_value '.code' 'usage' + assert_output_contains "--title" +} diff --git a/e2e/smoke/smoke_lifecycle.bats b/e2e/smoke/smoke_lifecycle.bats index c0878bb6..58bff07b 100644 --- a/e2e/smoke/smoke_lifecycle.bats +++ b/e2e/smoke/smoke_lifecycle.bats @@ -137,6 +137,26 @@ load smoke_helper mark_out_of_scope "Modifies project membership" } +@test "people clients add is out of scope" { + mark_out_of_scope "Modifies project membership" +} + +@test "people clients remove is out of scope" { + mark_out_of_scope "Modifies project membership" +} + +@test "people clients invite is out of scope" { + mark_out_of_scope "Invites a new client by email — consumes an account seat" +} + +@test "people clients enable is out of scope" { + mark_out_of_scope "Reconfigures project-wide client visibility" +} + +@test "people clients disable is out of scope" { + mark_out_of_scope "Reconfigures project-wide client visibility" +} + @test "todos sweep is out of scope" { mark_out_of_scope "Bulk completion — destructive, no undo" } diff --git a/e2e/smoke/smoke_projects.bats b/e2e/smoke/smoke_projects.bats index b0ebcfbe..fdbb2c42 100644 --- a/e2e/smoke/smoke_projects.bats +++ b/e2e/smoke/smoke_projects.bats @@ -31,6 +31,18 @@ setup_file() { echo "$output" | jq -r '.data[0].id' > "$BATS_FILE_TMPDIR/project_id" } +@test "people clients list returns the project's clients" { + local proj_file="$BATS_FILE_TMPDIR/project_id" + [[ -f "$proj_file" ]] || mark_unverifiable "projects list did not produce a project ID" + local proj_id + proj_id=$(<"$proj_file") + + run_smoke basecamp people clients list --in "$proj_id" --json + assert_success + assert_json_value '.ok' 'true' + assert_json_value '.data | type' 'array' +} + @test "projects show returns project detail" { local proj_file="$BATS_FILE_TMPDIR/project_id" [[ -f "$proj_file" ]] || mark_unverifiable "projects list did not produce a project ID" diff --git a/go.mod b/go.mod index 78a6aa9c..e0947fda 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( charm.land/bubbles/v2 v2.2.1 charm.land/bubbletea/v2 v2.0.9 charm.land/lipgloss/v2 v2.0.6 - github.com/basecamp/basecamp-sdk/go v0.16.1-0.20260903193203-47e7ca381a49 + github.com/basecamp/basecamp-sdk/go v0.17.0 github.com/basecamp/cli v0.2.2-0.20260828230226-767413fc712d github.com/basecamp/mcp v0.0.0-20260828100356-2d6f44b51e9d github.com/basecamp/surfguard/go v0.1.0 diff --git a/go.sum b/go.sum index 3e14efb1..f200b7c8 100644 --- a/go.sum +++ b/go.sum @@ -87,8 +87,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.16.1-0.20260903193203-47e7ca381a49 h1:acgFif/siLT3R1/XWHdXLGGsE4x7HqKCvSaXw6j7xHo= -github.com/basecamp/basecamp-sdk/go v0.16.1-0.20260903193203-47e7ca381a49/go.mod h1:Cs9DV8iRJaVT4+IQXGZTeyG2nQAV2kQda7GHuBOeonY= +github.com/basecamp/basecamp-sdk/go v0.17.0 h1:u2kIDtr7bmHXLSkF61HXkLDymQdwoI+pzbT0F6OwHYM= +github.com/basecamp/basecamp-sdk/go v0.17.0/go.mod h1:Cs9DV8iRJaVT4+IQXGZTeyG2nQAV2kQda7GHuBOeonY= github.com/basecamp/cli v0.2.2-0.20260828230226-767413fc712d h1:jAzDrCCzDpIwhbFT1xVVs0z2xpXoDEkomHfKB2bUUp8= github.com/basecamp/cli v0.2.2-0.20260828230226-767413fc712d/go.mod h1:iTBTaWvsPEFIcZfkxQHEfISyJ6sZ7036K6bNx0RY3EE= github.com/basecamp/mcp v0.0.0-20260828100356-2d6f44b51e9d h1:zEQVGq1x1nhKMZ2TudFAcSJ32CHT8richI1vQakIKz4= diff --git a/internal/commands/commands.go b/internal/commands/commands.go index 6841afd3..61f738d1 100644 --- a/internal/commands/commands.go +++ b/internal/commands/commands.go @@ -88,7 +88,7 @@ func CommandCategories() []CommandCategory { { Name: "Organization", Commands: []CommandInfo{ - {Name: "people", Category: "organization", Description: "Manage people and access", Actions: []string{"list", "show", "update", "out-of-office", "pingable", "add", "remove"}}, + {Name: "people", Category: "organization", Description: "Manage people and access", Actions: []string{"list", "show", "update", "out-of-office", "pingable", "add", "remove", "clients"}}, {Name: "templates", Category: "organization", Description: "Manage project and to-do list templates", Actions: []string{"list", "show", "create", "update", "delete", "construct", "construction", "library", "copy", "copy-status"}}, {Name: "webhooks", Category: "organization", Description: "Manage webhooks", Actions: []string{"list", "show", "create", "update", "delete"}}, {Name: "lineup", Category: "organization", Description: "Manage lineup markers", Actions: []string{"list", "create", "update", "delete"}}, diff --git a/internal/commands/people.go b/internal/commands/people.go index 15af73c9..fe2335ff 100644 --- a/internal/commands/people.go +++ b/internal/commands/people.go @@ -1,7 +1,10 @@ package commands import ( + "context" + "errors" "fmt" + "net/mail" "slices" "sort" "strconv" @@ -146,7 +149,7 @@ func NewPeopleCmd() *cobra.Command { Use: "people [action]", Short: "Manage people", Long: "List, show, and manage people in your Basecamp account.", - Annotations: map[string]string{"agent_notes": "--assignee me resolves to the current user's ID automatically\nPerson IDs are needed for --participants, --people, assign --to\nbasecamp people pingable lists people who can be @mentioned"}, + Annotations: map[string]string{"agent_notes": "--assignee me resolves to the current user's ID automatically\nPerson IDs are needed for --participants, --people, assign --to\nbasecamp people pingable lists people who can be @mentioned\nadd/remove manage team members only; clients go through basecamp people clients"}, } cmd.AddCommand(newPeopleListCmd()) @@ -156,6 +159,7 @@ func NewPeopleCmd() *cobra.Command { cmd.AddCommand(newPeoplePingableCmd()) cmd.AddCommand(newPeopleAddCmd()) cmd.AddCommand(newPeopleRemoveCmd()) + cmd.AddCommand(newPeopleClientsCmd()) return cmd } @@ -525,6 +529,7 @@ func runPeopleList(cmd *cobra.Command, projectID string, limit, page int, all bo Title string `json:"title"` Employee bool `json:"employee"` Admin bool `json:"admin"` + Client bool `json:"client"` } items := make([]personListItem, len(people)) for i, p := range people { @@ -535,6 +540,7 @@ func runPeopleList(cmd *cobra.Command, projectID string, limit, page int, all bo Title: p.Title, Employee: p.Employee, Admin: p.Admin, + Client: p.Client, } } @@ -812,3 +818,749 @@ func runPeopleRemove(cmd *cobra.Command, personIDs []string, projectID string) e output.WithBreadcrumbs(breadcrumbs...), ) } + +// newPeopleClientsCmd groups the client-side counterpart of people add/remove. +// +// Basecamp keeps clients and team members apart on the wire: the team endpoint +// (PUT /projects/:id/people/users.json) silently drops a client's id, and the +// client endpoint (PUT /projects/:id/people/client_users.json) rejects a team +// member's, so neither can cross-grade someone into the other kind of access. +// The CLI mirrors that split with a separate group rather than a --client flag. +func newPeopleClientsCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "clients", + Short: "Manage clients on a project", + Long: `List, add, remove, and invite clients on a project, and turn client +access on or off. + +Clients are external collaborators who see only what the project shares with +them. A project must have clients enabled before any can be added, and +enabling is a deliberate, separate step: it applies the project's default +client visibility (the timeline and most tools become visible; the card table, +Campfire, and Doors stay private), so adding a client never enables the +project implicitly. + + basecamp people clients enable --in + basecamp people clients invite "Annie Bryan " --in + basecamp people clients add 1049715915 --in + basecamp people clients list --in + basecamp people clients remove 1049715915 --in + basecamp people clients disable --in + +Only client users go through here. Team members use "basecamp people add" +and "basecamp people remove".`, + Annotations: map[string]string{ + "agent_notes": "Enable clients on the project before adding or inviting any; the API answers 403 otherwise.\n" + + "add takes existing client users (id, email, or name); invite creates new clients by email.\n" + + "Invitations are all-or-nothing: one bad address or a seat shortfall invites nobody.\n" + + "disable refuses while any client still has access; remove them first.", + }, + } + + cmd.AddCommand( + newPeopleClientsListCmd(), + newPeopleClientsAddCmd(), + newPeopleClientsRemoveCmd(), + newPeopleClientsInviteCmd(), + newPeopleClientsEnableCmd(), + newPeopleClientsDisableCmd(), + ) + + return cmd +} + +// addProjectFlags registers the --project/--in pair every clients verb takes +// and returns the value they share. The two spellings are one flag: --in is +// the repo-wide alias for --project and both bind to the same variable. +func addProjectFlags(cmd *cobra.Command, projectID *string, usage string) { + cmd.Flags().StringVarP(projectID, "project", "p", "", usage) + cmd.Flags().StringVar(projectID, "in", "", usage+" (alias for --project)") + + completer := completion.NewCompleter(nil) + _ = cmd.RegisterFlagCompletionFunc("project", completer.ProjectNameCompletion()) + _ = cmd.RegisterFlagCompletionFunc("in", completer.ProjectNameCompletion()) +} + +// requireProject settles the project from the flag, then the global --project, +// then the configured default (.basecamp/config.json), as a usage error when +// none names one. +func requireProject(cmd *cobra.Command, projectID string) (string, error) { + app := appctx.FromContext(cmd.Context()) + if projectID == "" { + projectID = app.Flags.Project + } + if projectID == "" { + projectID = app.Config.ProjectID + } + if projectID == "" { + return "", output.ErrUsage("--project (or --in) is required") + } + return projectID, nil +} + +// resolveProjectBucket resolves a project name, id, or URL to its bucket id, +// returning both the numeric id and its string form for messages. +func resolveProjectBucket(cmd *cobra.Command, app *appctx.App, projectID string) (int64, string, error) { + resolvedProjectID, _, err := app.Names.ResolveProject(cmd.Context(), projectID) + if err != nil { + return 0, "", err + } + bucketID, err := strconv.ParseInt(resolvedProjectID, 10, 64) + if err != nil { + return 0, "", output.ErrUsage("Invalid project ID") + } + return bucketID, resolvedProjectID, nil +} + +// resolvePeopleArgs resolves each positional id, email, or name to a person +// id, in the order given. Positionals are already split, so unlike the +// comma-separated resolvePersonIDs a name containing a comma stays whole. +func resolvePeopleArgs(cmd *cobra.Command, app *appctx.App, people []string) ([]int64, error) { + ids := make([]int64, 0, len(people)) + for _, person := range people { + resolvedID, _, err := app.Names.ResolvePerson(cmd.Context(), person) + if err != nil { + return nil, err + } + id, err := strconv.ParseInt(resolvedID, 10, 64) + if err != nil { + return nil, output.ErrUsage("Invalid person ID") + } + ids = append(ids, id) + } + return ids, nil +} + +func newPeopleClientsListCmd() *cobra.Command { + var projectID string + + cmd := &cobra.Command{ + Use: "list", + Short: "List clients on a project", + Long: `List the clients who have access to a project. + +This is the project's people list narrowed to client users. Team members are +listed by "basecamp people list --in ", which also reports each +person's "client" flag. + + basecamp people clients list --in `, + Example: `basecamp people clients list --in `, + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, args []string) error { + projectID, err := requireProject(cmd, projectID) + if err != nil { + return err + } + return runPeopleClientsList(cmd, projectID) + }, + } + + addProjectFlags(cmd, &projectID, "Project to list clients on (required)") + + return cmd +} + +func runPeopleClientsList(cmd *cobra.Command, projectID string) error { + app := appctx.FromContext(cmd.Context()) + + if err := ensureAccount(cmd, app); err != nil { + return err + } + + bucketID, resolvedProjectID, err := resolveProjectBucket(cmd, app, projectID) + if err != nil { + return err + } + + // The project roster is small and unpaginated in practice; fetch it whole + // so the client filter sees everyone rather than a first page. + result, err := app.Account().People().ListProjectPeople(cmd.Context(), bucketID, &basecamp.PeopleListOptions{}) + if err != nil { + return convertSDKError(err) + } + + type clientListItem struct { + ID int64 `json:"id"` + Name string `json:"name"` + EmailAddress string `json:"email_address"` + Title string `json:"title"` + Company string `json:"company,omitempty"` + } + items := make([]clientListItem, 0) + for _, p := range result.People { + if !p.Client { + continue + } + item := clientListItem{ID: p.ID, Name: p.Name, EmailAddress: p.EmailAddress, Title: p.Title} + if p.Company != nil { + item.Company = p.Company.Name + } + items = append(items, item) + } + sort.Slice(items, func(i, j int) bool { + return strings.ToLower(items[i].Name) < strings.ToLower(items[j].Name) + }) + + return app.OK(items, + output.WithSummary(fmt.Sprintf("%d client(s) on project #%s", len(items), resolvedProjectID)), + output.WithBreadcrumbs( + output.Breadcrumb{Action: "add", Cmd: fmt.Sprintf("basecamp people clients add --in %s", resolvedProjectID), Description: "Add an existing client"}, + output.Breadcrumb{Action: "invite", Cmd: fmt.Sprintf("basecamp people clients invite --in %s", resolvedProjectID), Description: "Invite a new client"}, + ), + ) +} + +// clientInvitee is one row of a client invitation. +type clientInvitee struct { + Name string + EmailAddress string +} + +// parseClientInvitees reads each token as a bare email address or as +// "Name ", the RFC 5322 mailbox form net/mail accepts, so a quoted +// display name and a bare address both work without a second flag. A missing +// name is left empty and the server defaults it to the address. Every +// malformed token is named in one usage error, before any request. +func parseClientInvitees(tokens []string) ([]clientInvitee, error) { + invitees := make([]clientInvitee, 0, len(tokens)) + var malformed []string + for _, token := range tokens { + addr, err := mail.ParseAddress(strings.TrimSpace(token)) + if err != nil { + malformed = append(malformed, fmt.Sprintf("%q", token)) + continue + } + invitees = append(invitees, clientInvitee{Name: addr.Name, EmailAddress: addr.Address}) + } + if len(malformed) != 0 { + return nil, output.ErrUsageHint( + fmt.Sprintf(`Not an email address or "Name ": %s`, strings.Join(malformed, ", ")), + `Name each client by email address, or as "Full Name " to set the name; nothing was sent`) + } + return invitees, nil +} + +// resolveClientInviteeTokens turns the invite positionals into invitee tokens: +// exactly ["-"] reads one invitee per line from piped stdin, anything else is +// taken as given. A "-" mixed with other tokens is a usage error, the same +// rule the join-all content commands apply. +func resolveClientInviteeTokens(cmd *cobra.Command, args []string) ([]string, error) { + dashes := 0 + for _, a := range args { + if a == "-" { + dashes++ + } + } + if dashes == 0 { + return args, nil + } + if len(args) != 1 { + return nil, output.ErrUsageHint( + `"-" cannot be combined with other invitees`, + `Pass "-" alone to read one invitee per line from stdin, or list the invitees as arguments`) + } + content, err := readStdinContent(cmd, "") + if err != nil { + return nil, err + } + var tokens []string + for _, line := range strings.Split(content, "\n") { + if line = strings.TrimSpace(line); line != "" { + tokens = append(tokens, line) + } + } + return tokens, nil +} + +// clientsForbiddenError explains a 403 from the client endpoints. The server +// answers the same empty 403 for "clients are not enabled here" and "you may +// not manage people here", so on that status the project is read back once to +// tell them apart and name the fix. Any other error converts as usual. +func clientsForbiddenError(ctx context.Context, app *appctx.App, bucketID int64, projectRef string, err error) error { + var sdkErr *basecamp.Error + if !errors.As(err, &sdkErr) || sdkErr.Code != basecamp.CodeForbidden { + return convertSDKError(err) + } + if project, getErr := app.Account().Projects().Get(ctx, bucketID); getErr == nil && !project.ClientsEnabled { + return &output.Error{ + Code: output.CodeForbidden, + Message: fmt.Sprintf("Clients are not enabled on project #%s", projectRef), + Hint: fmt.Sprintf("Enable them first: basecamp people clients enable --in %s", projectRef), + HTTPStatus: sdkErr.HTTPStatus, + Cause: sdkErr, + } + } + return &output.Error{ + Code: output.CodeForbidden, + Message: "Access denied: managing clients on this project requires permission to manage its people", + Hint: sdkErr.Hint, + HTTPStatus: sdkErr.HTTPStatus, + Cause: sdkErr, + } +} + +// clientSeatLimitError re-reads the 429 the client endpoint answers when the +// new addresses would exceed the account's user limit. That is a verdict, not +// throttling: no Retry-After accompanies it and waiting cannot change it, so it +// is reported as the account-limit code (the one a 507 carries elsewhere) and +// not as a retryable rate limit. A 429 that does name a Retry-After is real +// throttling and converts as usual. No count is stated: addresses already on +// the account take no seat and a repeated address counts once, and only the +// server knows which rows those are. +func clientSeatLimitError(err error) error { + var sdkErr *basecamp.Error + if !errors.As(err, &sdkErr) || sdkErr.Code != basecamp.CodeRateLimit || sdkErr.RetryAfter > 0 { + return convertSDKError(err) + } + return &output.Error{ + Code: output.CodeLimitExceeded, + Message: "Not enough seats on the account for the new clients in this batch", + Hint: "The account's user limit would be exceeded; nobody was invited. Free up seats or raise the limit, then retry", + HTTPStatus: sdkErr.HTTPStatus, + Retryable: false, + Cause: sdkErr, + } +} + +func newPeopleClientsAddCmd() *cobra.Command { + var projectID string + + cmd := &cobra.Command{ + Use: "add ...", + Short: "Add existing clients to a project", + Long: `Grant existing client users access to a project. + +Name each client by id, email address, or name. Only client users are +granted: the server drops a team member's id rather than cross-grading them +into client access, and any id it did not grant is reported in the notice. +To invite someone who is not on the account yet, use "basecamp people +clients invite". + + basecamp people clients add 1049715915 --in + basecamp people clients add annie@example.com "Bob Client" --in `, + Example: `basecamp people clients add 1049715915 --in `, + RunE: func(cmd *cobra.Command, args []string) error { + if len(args) == 0 { + return missingArg(cmd, "...") + } + projectID, err := requireProject(cmd, projectID) + if err != nil { + return err + } + return runPeopleClientsAccess(cmd, projectID, args, clientAccessGrant) + }, + } + + addProjectFlags(cmd, &projectID, "Project to add clients to (required)") + + return cmd +} + +func newPeopleClientsRemoveCmd() *cobra.Command { + var projectID string + + cmd := &cobra.Command{ + Use: "remove ...", + Short: "Remove clients from a project", + Long: `Revoke clients' access to a project. + +Name each client by id, email address, or name. Only client users are +revoked; any id the server did not revoke is reported in the notice. + + basecamp people clients remove 1049715915 --in `, + Example: `basecamp people clients remove 1049715915 --in `, + RunE: func(cmd *cobra.Command, args []string) error { + if len(args) == 0 { + return missingArg(cmd, "...") + } + projectID, err := requireProject(cmd, projectID) + if err != nil { + return err + } + return runPeopleClientsAccess(cmd, projectID, args, clientAccessRevoke) + }, + } + + addProjectFlags(cmd, &projectID, "Project to remove clients from (required)") + + return cmd +} + +// clientAccessChange selects which side of the client access triad a verb +// drives; add and remove differ only in which list carries the ids. +type clientAccessChange int + +const ( + clientAccessGrant clientAccessChange = iota + clientAccessRevoke +) + +func runPeopleClientsAccess(cmd *cobra.Command, projectID string, people []string, change clientAccessChange) error { + app := appctx.FromContext(cmd.Context()) + + if err := ensureAccount(cmd, app); err != nil { + return err + } + + bucketID, resolvedProjectID, err := resolveProjectBucket(cmd, app, projectID) + if err != nil { + return err + } + + ids, err := resolvePeopleArgs(cmd, app, people) + if err != nil { + return err + } + + req := &basecamp.UpdateProjectClientAccessRequest{} + if change == clientAccessGrant { + req.Grant = ids + } else { + req.Revoke = ids + } + + result, err := app.Account().People().UpdateProjectClientAccess(cmd.Context(), bucketID, req) + if err != nil { + return clientsForbiddenError(cmd.Context(), app, bucketID, resolvedProjectID, err) + } + + affected := result.Granted + verb, missedWhy := "Added", "already on the project, or not a client user" + if change == clientAccessRevoke { + affected = result.Revoked + verb, missedWhy = "Removed", "not on the project, or not a client user" + } + + respOpts := []output.ResponseOption{ + output.WithSummary(fmt.Sprintf("%s %d client(s) %s project #%s", verb, len(affected), accessPreposition(change), resolvedProjectID)), + output.WithBreadcrumbs(output.Breadcrumb{ + Action: "list", Cmd: fmt.Sprintf("basecamp people clients list --in %s", resolvedProjectID), Description: "List the project's clients", + }), + } + if missed := unaffectedPersonIDs(ids, affected); len(missed) != 0 { + respOpts = append(respOpts, output.WithDiagnostic( + fmt.Sprintf("Not %s (%s): %s", strings.ToLower(verb), missedWhy, joinInt64s(missed, ", ")))) + } + + return app.OK(result, respOpts...) +} + +func accessPreposition(change clientAccessChange) string { + if change == clientAccessRevoke { + return "from" + } + return "to" +} + +// unaffectedPersonIDs lists the requested ids absent from the server's +// granted/revoked echo, in request order and without repeats: the endpoint +// silently drops an ineligible id, and the caller needs to know which. +func unaffectedPersonIDs(requested []int64, affected []basecamp.Person) []int64 { + seen := make(map[int64]bool, len(affected)) + for _, p := range affected { + seen[p.ID] = true + } + var missed []int64 + for _, id := range requested { + if !seen[id] { + seen[id] = true + missed = append(missed, id) + } + } + return missed +} + +// joinInt64s renders ids separated by sep: ", " for prose, " " for a command +// line the caller is meant to paste. +func joinInt64s(ids []int64, sep string) string { + parts := make([]string, len(ids)) + for i, id := range ids { + parts[i] = strconv.FormatInt(id, 10) + } + return strings.Join(parts, sep) +} + +func newPeopleClientsInviteCmd() *cobra.Command { + var projectID, company, title string + + cmd := &cobra.Command{ + Use: "invite ...", + Short: "Invite new clients to a project by email", + Long: `Invite people who are not on the account yet as clients on a project. + +Each invitee is an email address, or "Name " to set the display name +(it defaults to the address). Pass "-" alone to read one invitee per line +from stdin. --company applies to every invitee in the invocation; --title +names one person's role, so it takes exactly one invitee. + +A token that is not an address is refused here, before any request, as a +usage error naming each one (exit 2). Invitations the server accepts are +all-or-nothing: an address Basecamp rejects fails the whole batch with each +rejected row named (exit 9, validation), and a batch that would exceed the +account's user limit fails with nobody invited (exit 10, limit_exceeded). +Addresses already on the account do not consume a seat. Clients must be +enabled on the project first; see "basecamp people clients enable". + + basecamp people clients invite annie@example.com --in + basecamp people clients invite "Annie Bryan " --in --company "Springfield Elementary" --title Owner + printf 'annie@example.com\nBob Client \n' | basecamp people clients invite - --in `, + Example: `basecamp people clients invite "Annie Bryan " --in `, + RunE: func(cmd *cobra.Command, args []string) error { + if len(args) == 0 { + return missingArg(cmd, "...") + } + projectID, err := requireProject(cmd, projectID) + if err != nil { + return err + } + return runPeopleClientsInvite(cmd, projectID, args, company, title) + }, + } + + addProjectFlags(cmd, &projectID, "Project to invite clients to (required)") + cmd.Flags().StringVar(&company, "company", "", "Company name for every invitee") + cmd.Flags().StringVar(&title, "title", "", "Job title (a single invitee only)") + allowDash(cmd, "arg:0") + + return cmd +} + +func runPeopleClientsInvite(cmd *cobra.Command, projectID string, args []string, company, title string) error { + app := appctx.FromContext(cmd.Context()) + + tokens, err := resolveClientInviteeTokens(cmd, args) + if err != nil { + return err + } + invitees, err := parseClientInvitees(tokens) + if err != nil { + return err + } + if title != "" && len(invitees) != 1 { + return output.ErrUsage("--title names one person's role; invite them on their own to set it") + } + + if err := ensureAccount(cmd, app); err != nil { + return err + } + + bucketID, resolvedProjectID, err := resolveProjectBucket(cmd, app, projectID) + if err != nil { + return err + } + + req := &basecamp.UpdateProjectClientAccessRequest{Create: make([]basecamp.CreateClientRequest, 0, len(invitees))} + for _, invitee := range invitees { + req.Create = append(req.Create, basecamp.CreateClientRequest{ + EmailAddress: invitee.EmailAddress, + Name: invitee.Name, + Title: title, + CompanyName: company, + }) + } + + result, err := app.Account().People().UpdateProjectClientAccess(cmd.Context(), bucketID, req) + if err != nil { + return clientInviteError(cmd.Context(), app, bucketID, resolvedProjectID, err, invitees) + } + + return app.OK(result, + output.WithSummary(fmt.Sprintf("Invited %d client(s) to project #%s", len(result.Granted), resolvedProjectID)), + output.WithBreadcrumbs(output.Breadcrumb{ + Action: "list", Cmd: fmt.Sprintf("basecamp people clients list --in %s", resolvedProjectID), Description: "List the project's clients", + }), + ) +} + +// clientInviteError maps the invite endpoint's three refusals: a 403 for +// clients being off (or no permission), a 429 for the seat limit, and a 422 +// naming each rejected row. +func clientInviteError(ctx context.Context, app *appctx.App, bucketID int64, projectRef string, err error, invitees []clientInvitee) error { + var sdkErr *basecamp.Error + if errors.As(err, &sdkErr) { + switch sdkErr.Code { + case basecamp.CodeForbidden: + return clientsForbiddenError(ctx, app, bucketID, projectRef, err) + case basecamp.CodeRateLimit: + return clientSeatLimitError(err) + case basecamp.CodeValidation: + return clientInviteValidationError(sdkErr, invitees) + } + } + return convertSDKError(err) +} + +// clientInviteValidationError renders the all-or-nothing 422. The wire body is +// row-keyed — {"errors": [{"email_address", "messages"}]}. When the SDK exposes +// it as FieldErrors keyed by address, the hint names each rejected row; when it +// carries only the status, the hint names the addresses submitted so the caller +// still knows which batch was refused. +func clientInviteValidationError(sdkErr *basecamp.Error, invitees []clientInvitee) error { + hint := "Nobody was invited. Check each address, then retry the whole batch" + if len(sdkErr.FieldErrors) != 0 { + addresses := make([]string, 0, len(sdkErr.FieldErrors)) + for address := range sdkErr.FieldErrors { + addresses = append(addresses, address) + } + sort.Strings(addresses) + rows := make([]string, 0, len(addresses)) + for _, address := range addresses { + rows = append(rows, fmt.Sprintf("%s: %s", address, strings.Join(sdkErr.FieldErrors[address], "; "))) + } + hint = "Nobody was invited. Rejected:\n" + strings.Join(rows, "\n") + } else if len(invitees) != 0 { + addresses := make([]string, len(invitees)) + for i, invitee := range invitees { + addresses[i] = invitee.EmailAddress + } + hint = "Nobody was invited. Basecamp rejected at least one of: " + strings.Join(addresses, ", ") + } + return &output.Error{ + Code: output.CodeValidation, + Message: "Invitation rejected: at least one client row is invalid", + Hint: hint, + HTTPStatus: sdkErr.HTTPStatus, + Cause: sdkErr, + } +} + +func newPeopleClientsEnableCmd() *cobra.Command { + var projectID string + + cmd := &cobra.Command{ + Use: "enable", + Short: "Turn on client access for a project", + Long: `Enable clients on a project so they can be added to it. + +This is a deliberate, separate step from adding clients: it applies the +project's default client visibility, so the timeline and most docked tools +become visible to clients (the card table, Campfire, and Doors stay private) +and later content inherits that default. Adjust visibility per tool and per +item from there. Refused (403) unless the account supports clients and this +is a standard project. + + basecamp people clients enable --in `, + Example: `basecamp people clients enable --in `, + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, args []string) error { + projectID, err := requireProject(cmd, projectID) + if err != nil { + return err + } + return runPeopleClientsEnablement(cmd, projectID, true) + }, + } + + addProjectFlags(cmd, &projectID, "Project to enable clients on (required)") + + return cmd +} + +func newPeopleClientsDisableCmd() *cobra.Command { + var projectID string + + cmd := &cobra.Command{ + Use: "disable", + Short: "Turn off client access for a project", + Long: `Disable clients on a project. + +Refused (403) while any client still has access; remove them first with +"basecamp people clients remove". + + basecamp people clients disable --in `, + Example: `basecamp people clients disable --in `, + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, args []string) error { + projectID, err := requireProject(cmd, projectID) + if err != nil { + return err + } + return runPeopleClientsEnablement(cmd, projectID, false) + }, + } + + addProjectFlags(cmd, &projectID, "Project to disable clients on (required)") + + return cmd +} + +func runPeopleClientsEnablement(cmd *cobra.Command, projectID string, enable bool) error { + app := appctx.FromContext(cmd.Context()) + + if err := ensureAccount(cmd, app); err != nil { + return err + } + + bucketID, resolvedProjectID, err := resolveProjectBucket(cmd, app, projectID) + if err != nil { + return err + } + + var result *basecamp.ProjectClientEnablement + if enable { + result, err = app.Account().People().EnableProjectClients(cmd.Context(), bucketID) + } else { + result, err = app.Account().People().DisableProjectClients(cmd.Context(), bucketID) + } + if err != nil { + return clientEnablementError(cmd.Context(), app, bucketID, resolvedProjectID, enable, err) + } + + if enable { + return app.OK(result, + output.WithSummary(fmt.Sprintf("Enabled clients on project #%s", resolvedProjectID)), + output.WithBreadcrumbs( + output.Breadcrumb{Action: "invite", Cmd: fmt.Sprintf("basecamp people clients invite --in %s", resolvedProjectID), Description: "Invite a new client"}, + output.Breadcrumb{Action: "add", Cmd: fmt.Sprintf("basecamp people clients add --in %s", resolvedProjectID), Description: "Add an existing client"}, + ), + ) + } + return app.OK(result, + output.WithSummary(fmt.Sprintf("Disabled clients on project #%s", resolvedProjectID)), + ) +} + +// clientEnablementError explains the two 403s the enablement endpoint answers. +// Enabling is refused when the project cannot have clients at all; disabling +// is refused while any client keeps access, so the roster is read back once +// and the remaining clients are named only when it shows some — the same 403 +// also answers a caller who may not manage the project's people. +func clientEnablementError(ctx context.Context, app *appctx.App, bucketID int64, projectRef string, enable bool, err error) error { + var sdkErr *basecamp.Error + if !errors.As(err, &sdkErr) || sdkErr.Code != basecamp.CodeForbidden { + return convertSDKError(err) + } + if enable { + return &output.Error{ + Code: output.CodeForbidden, + Message: fmt.Sprintf("Clients cannot be enabled on project #%s", projectRef), + Hint: "The account may not support clients, this may not be a standard project, or you may lack permission to manage its people", + HTTPStatus: sdkErr.HTTPStatus, + Cause: sdkErr, + } + } + if roster, listErr := app.Account().People().ListProjectPeople(ctx, bucketID, &basecamp.PeopleListOptions{}); listErr == nil { + var ids []int64 + for _, p := range roster.People { + if p.Client { + ids = append(ids, p.ID) + } + } + if len(ids) != 0 { + return &output.Error{ + Code: output.CodeForbidden, + Message: fmt.Sprintf("Clients cannot be disabled on project #%s while %d client(s) still have access", projectRef, len(ids)), + Hint: fmt.Sprintf("Remove them first: basecamp people clients remove %s --in %s", joinInt64s(ids, " "), projectRef), + HTTPStatus: sdkErr.HTTPStatus, + Cause: sdkErr, + } + } + } + return &output.Error{ + Code: output.CodeForbidden, + Message: fmt.Sprintf("Clients cannot be disabled on project #%s", projectRef), + Hint: "Disabling is refused while any client still has access, and requires permission to manage the project's people: basecamp people clients list --in " + projectRef, + HTTPStatus: sdkErr.HTTPStatus, + Cause: sdkErr, + } +} diff --git a/internal/commands/people_clients_test.go b/internal/commands/people_clients_test.go new file mode 100644 index 00000000..d023b689 --- /dev/null +++ b/internal/commands/people_clients_test.go @@ -0,0 +1,458 @@ +package commands + +import ( + "encoding/json" + "errors" + "net/http" + "strings" + "testing" + + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/basecamp/basecamp-sdk/go/pkg/basecamp" + + "github.com/basecamp/basecamp-cli/internal/output" +) + +const clientsProjectPeoplePath = "/99999/projects/123/people.json" + +func clientsProjectPeopleRoute() stubRoute { + body := `[ + {"id":1001,"name":"Zed Team","email_address":"zed@example.com","employee":true}, + {"id":3002,"name":"beth client","email_address":"beth@springfield.example.com","client":true,"title":"Owner","company":{"id":9,"name":"Springfield Elementary"}}, + {"id":3001,"name":"Annie Bryan","email_address":"annie@springfield.example.com","client":true} + ]` + return stubRoute{ + method: http.MethodGet, + path: clientsProjectPeoplePath, + status: http.StatusOK, + body: body, + pages: []string{body}, + } +} + +func TestPeopleClientsListKeepsOnlyClients(t *testing.T) { + app, _, out := setupPersonalFeedApp(t, projectsRoute(), clientsProjectPeopleRoute()) + + require.NoError(t, executeRecordingCommand(NewPeopleCmd(), app, "clients", "list", "--in", "123")) + + var envelope struct { + Data []struct { + ID int64 `json:"id"` + Name string `json:"name"` + EmailAddress string `json:"email_address"` + Company string `json:"company"` + } `json:"data"` + Summary string `json:"summary"` + } + require.NoError(t, json.Unmarshal(out.Bytes(), &envelope), "output: %s", out.String()) + require.Len(t, envelope.Data, 2) + assert.Equal(t, int64(3001), envelope.Data[0].ID, "sorted by name, case-insensitively") + assert.Equal(t, int64(3002), envelope.Data[1].ID) + assert.Equal(t, "Springfield Elementary", envelope.Data[1].Company) + assert.Equal(t, "2 client(s) on project #123", envelope.Summary) +} + +func TestPeopleClientsListRequiresProject(t *testing.T) { + app, transport, _ := setupPersonalFeedApp(t) + + err := executeRecordingCommand(NewPeopleCmd(), app, "clients", "list") + requireBookmarksUsageError(t, err) + assert.Empty(t, transport.recorded()) +} + +func TestParseClientInvitees(t *testing.T) { + invitees, err := parseClientInvitees([]string{ + "annie@example.com", + "Annie Bryan ", + `"Bryan, Annie" `, + " ", + }) + require.NoError(t, err) + assert.Equal(t, []clientInvitee{ + {EmailAddress: "annie@example.com"}, + {Name: "Annie Bryan", EmailAddress: "annie@example.com"}, + {Name: "Bryan, Annie", EmailAddress: "annie@example.com"}, + {EmailAddress: "annie@example.com"}, + }, invitees) +} + +func TestParseClientInviteesRejectsNonAddresses(t *testing.T) { + for _, bad := range []string{"Annie Bryan", "annie@", "annie@example.com bob@example.com", ""} { + _, err := parseClientInvitees([]string{bad}) + requireBookmarksUsageError(t, err) + } +} + +// Every malformed token is named at once, so a batch is fixed in one pass. +func TestParseClientInviteesNamesEveryMalformedToken(t *testing.T) { + _, err := parseClientInvitees([]string{"ok@example.com", "Annie Bryan", "annie@"}) + + outErr := requireBookmarksUsageError(t, err) + assert.Contains(t, outErr.Message, `"Annie Bryan", "annie@"`) + assert.NotContains(t, outErr.Message, "ok@example.com") +} + +// people list carries each person's client flag, the only way a caller can +// tell which ids belong on people clients add rather than people add. +func TestPeopleListReportsTheClientFlag(t *testing.T) { + app, _, out := setupPersonalFeedApp(t, accountPeopleRoute()) + + require.NoError(t, executeRecordingCommand(NewPeopleCmd(), app, "list")) + + var envelope struct { + Data []struct { + ID int64 `json:"id"` + Client bool `json:"client"` + } `json:"data"` + } + require.NoError(t, json.Unmarshal(out.Bytes(), &envelope), "output: %s", out.String()) + clientByID := map[int64]bool{} + for _, p := range envelope.Data { + clientByID[p.ID] = p.Client + } + assert.True(t, clientByID[3001]) + assert.False(t, clientByID[1001]) +} + +func TestResolveClientInviteeTokensReadsStdinLines(t *testing.T) { + cmd := &cobra.Command{Use: "invite"} + cmd.SetIn(strings.NewReader("annie@example.com\r\n\n Annie Bryan \n")) + + tokens, err := resolveClientInviteeTokens(cmd, []string{"-"}) + require.NoError(t, err) + assert.Equal(t, []string{"annie@example.com", "Annie Bryan "}, tokens) +} + +func TestResolveClientInviteeTokensPassesArgsThrough(t *testing.T) { + cmd := &cobra.Command{Use: "invite"} + cmd.SetIn(strings.NewReader("ignored@example.com\n")) + + tokens, err := resolveClientInviteeTokens(cmd, []string{"a@example.com", "b@example.com"}) + require.NoError(t, err) + assert.Equal(t, []string{"a@example.com", "b@example.com"}, tokens) +} + +func TestResolveClientInviteeTokensRejectsMixedDash(t *testing.T) { + cmd := &cobra.Command{Use: "invite"} + cmd.SetIn(strings.NewReader("annie@example.com\n")) + + _, err := resolveClientInviteeTokens(cmd, []string{"-", "b@example.com"}) + requireBookmarksUsageError(t, err) +} + +func clientsProjectRoute(clientsEnabled bool) stubRoute { + body := `{"id":123,"name":"Test Project","clients_enabled":false}` + if clientsEnabled { + body = `{"id":123,"name":"Test Project","clients_enabled":true}` + } + return stubRoute{method: http.MethodGet, path: "/99999/projects/123", status: http.StatusOK, body: body} +} + +func TestClientsForbiddenErrorNamesDisabledClients(t *testing.T) { + app, _ := setupRecordingTestApp(t, clientsProjectRoute(false)) + + err := clientsForbiddenError(t.Context(), app, 123, "123", basecamp.ErrForbidden("access denied")) + + var outErr *output.Error + require.True(t, errors.As(err, &outErr)) + assert.Equal(t, output.CodeForbidden, outErr.Code) + assert.Equal(t, "Clients are not enabled on project #123", outErr.Message) + assert.Contains(t, outErr.Hint, "basecamp people clients enable --in 123") +} + +func TestClientsForbiddenErrorFallsBackToPermission(t *testing.T) { + app, _ := setupRecordingTestApp(t, clientsProjectRoute(true)) + + err := clientsForbiddenError(t.Context(), app, 123, "123", basecamp.ErrForbidden("access denied")) + + var outErr *output.Error + require.True(t, errors.As(err, &outErr)) + assert.Equal(t, output.CodeForbidden, outErr.Code) + assert.Contains(t, outErr.Message, "permission to manage its people") +} + +func TestClientsForbiddenErrorPassesOtherErrorsThrough(t *testing.T) { + app, transport := setupRecordingTestApp(t) + + err := clientsForbiddenError(t.Context(), app, 123, "123", basecamp.ErrNotFound("project", "123")) + + var outErr *output.Error + require.True(t, errors.As(err, &outErr)) + assert.Equal(t, output.CodeNotFound, outErr.Code) + assert.Empty(t, transport.recorded(), "no project read-back for a non-403") +} + +func TestClientSeatLimitErrorRemapsBare429(t *testing.T) { + err := clientSeatLimitError(basecamp.ErrRateLimit(0)) + + var outErr *output.Error + require.True(t, errors.As(err, &outErr)) + assert.Equal(t, output.CodeLimitExceeded, outErr.Code) + assert.False(t, outErr.Retryable) + assert.Equal(t, output.ExitLimit, output.ExitCodeFor(outErr.Code)) + assert.Contains(t, outErr.Message, "Not enough seats") +} + +func TestClientSeatLimitErrorKeepsRealThrottling(t *testing.T) { + err := clientSeatLimitError(basecamp.ErrRateLimit(30)) + + var outErr *output.Error + require.True(t, errors.As(err, &outErr)) + assert.Equal(t, output.CodeRateLimit, outErr.Code) + assert.True(t, outErr.Retryable) +} + +const ( + clientUsersPath = "/99999/projects/123/people/client_users.json" + clientEnablementPath = "/99999/projects/123/client_enablement.json" +) + +func clientUsersRoute(status int, body string) stubRoute { + return stubRoute{method: http.MethodPut, path: clientUsersPath, status: status, body: body} +} + +// accountPeopleRoute serves the account roster the person resolver reads. +func accountPeopleRoute() stubRoute { + return stubRoute{ + method: http.MethodGet, + path: "/99999/people.json", + status: http.StatusOK, + body: `[{"id":3001,"name":"Annie Bryan","email_address":"annie@springfield.example.com","client":true},{"id":1001,"name":"Zed Team","email_address":"zed@example.com"}]`, + pages: []string{`[{"id":3001,"name":"Annie Bryan","email_address":"annie@springfield.example.com","client":true},{"id":1001,"name":"Zed Team","email_address":"zed@example.com"}]`}, + } +} + +func clientUsersBody(t *testing.T, transport *recordingTransport) map[string]any { + t.Helper() + for _, call := range transport.recorded() { + if call.Method == http.MethodPut && call.Path == clientUsersPath { + var body map[string]any + require.NoError(t, json.Unmarshal([]byte(call.Body), &body)) + return body + } + } + t.Fatalf("no PUT %s recorded: %+v", clientUsersPath, transport.recorded()) + return nil +} + +func TestPeopleClientsAddGrantsResolvedIDs(t *testing.T) { + app, transport, out := setupPersonalFeedApp(t, projectsRoute(), accountPeopleRoute(), + clientUsersRoute(http.StatusOK, `{"granted":[{"id":3001,"name":"Annie Bryan","client":true}],"revoked":[]}`)) + + require.NoError(t, executeRecordingCommand(NewPeopleCmd(), app, "clients", "add", "annie@springfield.example.com", "--in", "123")) + + body := clientUsersBody(t, transport) + assert.Equal(t, []any{float64(3001)}, body["grant"]) + assert.NotContains(t, body, "revoke") + assert.NotContains(t, body, "create") + + env := bubbleUpData(t, out) + assert.Equal(t, "Added 1 client(s) to project #123", env["summary"]) +} + +func TestPeopleClientsAddReportsIDsTheServerDropped(t *testing.T) { + app, _, out := setupPersonalFeedApp(t, projectsRoute(), accountPeopleRoute(), + clientUsersRoute(http.StatusOK, `{"granted":[{"id":3001,"name":"Annie Bryan","client":true}],"revoked":[]}`)) + + require.NoError(t, executeRecordingCommand(NewPeopleCmd(), app, "clients", "add", "3001", "1001", "1001", "--in", "123")) + + var envelope struct { + Summary string `json:"summary"` + Notice string `json:"notice"` + } + require.NoError(t, json.Unmarshal(out.Bytes(), &envelope)) + assert.Equal(t, "Added 1 client(s) to project #123", envelope.Summary) + assert.Equal(t, "Not added (already on the project, or not a client user): 1001", envelope.Notice) +} + +func TestPeopleClientsRemoveRevokes(t *testing.T) { + app, transport, out := setupPersonalFeedApp(t, projectsRoute(), accountPeopleRoute(), + clientUsersRoute(http.StatusOK, `{"granted":[],"revoked":[{"id":3001,"name":"Annie Bryan","client":true}]}`)) + + require.NoError(t, executeRecordingCommand(NewPeopleCmd(), app, "clients", "remove", "3001", "--in", "123")) + + body := clientUsersBody(t, transport) + assert.Equal(t, []any{float64(3001)}, body["revoke"]) + assert.NotContains(t, body, "grant") + + env := bubbleUpData(t, out) + assert.Equal(t, "Removed 1 client(s) from project #123", env["summary"]) +} + +func TestPeopleClientsInviteSendsCreateRows(t *testing.T) { + app, transport, out := setupPersonalFeedApp(t, projectsRoute(), + clientUsersRoute(http.StatusOK, `{"granted":[{"id":4001,"name":"Annie Bryan","client":true},{"id":4002,"name":"bob@example.com","client":true}],"revoked":[]}`)) + + require.NoError(t, executeRecordingCommand(NewPeopleCmd(), app, "clients", "invite", + "Annie Bryan ", "bob@example.com", "--company", "Springfield Elementary", "--in", "123")) + + body := clientUsersBody(t, transport) + assert.NotContains(t, body, "grant") + assert.NotContains(t, body, "revoke") + assert.Equal(t, []any{ + map[string]any{"name": "Annie Bryan", "email_address": "annie@example.com", "company_name": "Springfield Elementary"}, + map[string]any{"email_address": "bob@example.com", "company_name": "Springfield Elementary"}, + }, body["create"]) + + env := bubbleUpData(t, out) + assert.Equal(t, "Invited 2 client(s) to project #123", env["summary"]) +} + +func TestPeopleClientsInviteReadsStdin(t *testing.T) { + app, transport, _ := setupPersonalFeedApp(t, projectsRoute(), + clientUsersRoute(http.StatusOK, `{"granted":[],"revoked":[]}`)) + + cmd := NewPeopleCmd() + cmd.SetIn(strings.NewReader("annie@example.com\nBob Client \n")) + require.NoError(t, executeRecordingCommand(cmd, app, "clients", "invite", "-", "--in", "123")) + + body := clientUsersBody(t, transport) + assert.Equal(t, []any{ + map[string]any{"email_address": "annie@example.com"}, + map[string]any{"name": "Bob Client", "email_address": "bob@example.com"}, + }, body["create"]) +} + +func TestPeopleClientsInviteTitleTakesOneInvitee(t *testing.T) { + app, transport, _ := setupPersonalFeedApp(t, projectsRoute()) + + err := executeRecordingCommand(NewPeopleCmd(), app, "clients", "invite", "a@example.com", "b@example.com", "--title", "Owner", "--in", "123") + requireBookmarksUsageError(t, err) + assert.Empty(t, transport.recorded()) + + app, transport, _ = setupPersonalFeedApp(t, projectsRoute(), clientUsersRoute(http.StatusOK, `{"granted":[],"revoked":[]}`)) + require.NoError(t, executeRecordingCommand(NewPeopleCmd(), app, "clients", "invite", "a@example.com", "--title", "Owner", "--in", "123")) + body := clientUsersBody(t, transport) + assert.Equal(t, []any{map[string]any{"email_address": "a@example.com", "title": "Owner"}}, body["create"]) +} + +// The wire 422 is row-keyed; the SDK folds it into FieldErrors by address, and +// the hint names each rejected row. +func TestPeopleClientsInviteNamesRejectedRows(t *testing.T) { + app, _, _ := setupPersonalFeedApp(t, projectsRoute(), + clientUsersRoute(http.StatusUnprocessableEntity, `{"errors":[{"email_address":"nope@example.com","messages":["Email address is invalid"]}]}`)) + + err := executeRecordingCommand(NewPeopleCmd(), app, "clients", "invite", "ok@example.com", "nope@example.com", "--in", "123") + + var outErr *output.Error + require.True(t, errors.As(err, &outErr), "got %T: %v", err, err) + assert.Equal(t, output.CodeValidation, outErr.Code) + assert.Equal(t, output.ExitValidation, output.ExitCodeFor(outErr.Code)) + assert.Contains(t, outErr.Message, "Invitation rejected") + assert.Contains(t, outErr.Hint, "Nobody was invited") + assert.Contains(t, outErr.Hint, "nope@example.com: Email address is invalid") + assert.NotContains(t, outErr.Hint, "ok@example.com") +} + +func TestClientInviteValidationErrorNamesEachRejectedRow(t *testing.T) { + sdkErr := &basecamp.Error{ + Code: basecamp.CodeValidation, + HTTPStatus: 422, + FieldErrors: map[string][]string{ + "nope@example.com": {"Email address is invalid"}, + "": {"Email address can't be blank"}, + }, + } + + err := clientInviteValidationError(sdkErr, []clientInvitee{{EmailAddress: "nope@example.com"}}) + + var outErr *output.Error + require.True(t, errors.As(err, &outErr)) + assert.Equal(t, output.CodeValidation, outErr.Code) + assert.Contains(t, outErr.Hint, "nope@example.com: Email address is invalid") + assert.Contains(t, outErr.Hint, ": Email address can't be blank") +} + +func TestPeopleClientsInviteSeatLimit(t *testing.T) { + app, _, _ := setupPersonalFeedApp(t, projectsRoute(), clientUsersRoute(http.StatusTooManyRequests, ``)) + + err := executeRecordingCommand(NewPeopleCmd(), app, "clients", "invite", "a@example.com", "--in", "123") + + var outErr *output.Error + require.True(t, errors.As(err, &outErr)) + assert.Equal(t, output.CodeLimitExceeded, outErr.Code) + assert.False(t, outErr.Retryable) +} + +func TestPeopleClientsAddWhenClientsAreOff(t *testing.T) { + app, _, _ := setupPersonalFeedApp(t, projectsRoute(), accountPeopleRoute(), clientsProjectRoute(false), + clientUsersRoute(http.StatusForbidden, ``)) + + err := executeRecordingCommand(NewPeopleCmd(), app, "clients", "add", "3001", "--in", "123") + + var outErr *output.Error + require.True(t, errors.As(err, &outErr)) + assert.Equal(t, output.CodeForbidden, outErr.Code) + assert.Equal(t, "Clients are not enabled on project #123", outErr.Message) + assert.Contains(t, outErr.Hint, "basecamp people clients enable --in 123") +} + +func TestPeopleClientsEnable(t *testing.T) { + app, transport, out := setupPersonalFeedApp(t, projectsRoute(), + stubRoute{method: http.MethodPost, path: clientEnablementPath, status: http.StatusOK, body: `{"clients_enabled":true}`}) + + require.NoError(t, executeRecordingCommand(NewPeopleCmd(), app, "clients", "enable", "--in", "123")) + + assert.Equal(t, http.MethodPost, transport.last(t).Method) + assert.Equal(t, clientEnablementPath, transport.last(t).Path) + env := bubbleUpData(t, out) + assert.Equal(t, true, env["data"].(map[string]any)["clients_enabled"]) + assert.Equal(t, "Enabled clients on project #123", env["summary"]) +} + +func TestPeopleClientsDisable(t *testing.T) { + app, transport, out := setupPersonalFeedApp(t, projectsRoute(), + stubRoute{method: http.MethodDelete, path: clientEnablementPath, status: http.StatusOK, body: `{"clients_enabled":false}`}) + + require.NoError(t, executeRecordingCommand(NewPeopleCmd(), app, "clients", "disable", "--in", "123")) + + assert.Equal(t, http.MethodDelete, transport.last(t).Method) + env := bubbleUpData(t, out) + assert.Equal(t, false, env["data"].(map[string]any)["clients_enabled"]) +} + +func TestPeopleClientsDisableNamesRemainingClients(t *testing.T) { + app, _, _ := setupPersonalFeedApp(t, projectsRoute(), clientsProjectPeopleRoute(), + stubRoute{method: http.MethodDelete, path: clientEnablementPath, status: http.StatusForbidden, body: ``}) + + err := executeRecordingCommand(NewPeopleCmd(), app, "clients", "disable", "--in", "123") + + var outErr *output.Error + require.True(t, errors.As(err, &outErr)) + assert.Equal(t, output.CodeForbidden, outErr.Code) + assert.Contains(t, outErr.Message, "2 client(s) still have access") + assert.Contains(t, outErr.Hint, "basecamp people clients remove 3002 3001 --in 123") +} + +// With no client on the roster the 403 is not evidence that clients remain, +// so the message must not claim it. +func TestPeopleClientsDisableWithoutClientsIsAPlainDenial(t *testing.T) { + roster := stubRoute{method: http.MethodGet, path: clientsProjectPeoplePath, status: http.StatusOK, + body: `[{"id":1001,"name":"Zed Team","employee":true}]`, pages: []string{`[{"id":1001,"name":"Zed Team","employee":true}]`}} + app, _, _ := setupPersonalFeedApp(t, projectsRoute(), roster, + stubRoute{method: http.MethodDelete, path: clientEnablementPath, status: http.StatusForbidden, body: ``}) + + err := executeRecordingCommand(NewPeopleCmd(), app, "clients", "disable", "--in", "123") + + var outErr *output.Error + require.True(t, errors.As(err, &outErr)) + assert.Equal(t, output.CodeForbidden, outErr.Code) + assert.NotContains(t, outErr.Message, "still have access") + assert.Contains(t, outErr.Hint, "permission") +} + +// A stray positional must not be discarded: with a default project configured, +// "disable 123" would otherwise act on the configured project. +func TestPeopleClientsProjectOnlyVerbsRejectPositionals(t *testing.T) { + for _, verb := range []string{"list", "enable", "disable"} { + app, transport, _ := setupPersonalFeedApp(t, projectsRoute()) + app.Config.ProjectID = "123" + + err := executeRecordingCommand(NewPeopleCmd(), app, "clients", verb, "456") + require.Error(t, err, verb) + assert.Empty(t, transport.recorded(), "%s must not reach the API", verb) + } +} diff --git a/internal/mcpserver/catalog_test.go b/internal/mcpserver/catalog_test.go index 4f86f4df..64134727 100644 --- a/internal/mcpserver/catalog_test.go +++ b/internal/mcpserver/catalog_test.go @@ -61,7 +61,7 @@ func TestCatalogExcludesBinaryUploads(t *testing.T) { assert.False(t, excluded[op.ID], "operation %q should be excluded from the vendored model", op.ID) } } - assert.Equal(t, 256, total, "served operation count") + assert.Equal(t, 259, total, "served operation count") } // TestCatalogIsAccountScoped pins the rescope: the CLI's account-scoped SDK diff --git a/internal/mcpserver/model/PROVENANCE.json b/internal/mcpserver/model/PROVENANCE.json index 716d1bd1..30764f81 100644 --- a/internal/mcpserver/model/PROVENANCE.json +++ b/internal/mcpserver/model/PROVENANCE.json @@ -1,7 +1,7 @@ { "source": "github.com/basecamp/basecamp-sdk", - "commit": "47e7ca381a4935ae5c8197adaa26606dc6c53074", - "ref": "v0.16.1-0.20260903193203-47e7ca381a49", + "commit": "6e73a06f4262062b2c973b2d0787ab0029b08d0d", + "ref": "go/v0.17.0", "files": ["behavior-model.json", "openapi.json"], "synced_by": "scripts/sync-mcp-model.sh", "patches": "tags assigned to operations the export leaves untagged (PATCHED_TAGS); binary-upload operations dropped (EXCLUDED_OPERATIONS) — see the sync script" diff --git a/internal/mcpserver/model/behavior-model.json b/internal/mcpserver/model/behavior-model.json index 2783db9f..b74aca9f 100644 --- a/internal/mcpserver/model/behavior-model.json +++ b/internal/mcpserver/model/behavior-model.json @@ -641,6 +641,18 @@ ] } }, + "DisableProjectClients": { + "idempotent": true, + "retry": { + "max": 3, + "base_delay_ms": 1000, + "backoff": "exponential", + "retry_on": [ + 429, + 503 + ] + } + }, "DisableTool": { "idempotent": true, "retry": { @@ -676,6 +688,18 @@ ] } }, + "EnableProjectClients": { + "idempotent": true, + "retry": { + "max": 3, + "base_delay_ms": 1000, + "backoff": "exponential", + "retry_on": [ + 429, + 503 + ] + } + }, "EnableTool": { "retry": { "max": 2, @@ -3159,6 +3183,17 @@ ] } }, + "UpdateProjectClientAccess": { + "idempotent": true, + "retry": { + "max": 3, + "base_delay_ms": 1000, + "backoff": "exponential", + "retry_on": [ + 503 + ] + } + }, "UpdateQuestion": { "idempotent": true, "retry": { @@ -3309,6 +3344,12 @@ } }, "redaction": { + "CreateClientRequest": [ + "$.email_address", + "$.name", + "$.title", + "$.company_name" + ], "CreatePersonRequest": [ "$.name", "$.email_address", diff --git a/internal/mcpserver/model/openapi.json b/internal/mcpserver/model/openapi.json index 51bffd70..6714b588 100644 --- a/internal/mcpserver/model/openapi.json +++ b/internal/mcpserver/model/openapi.json @@ -13635,6 +13635,214 @@ } } }, + "/{accountId}/projects/{projectId}/client_enablement.json": { + "delete": { + "description": "Disable clients on a project\n\n403 while the project still has any client users \u2014 revoke them first with\nUpdateProjectClientAccess. Naturally idempotent: disabling a project with\nclients already off re-answers `{\"clients_enabled\": false}`.", + "operationId": "DisableProjectClients", + "parameters": [ + { + "name": "accountId", + "in": "path", + "description": "Basecamp account ID (numeric string)", + "schema": { + "type": "string", + "pattern": "^[0-9]+$", + "description": "Basecamp account ID (numeric string)" + }, + "required": true + }, + { + "name": "projectId", + "in": "path", + "schema": { + "type": "integer", + "format": "int64" + }, + "required": true + } + ], + "responses": { + "200": { + "description": "DisableProjectClients 200 response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/DisableProjectClientsResponseContent" + } + } + } + }, + "401": { + "description": "UnauthorizedError 401 response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/UnauthorizedErrorResponseContent" + } + } + } + }, + "403": { + "description": "ForbiddenError 403 response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ForbiddenErrorResponseContent" + } + } + } + }, + "404": { + "description": "NotFoundError 404 response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/NotFoundErrorResponseContent" + } + } + } + }, + "429": { + "description": "RateLimitError 429 response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RateLimitErrorResponseContent" + } + } + } + }, + "500": { + "description": "InternalServerError 500 response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/InternalServerErrorResponseContent" + } + } + } + } + }, + "tags": [ + "People" + ], + "x-basecamp-idempotent": { + "natural": true + }, + "x-basecamp-retry": { + "maxAttempts": 3, + "baseDelayMs": 1000, + "backoff": "exponential", + "retryOn": [ + 429, + 503 + ] + } + }, + "post": { + "description": "Enable clients on a project so client users can be added to it\n\nA deliberate step separate from adding clients: it turns on the project's\nclient-facing surface and applies the default client visibility (the\ntimeline and most docked tools become client-visible; the card table,\nCampfire, and Doors stay private). UpdateProjectClientAccess never enables\nclients implicitly \u2014 enable first, then add. 403 unless the project can have\nclients (the account supports clients and the project is a standard\nproject). Naturally idempotent: enabling an enabled project re-answers\n`{\"clients_enabled\": true}`.", + "operationId": "EnableProjectClients", + "parameters": [ + { + "name": "accountId", + "in": "path", + "description": "Basecamp account ID (numeric string)", + "schema": { + "type": "string", + "pattern": "^[0-9]+$", + "description": "Basecamp account ID (numeric string)" + }, + "required": true + }, + { + "name": "projectId", + "in": "path", + "schema": { + "type": "integer", + "format": "int64" + }, + "required": true + } + ], + "responses": { + "200": { + "description": "EnableProjectClients 200 response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/EnableProjectClientsResponseContent" + } + } + } + }, + "401": { + "description": "UnauthorizedError 401 response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/UnauthorizedErrorResponseContent" + } + } + } + }, + "403": { + "description": "ForbiddenError 403 response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ForbiddenErrorResponseContent" + } + } + } + }, + "404": { + "description": "NotFoundError 404 response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/NotFoundErrorResponseContent" + } + } + } + }, + "429": { + "description": "RateLimitError 429 response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RateLimitErrorResponseContent" + } + } + } + }, + "500": { + "description": "InternalServerError 500 response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/InternalServerErrorResponseContent" + } + } + } + } + }, + "tags": [ + "People" + ], + "x-basecamp-idempotent": { + "natural": true + }, + "x-basecamp-retry": { + "maxAttempts": 3, + "baseDelayMs": 1000, + "backoff": "exponential", + "retryOn": [ + 429, + 503 + ] + } + } + }, "/{accountId}/projects/{projectId}/gauge.json": { "put": { "description": "Enable or disable the gauge for a project. Only project admins can toggle gauges.", @@ -14067,6 +14275,207 @@ } } }, + "/{accountId}/projects/{projectId}/people/client_users.json": { + "put": { + "description": "Update project client access (grant/revoke/create client users)\n\nThe client-side counterpart to UpdateProjectAccess: `grant` adds existing\nclient users by id, `revoke` removes client users, and `create` invites\nbrand-new clients by email (`name` optional, defaulting to the address).\nOnly client users are eligible \u2014 a `grant` id belonging to a team member is\nrejected (omitted from `granted`) rather than cross-graded, and `revoke` never removes a team\nmember. The response mirrors UpdateProjectAccess: `granted` and `revoked`\npeople, each with `client: true`.\n\nRequires clients to be enabled on the project (EnableProjectClients);\notherwise 403. Invitations are all-or-nothing: an invalid `create` row\n(including one with no email address) answers 422 with the rejected\naddresses and nobody is invited; new addresses that would exceed the\naccount's user limit answer 429 and nobody is invited. Addresses already on\nthe account take no seat and a repeated address counts once.\n\nThe seat-limit 429 is a verdict, not throttling: it carries no Retry-After\nand re-asking cannot change the answer, so this operation declares\n`retryOn: [503]` and a 429 surfaces on the first attempt (status-mapped to\n`rate_limit`, since the wire status is the only signal). Every other\noperation retries on 429.", + "operationId": "UpdateProjectClientAccess", + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/UpdateProjectClientAccessRequestContent" + }, + "examples": { + "UpdateProjectClientAccess_example1": { + "summary": "Grant access to existing clients", + "description": "Use grant array with client person IDs; team-member IDs are omitted from granted", + "value": { + "grant": [ + 111 + ] + } + }, + "UpdateProjectClientAccess_example2": { + "summary": "Invite new clients by email", + "description": "Use create array; name is optional and defaults to the address", + "value": { + "create": [ + { + "email_address": "annie@example.com", + "company_name": "Springfield Elementary" + } + ] + } + } + } + } + } + }, + "parameters": [ + { + "name": "accountId", + "in": "path", + "description": "Basecamp account ID (numeric string)", + "schema": { + "type": "string", + "pattern": "^[0-9]+$", + "description": "Basecamp account ID (numeric string)" + }, + "required": true, + "examples": { + "UpdateProjectClientAccess_example1": { + "summary": "Grant access to existing clients", + "description": "Use grant array with client person IDs; team-member IDs are omitted from granted", + "value": "999" + }, + "UpdateProjectClientAccess_example2": { + "summary": "Invite new clients by email", + "description": "Use create array; name is optional and defaults to the address", + "value": "999" + } + } + }, + { + "name": "projectId", + "in": "path", + "schema": { + "type": "integer", + "format": "int64" + }, + "required": true, + "examples": { + "UpdateProjectClientAccess_example1": { + "summary": "Grant access to existing clients", + "description": "Use grant array with client person IDs; team-member IDs are omitted from granted", + "value": 12345678 + }, + "UpdateProjectClientAccess_example2": { + "summary": "Invite new clients by email", + "description": "Use create array; name is optional and defaults to the address", + "value": 12345678 + } + } + } + ], + "responses": { + "200": { + "description": "UpdateProjectClientAccess 200 response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/UpdateProjectClientAccessResponseContent" + }, + "examples": { + "UpdateProjectClientAccess_example1": { + "summary": "Grant access to existing clients", + "description": "Use grant array with client person IDs; team-member IDs are omitted from granted", + "value": { + "granted": [ + { + "id": 111, + "name": "Annie Bryan", + "client": true + } + ], + "revoked": [] + } + }, + "UpdateProjectClientAccess_example2": { + "summary": "Invite new clients by email", + "description": "Use create array; name is optional and defaults to the address", + "value": { + "granted": [ + { + "id": 444, + "name": "annie@example.com", + "email_address": "annie@example.com", + "client": true + } + ], + "revoked": [] + } + } + } + } + } + }, + "401": { + "description": "UnauthorizedError 401 response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/UnauthorizedErrorResponseContent" + } + } + } + }, + "403": { + "description": "ForbiddenError 403 response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ForbiddenErrorResponseContent" + } + } + } + }, + "404": { + "description": "NotFoundError 404 response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/NotFoundErrorResponseContent" + } + } + } + }, + "422": { + "description": "ClientInvitationValidationError 422 response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ClientInvitationValidationErrorResponseContent" + } + } + } + }, + "429": { + "description": "RateLimitError 429 response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RateLimitErrorResponseContent" + } + } + } + }, + "500": { + "description": "InternalServerError 500 response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/InternalServerErrorResponseContent" + } + } + } + } + }, + "tags": [ + "People" + ], + "x-basecamp-idempotent": { + "natural": true + }, + "x-basecamp-retry": { + "maxAttempts": 3, + "baseDelayMs": 1000, + "backoff": "exponential", + "retryOn": [ + 503 + ] + } + } + }, "/{accountId}/projects/{projectId}/people/users.json": { "put": { "description": "Update project access (grant/revoke/create people)", @@ -28630,6 +29039,47 @@ "visible_to_clients" ] }, + "ClientInvitationError": { + "type": "object", + "description": "One rejected `create` row: the address as submitted and the validation\nmessages for it. Always emitted; `null` when the row carried no address (the\nrow is still rejected, with a \"can't be blank\" message). `@required` models\nthe presence and the nullability is layered on in the OpenAPI\n(smithy-build.json jsonAdd -> type: [\"string\",\"null\"]), the Wormhole.color\ntreatment.", + "properties": { + "email_address": { + "type": [ + "string", + "null" + ], + "x-go-type": "string" + }, + "messages": { + "type": "array", + "items": { + "type": "string" + } + } + }, + "required": [ + "email_address", + "messages" + ] + }, + "ClientInvitationErrors": { + "type": "object", + "description": "The per-row 422 body: {\"errors\": [{\"email_address\": ..., \"messages\": [...]}]}.", + "properties": { + "errors": { + "type": "array", + "items": { + "$ref": "#/components/schemas/ClientInvitationError" + } + } + }, + "required": [ + "errors" + ] + }, + "ClientInvitationValidationErrorResponseContent": { + "$ref": "#/components/schemas/ClientInvitationErrors" + }, "ClientReply": { "type": "object", "properties": { @@ -29097,6 +29547,31 @@ "CreateChatbotResponseContent": { "$ref": "#/components/schemas/Chatbot" }, + "CreateClientRequest": { + "type": "object", + "description": "A new client to invite. Unlike CreatePersonRequest, only the address is\nrequired: bc3 defaults `name` to the email address when omitted.", + "properties": { + "email_address": { + "type": "string", + "format": "password" + }, + "name": { + "type": "string", + "format": "password" + }, + "title": { + "type": "string", + "format": "password" + }, + "company_name": { + "type": "string", + "format": "password" + } + }, + "required": [ + "email_address" + ] + }, "CreateCloudFileRequestContent": { "type": "object", "properties": { @@ -29841,6 +30316,9 @@ "DisableCardColumnOnHoldResponseContent": { "$ref": "#/components/schemas/CardColumn" }, + "DisableProjectClientsResponseContent": { + "$ref": "#/components/schemas/ProjectClientEnablement" + }, "DockItem": { "type": "object", "properties": { @@ -30135,6 +30613,9 @@ "EnableOutOfOfficeResponseContent": { "$ref": "#/components/schemas/OutOfOffice" }, + "EnableProjectClientsResponseContent": { + "$ref": "#/components/schemas/ProjectClientEnablement" + }, "Event": { "type": "object", "properties": { @@ -33078,6 +33559,18 @@ } } }, + "ProjectClientEnablement": { + "type": "object", + "description": "The project's client-enablement state after a toggle.", + "properties": { + "clients_enabled": { + "type": "boolean" + } + }, + "required": [ + "clients_enabled" + ] + }, "ProjectConstruction": { "type": "object", "properties": { @@ -36462,6 +36955,37 @@ "UpdateProjectAccessResponseContent": { "$ref": "#/components/schemas/ProjectAccessResult" }, + "UpdateProjectClientAccessRequestContent": { + "type": "object", + "properties": { + "grant": { + "type": "array", + "items": { + "type": "integer", + "format": "int64" + }, + "description": "Existing client people IDs to add to the project." + }, + "revoke": { + "type": "array", + "items": { + "type": "integer", + "format": "int64" + }, + "description": "Client people IDs to remove from the project." + }, + "create": { + "type": "array", + "items": { + "$ref": "#/components/schemas/CreateClientRequest" + }, + "description": "New clients to invite by email." + } + } + }, + "UpdateProjectClientAccessResponseContent": { + "$ref": "#/components/schemas/ProjectAccessResult" + }, "UpdateProjectRequestContent": { "type": "object", "properties": { diff --git a/internal/mcpserver/testdata/catalog_snapshot.txt b/internal/mcpserver/testdata/catalog_snapshot.txt index fb438aef..bd995d80 100644 --- a/internal/mcpserver/testdata/catalog_snapshot.txt +++ b/internal/mcpserver/testdata/catalog_snapshot.txt @@ -197,7 +197,9 @@ Call {"action": "describe", "params": {"action": "NAME"}} for an action's full p ACTIONS (RO = read-only): - disable_out_of_office: Disable out of office for a person +- disable_project_clients: Disable clients on a project - enable_out_of_office: Enable or replace out of office for a person +- enable_project_clients: Enable clients on a project so client users can be added to it - get_my_preferences (RO): Get the current user's preferences - get_my_profile (RO): Get the current authenticated user's profile - get_out_of_office (RO): Get the out of office status for a person @@ -211,6 +213,7 @@ ACTIONS (RO = read-only): - update_my_preferences: Update the current user's preferences - update_my_profile: Update the current authenticated user's profile (returns 204 No Content) - update_project_access: Update project access (grant/revoke/create people) +- update_project_client_access: Update project client access (grant/revoke/create client users) - update_subscription: Update subscriptions by adding or removing specific users == basecamp_automation diff --git a/internal/resilience/hooks.go b/internal/resilience/hooks.go index 9669aac4..6f9c4504 100644 --- a/internal/resilience/hooks.go +++ b/internal/resilience/hooks.go @@ -2,9 +2,11 @@ package resilience import ( "context" + "slices" "time" "github.com/basecamp/basecamp-sdk/go/pkg/basecamp" + "github.com/basecamp/basecamp-sdk/go/pkg/generated" ) // Verify GatingHooks implements basecamp.GatingHooks at compile time. @@ -13,6 +15,10 @@ var _ basecamp.GatingHooks = (*GatingHooks)(nil) // releaseKey is the context key for the bulkhead release function. type releaseKey struct{} +// operationKey carries the operation name from OnOperationStart to the +// request hooks, which only see method and URL. +type operationKey struct{} + // GatingHooks implements basecamp.GatingHooks to provide resilience patterns // for SDK operations. It gates requests through circuit breaker, rate limiter, // and bulkhead before they execute. @@ -90,10 +96,11 @@ func (h *GatingHooks) OnOperationGate(ctx context.Context, op basecamp.Operation return ctx, nil } -// OnOperationStart is called when a semantic SDK operation begins. +// OnOperationStart is called when a semantic SDK operation begins. Gating +// already happened in OnOperationGate; this only names the operation for the +// request hooks, which otherwise see method and URL alone. func (h *GatingHooks) OnOperationStart(ctx context.Context, op basecamp.OperationInfo) context.Context { - // No additional setup needed; gating already happened in OnOperationGate - return ctx + return context.WithValue(ctx, operationKey{}, op.Operation) } // OnOperationEnd is called when a semantic SDK operation completes. @@ -136,13 +143,29 @@ func (h *GatingHooks) OnRequestEnd(ctx context.Context, info basecamp.RequestInf // Honor Retry-After header from rate-limited or overloaded responses if result.RetryAfter > 0 { _ = h.rateLimiter.SetRetryAfterDuration(time.Duration(result.RetryAfter) * time.Second) //nolint:contextcheck // lock acquisition is context-independent by design - } else if result.StatusCode == 429 { + } else if result.StatusCode == 429 && !isVerdict429(ctx) { // Default to 60 seconds if no Retry-After specified (SDK parity for 429 only) // Note: 503 requires explicit Retry-After header per SDK behavior _ = h.rateLimiter.SetRetryAfterDuration(60 * time.Second) //nolint:contextcheck // lock acquisition is context-independent by design } } +// isVerdict429 reports whether a headerless 429 on the operation in ctx is an +// answer rather than throttling. The SDK's behavior model declares which +// statuses each operation retries on, and an operation whose set excludes 429 +// (UpdateProjectClientAccess: its 429 is the account seat-limit verdict) is +// one the server answers 429 deterministically. Blocking every later command +// for a minute on such an answer would gate unrelated work on a fact about +// one request's input. Operations the model does not name keep the default. +func isVerdict429(ctx context.Context) bool { + operation, _ := ctx.Value(operationKey{}).(string) + if operation == "" { + return false + } + retryOn, ok := generated.GetOperationRetryOn(operation) + return ok && !slices.Contains(retryOn, 429) +} + // OnRetry is called before a retry attempt. func (h *GatingHooks) OnRetry(ctx context.Context, info basecamp.RequestInfo, attempt int, err error) { // Nothing to do; the SDK handles retries automatically diff --git a/internal/resilience/hooks_test.go b/internal/resilience/hooks_test.go index e234224a..5255c1f1 100644 --- a/internal/resilience/hooks_test.go +++ b/internal/resilience/hooks_test.go @@ -262,6 +262,35 @@ func TestGatingHooksOnOperationStartAndRequestMethods(t *testing.T) { hooks.OnRetry(ctx, basecamp.RequestInfo{}, 1, nil) } +// A headerless 429 blocks later operations for a minute, except when the +// operation's declared retry set excludes 429: that 429 is the server's answer +// (the client seat limit), not throttling, and must not gate unrelated work. +func TestGatingHooksHeaderless429BlocksUnlessTheOperationTreatsItAsAVerdict(t *testing.T) { + block := func(operation string) time.Duration { + hooks := NewGatingHooksFromConfig(NewStore(t.TempDir()), DefaultConfig()) + ctx := hooks.OnOperationStart(context.Background(), basecamp.OperationInfo{Service: "People", Operation: operation}) + hooks.OnRequestEnd(ctx, basecamp.RequestInfo{Method: "PUT"}, basecamp.RequestResult{StatusCode: 429}) + remaining, err := hooks.rateLimiter.RetryAfterRemaining() + require.NoError(t, err) + return remaining + } + + assert.Greater(t, block("UpdateProjectAccess"), 50*time.Second, "throttling 429 keeps the default block") + assert.Greater(t, block(""), 50*time.Second, "an unnamed operation keeps the default block") + assert.Zero(t, block("UpdateProjectClientAccess"), "a seat-limit verdict sets no block") +} + +// A 429 that names a Retry-After is honored regardless of the operation. +func TestGatingHooksRetryAfterHeaderIsHonoredOnAVerdictOperation(t *testing.T) { + hooks := NewGatingHooksFromConfig(NewStore(t.TempDir()), DefaultConfig()) + ctx := hooks.OnOperationStart(context.Background(), basecamp.OperationInfo{Service: "People", Operation: "UpdateProjectClientAccess"}) + hooks.OnRequestEnd(ctx, basecamp.RequestInfo{Method: "PUT"}, basecamp.RequestResult{StatusCode: 429, RetryAfter: 7}) + + remaining, err := hooks.rateLimiter.RetryAfterRemaining() + require.NoError(t, err) + assert.Greater(t, remaining, 5*time.Second) +} + func TestGatingHooksResetsStaleHalfOpenAttemptsIntegration(t *testing.T) { // This integration test verifies stale cleanup works even when the rate // limiter updates State.UpdatedAt before the circuit breaker runs. diff --git a/internal/version/sdk-provenance.json b/internal/version/sdk-provenance.json index 20e1b7c6..f5287d19 100644 --- a/internal/version/sdk-provenance.json +++ b/internal/version/sdk-provenance.json @@ -1,9 +1,9 @@ { "sdk": { "module": "github.com/basecamp/basecamp-sdk/go", - "version": "v0.16.1-0.20260903193203-47e7ca381a49", - "revision": "47e7ca381a49", - "updated_at": "2026-09-03T19:32:03Z" + "version": "v0.17.0", + "revision": "6e73a06f4262", + "updated_at": "2026-09-09T22:00:24Z" }, "api": { "repo": "basecamp/bc3", diff --git a/nix/package.nix b/nix/package.nix index d0295541..4b847313 100644 --- a/nix/package.nix +++ b/nix/package.nix @@ -8,7 +8,7 @@ buildGoModule.override { go = go_1_26; } (finalAttrs: { src = lib.cleanSource ./..; # To update: set to lib.fakeHash, run `nix build`, use the hash from the error. - vendorHash = "sha256-1/J2ORc4ZUivW23PyuQ6cP3QlHFRHZYZG+nVihrand4="; + vendorHash = "sha256-GCw7WkPmXDWDAhJrVqbXUfwLa9YzpWdTcSimwovjpcQ="; subPackages = [ "cmd/basecamp" ]; diff --git a/skills/basecamp/SKILL.md b/skills/basecamp/SKILL.md index 06144880..29886ae9 100644 --- a/skills/basecamp/SKILL.md +++ b/skills/basecamp/SKILL.md @@ -1240,8 +1240,8 @@ basecamp people update me --bio "..." --title "..." --json # Edit your own pro basecamp people out-of-office me --json # Your out-of-office status basecamp people out-of-office me --start 2026-09-14 --end 2026-09-18 --json # Set out-of-office basecamp people out-of-office me --clear --json # Clear out-of-office -basecamp people add --project # Add to project -basecamp people remove --project # Remove from project +basecamp people add --project # Add a team member to a project +basecamp people remove --project # Remove a team member from a project ``` `people update me` edits your own profile (bio, title, name, email, location, @@ -1250,6 +1250,32 @@ out-of-office me` shows your away status, sets it with `--start`/`--end` (natural language or YYYY-MM-DD, end not before start), or clears it with `--clear`. +`people list` reports each person's `client` flag. `people add`/`remove` manage +team members only — a client's id passed to them is dropped server-side, never +cross-graded — so clients have their own verbs: + +```bash +basecamp people clients enable --in # Turn client access on (do this first) +basecamp people clients list --in # Clients on the project +basecamp people clients add --in # Grant an existing client user +basecamp people clients invite annie@example.com --in # Invite a new client by email +basecamp people clients invite "Annie Bryan " --in # ... with a name +basecamp people clients invite - --in # One invitee per line on stdin +basecamp people clients remove --in # Revoke a client's access +basecamp people clients disable --in # Turn client access off (after removing every client) +``` + +Enabling clients is a deliberate, separate step: it applies the project's +default client visibility (timeline and most tools shared; card table, +Campfire, and Doors private), so `add`/`invite` never enable implicitly and +answer `forbidden` with an `enable` hint while clients are off. `invite` takes +`--company` (applies to every invitee) and `--title` (one invitee only), and is +all-or-nothing: a token that is not an address is refused locally as `usage` +(2) naming each one, an address the server rejects exits `validation` (9) +naming each rejected row, and a seat shortfall exits `limit_exceeded` (10) — in +every case nobody was invited. `add`/`remove` report the ids the server did not grant or revoke +(already on the project, or not a client user) in the notice. + ### Search ```bash