From 3e977a8e7de7e4e290c2b064c35b7b1e21fdaa93 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Tue, 8 Sep 2026 17:28:52 -0700 Subject: [PATCH 1/6] Pin basecamp-sdk to the project-client-users branch The client-admission operations (UpdateProjectClientAccess, EnableProjectClients, DisableProjectClients) live on basecamp-sdk's feat/project-client-users branch (basecamp/basecamp-sdk#847) ahead of a release, so go.mod pins its head as a pseudo-version, the way the bubble-up pin did. Re-pin to the tagged go/vX.Y.Z release once it is cut, and re-sync the vendored MCP model and the Nix vendorHash from it. The vendored MCP model is synced from that head: three People operations join the catalog (served count 256 -> 259) and the seat-limit 429 on UpdateProjectClientAccess is declared non-retryable (retry_on [503]). --- internal/mcpserver/catalog_test.go | 2 +- internal/mcpserver/model/behavior-model.json | 41 ++ internal/mcpserver/model/openapi.json | 524 ++++++++++++++++++ .../mcpserver/testdata/catalog_snapshot.txt | 3 + 4 files changed, 569 insertions(+), 1 deletion(-) 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/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 From 076e783ab8156f96c11f62402459559aa96d2e2a Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Tue, 8 Sep 2026 17:28:52 -0700 Subject: [PATCH 2/6] Add people clients: list, add, remove, invite, enable, disable The client-side counterpart of people add/remove, over the endpoints bc3#13098 shipped: PUT /projects/:id/people/client_users.json and POST/DELETE /projects/:id/client_enablement.json. A separate group rather than a --client flag: bc3 keeps the two kinds of access apart on the wire (the team endpoint drops a client's id, the client endpoint rejects a team member's), so the CLI mirrors the split. The ids the server silently drops come back as a diagnostic notice. invite takes a bare address or "Name " (net/mail), or "-" for one invitee per line on stdin; --company applies to every invitee, --title to exactly one. Refusals map to verdicts: a 403 reads the project back to name "clients not enabled" against a permission problem; the all-or-nothing 422 exits validation naming each rejected row from the SDK's field errors; the seat-limit 429 exits limit_exceeded, not a retryable rate limit. disable's 403 reads the roster back so the hint is the exact remove command. people list now carries each person's client flag. --- .surface | 166 +++++ API-COVERAGE.md | 11 +- e2e/people.bats | 80 +++ e2e/smoke/smoke_lifecycle.bats | 20 + e2e/smoke/smoke_projects.bats | 12 + internal/commands/commands.go | 2 +- internal/commands/people.go | 733 ++++++++++++++++++++++- internal/commands/people_clients_test.go | 398 ++++++++++++ skills/basecamp/SKILL.md | 29 +- 9 files changed, 1443 insertions(+), 8 deletions(-) create mode 100644 e2e/people.bats create mode 100644 internal/commands/people_clients_test.go 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/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..ec805311 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,728 @@ 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 `, + 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. +func parseClientInvitees(tokens []string) ([]clientInvitee, error) { + invitees := make([]clientInvitee, 0, len(tokens)) + for _, token := range tokens { + addr, err := mail.ParseAddress(strings.TrimSpace(token)) + if err != nil { + return nil, output.ErrUsageHint( + fmt.Sprintf(`%q is not an email address or "Name "`, token), + `Name each client by email address, or as "Full Name " to set the name`) + } + invitees = append(invitees, clientInvitee{Name: addr.Name, EmailAddress: addr.Address}) + } + 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. +func clientSeatLimitError(err error, invitees []clientInvitee) 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: fmt.Sprintf("Not enough seats on the account to invite %d new client(s)", len(invitees)), + 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 +} + +func joinInt64s(ids []int64) string { + parts := make([]string, len(ids)) + for i, id := range ids { + parts[i] = strconv.FormatInt(id, 10) + } + return strings.Join(parts, ", ") +} + +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. + +Invitations are all-or-nothing: an invalid address fails the whole batch +with each bad 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, invitees) + 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 `, + 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 `, + 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, and the roster is read back once +// to name them. +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, + } + } + hint := "Remove every client first: basecamp people clients list --in " + projectRef + 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 { + hint = fmt.Sprintf("Remove every client first: basecamp people clients remove %s --in %s", joinInt64s(ids), projectRef) + } + } + return &output.Error{ + Code: output.CodeForbidden, + Message: fmt.Sprintf("Clients cannot be disabled on project #%s while any client still has access", projectRef), + Hint: hint, + 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..6e2d2666 --- /dev/null +++ b/internal/commands/people_clients_test.go @@ -0,0 +1,398 @@ +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) + } +} + +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) { + invitees := []clientInvitee{{EmailAddress: "a@example.com"}, {EmailAddress: "b@example.com"}} + + err := clientSeatLimitError(basecamp.ErrRateLimit(0), invitees) + + 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, "2 new client(s)") +} + +func TestClientSeatLimitErrorKeepsRealThrottling(t *testing.T) { + err := clientSeatLimitError(basecamp.ErrRateLimit(30), nil) + + 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.Hint, "basecamp people clients remove 3002, 3001 --in 123") +} diff --git a/skills/basecamp/SKILL.md b/skills/basecamp/SKILL.md index 06144880..931d6677 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,31 @@ 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: an invalid address exits `validation` (9) naming each rejected +row, and a seat shortfall exits `limit_exceeded` (10) — in both cases 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 From 11a6efb1e7864754a6a0135cf7f22b2e78ea2dbc Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Tue, 8 Sep 2026 17:39:04 -0700 Subject: [PATCH 3/6] Keep a seat-limit 429 from gating later commands; answer disable's 403 honestly The resilience hooks persisted a 60-second block for every headerless 429, so the client seat-limit verdict would have refused unrelated commands for a minute. OnOperationStart now names the operation in the context and OnRequestEnd consults the SDK's declared retry set: an operation that does not retry on 429 (UpdateProjectClientAccess) is one the server answers 429 as a verdict, and it sets no block. A 429 that names a Retry-After is honored as before. disable's 403 names the remaining clients only when the roster shows some; otherwise it is a plain denial, since the same 403 answers a caller without permission. The seat-limit message no longer states a count: existing addresses take no seat and repeats count once, and only the server knows which rows those are. --- internal/commands/people.go | 28 +++++++++++++-------- internal/commands/people_clients_test.go | 26 ++++++++++++++++---- internal/resilience/hooks.go | 31 +++++++++++++++++++++--- internal/resilience/hooks_test.go | 29 ++++++++++++++++++++++ 4 files changed, 95 insertions(+), 19 deletions(-) diff --git a/internal/commands/people.go b/internal/commands/people.go index ec805311..c7113126 100644 --- a/internal/commands/people.go +++ b/internal/commands/people.go @@ -1097,15 +1097,17 @@ func clientsForbiddenError(ctx context.Context, app *appctx.App, bucketID int64, // 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. -func clientSeatLimitError(err error, invitees []clientInvitee) error { +// 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: fmt.Sprintf("Not enough seats on the account to invite %d new client(s)", len(invitees)), + 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, @@ -1369,7 +1371,7 @@ func clientInviteError(ctx context.Context, app *appctx.App, bucketID int64, pro case basecamp.CodeForbidden: return clientsForbiddenError(ctx, app, bucketID, projectRef, err) case basecamp.CodeRateLimit: - return clientSeatLimitError(err, invitees) + return clientSeatLimitError(err) case basecamp.CodeValidation: return clientInviteValidationError(sdkErr, invitees) } @@ -1507,8 +1509,9 @@ func runPeopleClientsEnablement(cmd *cobra.Command, projectID string, enable boo // 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, and the roster is read back once -// to name them. +// 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 { @@ -1523,7 +1526,6 @@ func clientEnablementError(ctx context.Context, app *appctx.App, bucketID int64, Cause: sdkErr, } } - hint := "Remove every client first: basecamp people clients list --in " + projectRef if roster, listErr := app.Account().People().ListProjectPeople(ctx, bucketID, &basecamp.PeopleListOptions{}); listErr == nil { var ids []int64 for _, p := range roster.People { @@ -1532,13 +1534,19 @@ func clientEnablementError(ctx context.Context, app *appctx.App, bucketID int64, } } if len(ids) != 0 { - hint = fmt.Sprintf("Remove every client first: basecamp people clients remove %s --in %s", joinInt64s(ids), projectRef) + 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 while any client still has access", projectRef), - Hint: hint, + 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 index 6e2d2666..4af576ba 100644 --- a/internal/commands/people_clients_test.go +++ b/internal/commands/people_clients_test.go @@ -155,20 +155,18 @@ func TestClientsForbiddenErrorPassesOtherErrorsThrough(t *testing.T) { } func TestClientSeatLimitErrorRemapsBare429(t *testing.T) { - invitees := []clientInvitee{{EmailAddress: "a@example.com"}, {EmailAddress: "b@example.com"}} - - err := clientSeatLimitError(basecamp.ErrRateLimit(0), invitees) + 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, "2 new client(s)") + assert.Contains(t, outErr.Message, "Not enough seats") } func TestClientSeatLimitErrorKeepsRealThrottling(t *testing.T) { - err := clientSeatLimitError(basecamp.ErrRateLimit(30), nil) + err := clientSeatLimitError(basecamp.ErrRateLimit(30)) var outErr *output.Error require.True(t, errors.As(err, &outErr)) @@ -394,5 +392,23 @@ func TestPeopleClientsDisableNamesRemainingClients(t *testing.T) { 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") +} 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. From 1a208cf04ef4e51d9d89ab1f28cb4d418de93b20 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Tue, 8 Sep 2026 17:43:52 -0700 Subject: [PATCH 4/6] Re-pin basecamp-sdk to dcd9fbd6; reject stray positionals on the project-only verbs The SDK branch head decodes the row-error selectors independently and reports client access as project_access; the pin, provenance, vendored MCP model, and Nix vendorHash follow it. list, enable, and disable take no positional, so a stray one is refused instead of discarded: with a default project configured, "disable 123" would otherwise have acted on the configured project. The remove hint disable offers separates ids with spaces so it pastes as a command. --- internal/commands/people.go | 13 +++++++++---- internal/commands/people_clients_test.go | 15 ++++++++++++++- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/internal/commands/people.go b/internal/commands/people.go index c7113126..808ca073 100644 --- a/internal/commands/people.go +++ b/internal/commands/people.go @@ -945,6 +945,7 @@ 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 { @@ -1232,7 +1233,7 @@ func runPeopleClientsAccess(cmd *cobra.Command, projectID string, people []strin } 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)))) + fmt.Sprintf("Not %s (%s): %s", strings.ToLower(verb), missedWhy, joinInt64s(missed, ", ")))) } return app.OK(result, respOpts...) @@ -1263,12 +1264,14 @@ func unaffectedPersonIDs(requested []int64, affected []basecamp.Person) []int64 return missed } -func joinInt64s(ids []int64) string { +// 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, ", ") + return strings.Join(parts, sep) } func newPeopleClientsInviteCmd() *cobra.Command { @@ -1430,6 +1433,7 @@ 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 { @@ -1457,6 +1461,7 @@ Refused (403) while any client still has access; remove them first with 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 { @@ -1537,7 +1542,7 @@ func clientEnablementError(ctx context.Context, app *appctx.App, bucketID int64, 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), + Hint: fmt.Sprintf("Remove them first: basecamp people clients remove %s --in %s", joinInt64s(ids, " "), projectRef), HTTPStatus: sdkErr.HTTPStatus, Cause: sdkErr, } diff --git a/internal/commands/people_clients_test.go b/internal/commands/people_clients_test.go index 4af576ba..fa45a8e7 100644 --- a/internal/commands/people_clients_test.go +++ b/internal/commands/people_clients_test.go @@ -393,7 +393,7 @@ func TestPeopleClientsDisableNamesRemainingClients(t *testing.T) { 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") + 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, @@ -412,3 +412,16 @@ func TestPeopleClientsDisableWithoutClientsIsAPlainDenial(t *testing.T) { 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) + } +} From 129ece6c852bd873562ba8cd598c48c0155c218b Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Tue, 8 Sep 2026 17:55:47 -0700 Subject: [PATCH 5/6] Name every malformed invitee at once, and say which errors are local A token that is not an address is refused before any request as a usage error; the help and skill text had promised the server's validation exit for it. The local check now names every malformed token in one error instead of stopping at the first, and the docs distinguish the local refusal (usage, 2) from the server's all-or-nothing rejection (validation, 9) and seat shortfall (limit_exceeded, 10). people list's client flag is pinned by a test. --- internal/commands/people.go | 22 +++++++++++------ internal/commands/people_clients_test.go | 31 ++++++++++++++++++++++++ skills/basecamp/SKILL.md | 7 +++--- 3 files changed, 50 insertions(+), 10 deletions(-) diff --git a/internal/commands/people.go b/internal/commands/people.go index 808ca073..fe2335ff 100644 --- a/internal/commands/people.go +++ b/internal/commands/people.go @@ -1019,18 +1019,24 @@ type clientInvitee struct { // 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. +// 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 { - return nil, output.ErrUsageHint( - fmt.Sprintf(`%q is not an email address or "Name "`, token), - `Name each client by email address, or as "Full Name " to set the name`) + 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 } @@ -1287,9 +1293,11 @@ Each invitee is an email address, or "Name " to set the display name from stdin. --company applies to every invitee in the invocation; --title names one person's role, so it takes exactly one invitee. -Invitations are all-or-nothing: an invalid address fails the whole batch -with each bad row named (exit 9, validation), and a batch that would exceed -the account's user limit fails with nobody invited (exit 10, limit_exceeded). +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". diff --git a/internal/commands/people_clients_test.go b/internal/commands/people_clients_test.go index fa45a8e7..d023b689 100644 --- a/internal/commands/people_clients_test.go +++ b/internal/commands/people_clients_test.go @@ -86,6 +86,37 @@ func TestParseClientInviteesRejectsNonAddresses(t *testing.T) { } } +// 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")) diff --git a/skills/basecamp/SKILL.md b/skills/basecamp/SKILL.md index 931d6677..29886ae9 100644 --- a/skills/basecamp/SKILL.md +++ b/skills/basecamp/SKILL.md @@ -1270,9 +1270,10 @@ 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: an invalid address exits `validation` (9) naming each rejected -row, and a seat shortfall exits `limit_exceeded` (10) — in both cases nobody -was invited. `add`/`remove` report the ids the server did not grant or revoke +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 From 09de5e5ac6822e7e516084fb165095ba8c135c9b Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Wed, 9 Sep 2026 15:14:52 -0700 Subject: [PATCH 6/6] Pin basecamp-sdk to v0.17.0 --- go.mod | 2 +- go.sum | 4 ++-- internal/mcpserver/model/PROVENANCE.json | 4 ++-- internal/version/sdk-provenance.json | 6 +++--- nix/package.nix | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) 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/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/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" ];