From 6c80306ccf14c060b86ad381d80e80cda811a273 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Mon, 19 Oct 2026 20:50:05 -0400 Subject: [PATCH 1/7] Build against SDK v0.26.0 v0.26.0 adds the Pings service and UpdateBoost. The vendored MCP model is resynced to it: the campfires domain claims the new Pings tag, since a Ping is a private conversation held in its own Campfire, and the boosts domain gains update_boost. The Summarize composite test serves the /circles probe the SDK now makes for a bucket with no dock. Moving from v0.24.0 also takes v0.25.0's lineup marker change: CreateMarker and UpdateMarker now return the saved marker, so lineup.go discards it where it discarded nothing before. --- go.mod | 2 +- go.sum | 4 +- internal/commands/lineup.go | 4 +- internal/mcpserver/catalog_test.go | 2 +- internal/mcpserver/composite_test.go | 2 + internal/mcpserver/domains.go | 6 +- internal/mcpserver/model/PROVENANCE.json | 4 +- internal/mcpserver/model/behavior-model.json | 50 ++ internal/mcpserver/model/openapi.json | 607 +++++++++++++++++- .../mcpserver/testdata/catalog_snapshot.txt | 22 +- internal/version/sdk-provenance.json | 10 +- nix/package.nix | 2 +- 12 files changed, 669 insertions(+), 46 deletions(-) diff --git a/go.mod b/go.mod index f6a74d0d8..c8a17713f 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,7 @@ require ( charm.land/bubbles/v2 v2.2.1 charm.land/bubbletea/v2 v2.1.0 charm.land/lipgloss/v2 v2.0.6 - github.com/basecamp/basecamp-sdk/go v0.24.0 + github.com/basecamp/basecamp-sdk/go v0.26.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 0257b54aa..f28371adc 100644 --- a/go.sum +++ b/go.sum @@ -89,8 +89,8 @@ github.com/aymerick/douceur v0.2.0 h1:Mv+mAeH1Q+n9Fr+oyamOlAkUNPWPlA8PPGR0QAaYuP github.com/aymerick/douceur v0.2.0/go.mod h1:wlT5vV2O3h55X9m7iVYN0TBM0NH/MmbLnd30/FjWUq4= github.com/basecamp/actioncable-go v1.2.0 h1:t0MxlfMTcmaGXZ5ztgKRs7ZiY1MwXtBRwSSSAh1ofVg= github.com/basecamp/actioncable-go v1.2.0/go.mod h1:ezaV5z1GXQAsqyejqTs6wCFl2D8Wj+COLQkHc/kwoRs= -github.com/basecamp/basecamp-sdk/go v0.24.0 h1:kJGxd96qgtp5IH9eAVkzoo8LiHs9Raahcb5dWUTTSEI= -github.com/basecamp/basecamp-sdk/go v0.24.0/go.mod h1:vdplxIcZMYSAIOvVm7AOIgLBUAPoJHESStTh9mF/ww4= +github.com/basecamp/basecamp-sdk/go v0.26.0 h1:ENIK+0Q9WWRCzcJjz0RLLpvAMSfKoQOl8lHRumaEeOg= +github.com/basecamp/basecamp-sdk/go v0.26.0/go.mod h1:PAp3gBnEPH7LtAyWXwgQ1tfmCAJiMkRHgRJAKT/Xefc= github.com/basecamp/cli v0.2.2-0.20260828230226-767413fc712d h1:jAzDrCCzDpIwhbFT1xVVs0z2xpXoDEkomHfKB2bUUp8= github.com/basecamp/cli v0.2.2-0.20260828230226-767413fc712d/go.mod h1:iTBTaWvsPEFIcZfkxQHEfISyJ6sZ7036K6bNx0RY3EE= github.com/basecamp/mcp v0.0.0-20260828100356-2d6f44b51e9d h1:zEQVGq1x1nhKMZ2TudFAcSJ32CHT8richI1vQakIKz4= diff --git a/internal/commands/lineup.go b/internal/commands/lineup.go index ec6713c5c..95c5a6225 100644 --- a/internal/commands/lineup.go +++ b/internal/commands/lineup.go @@ -114,7 +114,7 @@ The date accepts natural language dates: Date: parsedDate, } - if err := app.Account().Lineup().CreateMarker(cmd.Context(), req); err != nil { + if _, err := app.Account().Lineup().CreateMarker(cmd.Context(), req); err != nil { return convertSDKError(err) } @@ -193,7 +193,7 @@ You can pass either a marker ID or a Basecamp URL: req.Date = parsedDate } - if err := app.Account().Lineup().UpdateMarker(cmd.Context(), markerID, req); err != nil { + if _, err := app.Account().Lineup().UpdateMarker(cmd.Context(), markerID, req); err != nil { return convertSDKError(err) } diff --git a/internal/mcpserver/catalog_test.go b/internal/mcpserver/catalog_test.go index 550f46455..d13c68d5c 100644 --- a/internal/mcpserver/catalog_test.go +++ b/internal/mcpserver/catalog_test.go @@ -71,7 +71,7 @@ func TestCatalogExcludesUnservedOperations(t *testing.T) { assert.False(t, excluded[op.ID], "operation %q should be excluded from the vendored model", op.ID) } } - assert.Equal(t, 275, model, "served model operation count") + assert.Equal(t, 279, model, "served model operation count") assert.Equal(t, 1, composite, "served composite operation count") } diff --git a/internal/mcpserver/composite_test.go b/internal/mcpserver/composite_test.go index 1b42d8bcc..9f261d851 100644 --- a/internal/mcpserver/composite_test.go +++ b/internal/mcpserver/composite_test.go @@ -220,6 +220,8 @@ func TestSummarizeFallsBackToTheAccountCampfireListing(t *testing.T) { session, log := compositeSession(t, map[string]http.HandlerFunc{ // Not a project: the dock source has nothing to offer. "/999/projects/": serveJSON(t, http.StatusNotFound, `{"error":"not found"}`), + // Nor a Ping, which the SDK tries next for a bucket with no dock. + "/999/circles/": serveJSON(t, http.StatusNotFound, `{"error":"not found"}`), "/999/chats.json": serveJSON(t, http.StatusOK, `[ {"id": 800, "title": "Elsewhere", "bucket": {"id": 12, "name": "Other", "type": "Project"}}, {"id": 901, "title": "Ping", "bucket": {"id": 77, "name": "Connector", "type": "Project"}}, diff --git a/internal/mcpserver/domains.go b/internal/mcpserver/domains.go index 21c556481..339c22e92 100644 --- a/internal/mcpserver/domains.go +++ b/internal/mcpserver/domains.go @@ -41,13 +41,13 @@ var DomainSpecs = []catalog.DomainSpec{ }, { Key: "campfires", - Tags: []string{"Campfire"}, - Blurb: "Campfire chat: rooms, chat lines, uploads, and chatbots.", + Tags: []string{"Campfire", "Pings"}, + Blurb: "Campfire chat: rooms, chat lines, uploads, and chatbots, and Pings: private conversations, direct or group, each held in its own Campfire.", }, { Key: "boosts", Tags: []string{"Boosts"}, - Blurb: "Boosts (emoji reactions) on recordings and their events: list, get, create, and delete.", + Blurb: "Boosts (emoji reactions) on recordings and their events: list, get, create, update, and delete.", }, { Key: "schedules", diff --git a/internal/mcpserver/model/PROVENANCE.json b/internal/mcpserver/model/PROVENANCE.json index f81074cce..987474f70 100644 --- a/internal/mcpserver/model/PROVENANCE.json +++ b/internal/mcpserver/model/PROVENANCE.json @@ -1,7 +1,7 @@ { "source": "github.com/basecamp/basecamp-sdk", - "commit": "490e10dcea4a933689ccaafa9214c03365fff969", - "ref": "v0.24.0", + "commit": "dfe90054e69065000bd3410d6f02435186fe4b1b", + "ref": "v0.26.0", "files": ["behavior-model.json", "openapi.json"], "synced_by": "scripts/sync-mcp-model.sh", "patches": "binary-upload operations dropped because the toolkit refuses their non-JSON bodies (EXCLUDED_OPERATIONS) and the stream-ticket mint dropped by policy (POLICY_EXCLUDED_OPERATIONS); no tag patches applied (PATCHED_TAGS is empty — the export tags every operation) — see the sync script" diff --git a/internal/mcpserver/model/behavior-model.json b/internal/mcpserver/model/behavior-model.json index 2b00b0784..98e2f2258 100644 --- a/internal/mcpserver/model/behavior-model.json +++ b/internal/mcpserver/model/behavior-model.json @@ -251,6 +251,17 @@ ] } }, + "CreatePing": { + "retry": { + "max": 2, + "base_delay_ms": 1000, + "backoff": "exponential", + "retry_on": [ + 429, + 503 + ] + } + }, "CreateProject": { "retry": { "max": 3, @@ -1543,6 +1554,18 @@ ] } }, + "GetPing": { + "readonly": true, + "retry": { + "max": 3, + "base_delay_ms": 1000, + "backoff": "exponential", + "retry_on": [ + 429, + 503 + ] + } + }, "GetProgressReport": { "readonly": true, "pagination": { @@ -2327,6 +2350,18 @@ ] } }, + "ListPings": { + "readonly": true, + "retry": { + "max": 3, + "base_delay_ms": 1000, + "backoff": "exponential", + "retry_on": [ + 429, + 503 + ] + } + }, "ListProjectPeople": { "readonly": true, "pagination": { @@ -3122,6 +3157,18 @@ ] } }, + "UpdateBoost": { + "idempotent": true, + "retry": { + "max": 3, + "base_delay_ms": 1000, + "backoff": "exponential", + "retry_on": [ + 429, + 503 + ] + } + }, "UpdateCalendar": { "idempotent": true, "retry": { @@ -3579,6 +3626,9 @@ "PersonCompany": [ "$.name" ], + "Ping": [ + "$.avatar_url" + ], "TemplateLibraryConfirmationPerson": [ "$.name", "$.avatar_url" diff --git a/internal/mcpserver/model/openapi.json b/internal/mcpserver/model/openapi.json index f5fbfc865..21f6cc92f 100644 --- a/internal/mcpserver/model/openapi.json +++ b/internal/mcpserver/model/openapi.json @@ -2,7 +2,7 @@ "openapi": "3.1.0", "info": { "title": "Basecamp", - "version": "2026-09-15", + "version": "2026-10-19", "description": "Basecamp API", "contact": { "name": "Basecamp", @@ -448,6 +448,112 @@ 503 ] } + }, + "put": { + "description": "Replace the content of a boost. Only the boost's creator may update it: an\nadmin updating someone else's boost gets 403 Forbidden, as does any caller\nwhen comments are closed on the boosted recording. A boost on a deleted\nrecording, or in another account, answers 404. A missing or invalid\n`content` answers a bodiless 422. There is no bucket-scoped route.", + "operationId": "UpdateBoost", + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/UpdateBoostRequestContent" + } + } + }, + "required": true + }, + "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": "boostId", + "in": "path", + "schema": { + "type": "integer", + "format": "int64" + }, + "required": true + } + ], + "responses": { + "200": { + "description": "UpdateBoost 200 response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/UpdateBoostResponseContent" + } + } + } + }, + "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": "BareValidationError 422 response" + }, + "500": { + "description": "InternalServerError 500 response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/InternalServerErrorResponseContent" + } + } + } + } + }, + "tags": [ + "Boosts" + ], + "x-basecamp-idempotent": { + "natural": true + }, + "x-basecamp-retry": { + "maxAttempts": 3, + "baseDelayMs": 1000, + "backoff": "exponential", + "retryOn": [ + 429, + 503 + ] + } } }, "/{accountId}/buckets/{bucketId}/card_tables/columns/{columnId}/color.json": { @@ -812,7 +918,7 @@ }, "/{accountId}/buckets/{bucketId}/card_tables/wormholes/{wormholeId}": { "delete": { - "description": "Delete a wormhole", + "description": "Permanently delete a card table's wormhole, the shortcut that sends cards to another card table.\n\nIt does not go to the trash and cannot be restored.", "operationId": "DeleteWormhole", "parameters": [ { @@ -917,7 +1023,7 @@ } }, "put": { - "description": "Update a wormhole's destination column", + "description": "Point a card table's wormhole, its shortcut for sending cards to another card table, at a different destination column", "operationId": "UpdateWormhole", "requestBody": { "content": { @@ -1031,7 +1137,7 @@ }, "/{accountId}/buckets/{bucketId}/card_tables/{cardTableId}/wormholes.json": { "post": { - "description": "Create a wormhole linking this card table to a column on another card table.\n\nA wormhole is the only mechanism for moving a card to a different project: its\nid is a valid `column_id` for MoveCard, teleporting the card across projects.\n`destinationRecordingId` is the id of a column on another accessible card table.", + "description": "Add a wormhole to a card table: a standing shortcut that sends cards to a column on another card table, usually in another project.\n\nA wormhole is the only mechanism for moving a card to a different project: its\nid is a valid `column_id` for MoveCard, teleporting the card there. It is not\nneeded for a one-off move within a project; MoveCard goes to any column there\ndirectly. A card table holds at most four wormholes.\n`destinationRecordingId` is the id of a column on another accessible card table.", "operationId": "CreateWormhole", "requestBody": { "content": { @@ -4105,7 +4211,7 @@ }, "/{accountId}/card_tables/cards/{cardId}/moves.json": { "post": { - "description": "Move a card to a different column", + "description": "Move a card to any column in its project, including on another card table there, or through a linked wormhole to another project (queued: the card gets a new id, and position, if sent, must still be positive but doesn't set placement).\n\n`column_id` may be any column in the card's project, including a column on\nanother card table in the same project; the card moves there directly, with\nno wormhole needed. To send the card to another project, pass the id of a\nlinked wormhole (`linked: true`) the card table already has, listed in\nGetCardTable's `wormholes`; an unlinked or unreachable one returns not found.\nA move through a wormhole is queued: the card arrives under a new id and its\nold id then returns not found. `position` does not control where it lands,\nbut if supplied it must still be a positive integer.", "operationId": "MoveCard", "requestBody": { "content": { @@ -5555,7 +5661,7 @@ }, "/{accountId}/card_tables/{cardTableId}/moves.json": { "post": { - "description": "Move a column within a card table", + "description": "Move a column to a slot within its card table. target_id is the card table's own id (the same as cardTableId), not a column; position is the 1-based slot among the columns, not counting Triage, Not Now or Done.", "operationId": "MoveCardColumn", "requestBody": { "content": { @@ -7344,6 +7450,183 @@ } } }, + "/{accountId}/circles.json": { + "get": { + "description": "Get Pings: the caller's active Pings that have a line in them, most\nrecently updated first, in one unpaginated response. Archived Pings, and\nPings no one has posted in yet, are not listed; GetPing reads either by id.\n\nAnswers `403 Forbidden` (`{\"error\": \"Pings have been disabled\"}`) when the\ncaller cannot ping.", + "operationId": "ListPings", + "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 + } + ], + "responses": { + "200": { + "description": "ListPings 200 response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ListPingsResponseContent" + } + } + } + }, + "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" + } + } + } + }, + "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": [ + "Pings" + ], + "x-basecamp-retry": { + "maxAttempts": 3, + "baseDelayMs": 1000, + "backoff": "exponential", + "retryOn": [ + 429, + 503 + ] + } + }, + "post": { + "description": "Create a Ping with people, or return the one that already holds exactly\nthem.\n\nThe caller is always included. Without `new_group`, an existing active\nPing with exactly these people (the oldest, if several) is returned rather\nthan a second one created; with `new_group: true` and more than one other\nperson, a new group is created every time. Either way the answer is\n`201 Created` with the Ping.\n\nAnswers `403 Forbidden` (`{\"error\": \"Pings have been disabled\"}`) when the\ncaller cannot ping, and a bodiless `422` when nobody in the Ping would be a\nperson who can ping.", + "operationId": "CreatePing", + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/CreatePingRequestContent" + } + } + }, + "required": true + }, + "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 + } + ], + "responses": { + "201": { + "description": "CreatePing 201 response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/CreatePingResponseContent" + } + } + } + }, + "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" + } + } + } + }, + "422": { + "description": "BareValidationError 422 response" + }, + "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": [ + "Pings" + ], + "x-basecamp-retry": { + "maxAttempts": 2, + "baseDelayMs": 1000, + "backoff": "exponential", + "retryOn": [ + 429, + 503 + ] + } + } + }, "/{accountId}/circles/people.json": { "get": { "description": "List all account users who can be pinged\n\n**Pagination**: Uses Link header (RFC5988). Follow the `next` rel URL\nto fetch additional pages. X-Total-Count header provides total count.", @@ -7432,6 +7715,110 @@ } } }, + "/{accountId}/circles/{pingId}": { + "get": { + "description": "Get a Ping by its id, which is its bucket's id.\n\nAnswers `403 Forbidden` (`{\"error\": \"Pings have been disabled\"}`) when the\ncaller cannot ping, and `404 Not Found` for a bucket that is not a Ping the\ncaller is in.", + "operationId": "GetPing", + "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": "pingId", + "in": "path", + "description": "A Ping's id is the id of the bucket that holds it (the bucket type is\n\"Circle\", which is also the wire path), so the `bucket.id` a Ping's\nrecordings carry is the id GetPing takes.", + "schema": { + "type": "integer", + "description": "A Ping's id is the id of the bucket that holds it (the bucket type is\n\"Circle\", which is also the wire path), so the `bucket.id` a Ping's\nrecordings carry is the id GetPing takes.", + "format": "int64" + }, + "required": true + } + ], + "responses": { + "200": { + "description": "GetPing 200 response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/GetPingResponseContent" + } + } + } + }, + "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": [ + "Pings" + ], + "x-basecamp-retry": { + "maxAttempts": 3, + "baseDelayMs": 1000, + "backoff": "exponential", + "retryOn": [ + 429, + 503 + ] + } + } + }, "/{accountId}/client/approvals/{approvalId}": { "get": { "description": "Get a single client approval by id", @@ -10313,7 +10700,7 @@ } }, "post": { - "description": "Create a new lineup marker", + "description": "Create a new lineup marker. Returns the created marker.", "operationId": "CreateLineupMarker", "requestBody": { "content": { @@ -10340,7 +10727,14 @@ ], "responses": { "201": { - "description": "CreateLineupMarker 201 response" + "description": "CreateLineupMarker 201 response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/CreateLineupMarkerResponseContent" + } + } + } }, "401": { "description": "UnauthorizedError 401 response", @@ -10495,7 +10889,7 @@ } }, "put": { - "description": "Update an existing lineup marker", + "description": "Update an existing lineup marker. Returns the updated marker.", "operationId": "UpdateLineupMarker", "requestBody": { "content": { @@ -10530,7 +10924,14 @@ ], "responses": { "200": { - "description": "UpdateLineupMarker 200 response" + "description": "UpdateLineupMarker 200 response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/UpdateLineupMarkerResponseContent" + } + } + } }, "401": { "description": "UnauthorizedError 401 response", @@ -12601,7 +13002,7 @@ }, "/{accountId}/my/readings.json": { "get": { - "description": "Get the current user's notification inbox (the \"Hey!\" menu).\nNotifications are grouped into unreads, reads, bubble-ups, and\nscheduled bubble-ups (`memories` remains as an always-empty\nplaceholder on BC5). Reads are paginated (50 per page). Unreads are\ncapped at 100. Bubble-ups are capped per `limit_bubble_ups`.", + "description": "Get the current user's notifications (the \"Hey!\" menu in Basecamp 4).\nNotifications are grouped into unreads, reads, bubble-ups, and\nscheduled bubble-ups (`memories` remains as an always-empty\nplaceholder on BC5). Reads are paginated (50 per page). Unreads are\ncapped at 100. Bubble-ups are capped per `limit_bubble_ups`.", "operationId": "GetMyNotifications", "parameters": [ { @@ -16461,7 +16862,7 @@ } }, "put": { - "description": "Update an existing question", + "description": "Update an existing question's title, schedule or both. A schedule replaces the whole schedule, so send every part of it: one without frequency or time_of_day is refused, and one without start_date re-anchors the recurrence on today. Pausing and resuming are PauseQuestion and ResumeQuestion.", "operationId": "UpdateQuestion", "requestBody": { "content": { @@ -18676,7 +19077,7 @@ }, "/{accountId}/recordings/{recordingId}/events/{eventId}/boosts.json": { "get": { - "description": "List boosts on a specific event within a recording", + "description": "List boosts on a specific event within a recording. Only boostable events carry boosts: those whose action is completed, adopted, column_changed or moved_in (their JSON includes boosts_count and boosts_url). Any other event answers 403 Forbidden; an event on a recording the person cannot see answers 404.", "operationId": "ListEventBoosts", "parameters": [ { @@ -18790,7 +19191,7 @@ } }, "post": { - "description": "Create a boost on a specific event within a recording", + "description": "Create a boost on a specific event within a recording. Only boostable events accept one: those whose action is completed, adopted, column_changed or moved_in (their JSON includes boosts_count and boosts_url). Any other event answers 403 Forbidden, as does an event on a recording whose comments are closed; an event on a recording the person cannot see answers 404.", "operationId": "CreateEventBoost", "requestBody": { "content": { @@ -30445,6 +30846,7 @@ "items": { "$ref": "#/components/schemas/Wormhole" }, + "description": "The card table's wormholes: shortcuts for sending cards to other card\ntables, usually in other projects. Only wormholes whose destination the\nrequester can reach are listed; the rest are omitted, not shown unlinked.\nNot needed to move a card within its project; MoveCard reaches any column\nthere directly.", "x-go-type-skip-optional-pointer": true } }, @@ -31523,6 +31925,9 @@ "name" ] }, + "CreateLineupMarkerResponseContent": { + "$ref": "#/components/schemas/LineupMarker" + }, "CreateMessageRequestContent": { "type": "object", "properties": { @@ -31601,6 +32006,29 @@ "name" ] }, + "CreatePingRequestContent": { + "type": "object", + "properties": { + "user_ids": { + "type": "array", + "items": { + "type": "integer", + "format": "int64" + }, + "description": "Person ids of the people to ping. People the caller cannot reach are\ndropped." + }, + "new_group": { + "type": "boolean", + "description": "Create a new group even when a Ping with exactly these people exists." + } + }, + "required": [ + "user_ids" + ] + }, + "CreatePingResponseContent": { + "$ref": "#/components/schemas/Ping" + }, "CreateProjectFromTemplateRequestContent": { "type": "object", "properties": { @@ -32528,10 +32956,12 @@ }, "boosts_count": { "type": "integer", + "description": "Present only on boostable events (action completed, adopted,\ncolumn_changed or moved_in); the event boost operations refuse any\nother event with 403.", "format": "int32" }, "boosts_url": { - "type": "string" + "type": "string", + "description": "Present only on boostable events; see boosts_count." } }, "required": [ @@ -33857,6 +34287,9 @@ "GetPersonResponseContent": { "$ref": "#/components/schemas/Person" }, + "GetPingResponseContent": { + "$ref": "#/components/schemas/Ping" + }, "GetProgressReportResponseContent": { "type": "array", "items": { @@ -34498,6 +34931,12 @@ "$ref": "#/components/schemas/Person" } }, + "ListPingsResponseContent": { + "type": "array", + "items": { + "$ref": "#/components/schemas/Ping" + } + }, "ListProjectPeopleResponseContent": { "type": "array", "items": { @@ -34844,14 +35283,17 @@ "properties": { "source_id": { "type": "integer", + "description": "The column to move.", "format": "int64" }, "target_id": { "type": "integer", + "description": "The card table the column moves within: the same id as `cardTableId`.\nBasecamp refuses any other destination, a column included, with 422.", "format": "int64" }, "position": { "type": "integer", + "description": "1-based slot among the card table's columns, not counting Triage,\nNot Now or Done. 1 puts the column first; a slot past the last puts it\nlast. Default: 1.", "format": "int32" } }, @@ -34865,6 +35307,7 @@ "properties": { "column_id": { "type": "integer", + "description": "Any column in the card's project, on this or another card table, or a\nwormhole id to send the card to another project.", "format": "int64" }, "position": { @@ -35490,6 +35933,110 @@ "name" ] }, + "Ping": { + "type": "object", + "description": "A Ping: a private conversation between people, held in its own bucket with\none Campfire. The Campfire's lines are read and written through the\nCampfires service; `room_url` and `links.lines` name it.", + "properties": { + "id": { + "type": "integer", + "description": "The Ping's id, which is also its bucket's id.", + "format": "int64" + }, + "created_at": { + "type": "string", + "x-go-type": "time.Time", + "x-go-type-import": { + "path": "time" + } + }, + "updated_at": { + "type": "string", + "x-go-type": "time.Time", + "x-go-type-import": { + "path": "time" + } + }, + "title": { + "type": "string", + "description": "The Ping's name for the caller: its given name when `named`, else the\nnames of the other people in it." + }, + "named": { + "type": "boolean", + "description": "Whether someone gave the Ping a name." + }, + "image_url": { + "type": "string", + "description": "The Ping's own image, present only when one was uploaded." + }, + "avatar_url": { + "type": "string", + "format": "password", + "x-basecamp-sensitive": { + "category": "pii", + "redact": true + } + }, + "url": { + "type": "string" + }, + "app_url": { + "type": "string" + }, + "bookmark_url": { + "type": "string" + }, + "room_url": { + "type": "string", + "description": "The Ping's Campfire (`/buckets/{bucketId}/chats/{campfireId}.json`)." + }, + "edit_title_url": { + "type": "string", + "description": "Present only when the Ping can be renamed: one with more than one other\nperson in it." + }, + "links": { + "$ref": "#/components/schemas/PingLinks" + }, + "people": { + "type": "array", + "items": { + "$ref": "#/components/schemas/Person" + }, + "description": "The people in the Ping other than the caller; the caller alone when\nnobody else is in it." + } + }, + "required": [ + "app_url", + "avatar_url", + "bookmark_url", + "created_at", + "id", + "links", + "named", + "people", + "room_url", + "title", + "updated_at", + "url" + ] + }, + "PingLinks": { + "type": "object", + "description": "The Ping's Campfire endpoints, so a client need not derive them from\n`room_url`. The wire object also repeats `url`, `app_url`, `bookmark_url`\nand `edit_title_url` as `self`, `app`, `bookmark` and `edit_title`; those\nare read from the Ping's own fields.", + "properties": { + "lines": { + "type": "string", + "description": "The Campfire's lines (`/buckets/{bucketId}/chats/{campfireId}/lines.json`)." + }, + "files": { + "type": "string", + "description": "The Campfire's uploads." + } + }, + "required": [ + "files", + "lines" + ] + }, "PollEventsResponseContent": { "type": "object", "description": "The poll envelope. Every 200 carries `events` and `position`; `next` only\nwhile the walk continues.", @@ -35666,6 +36213,10 @@ "clients_enabled": { "type": "boolean" }, + "timesheet_enabled": { + "type": "boolean", + "description": "True when the project's timesheet is turned on. The project timesheet\nreport (GET /projects/{projectId}/timesheet.json) answers 404 until it is." + }, "bookmark_url": { "type": "string" }, @@ -38991,6 +39542,21 @@ "UpdateAccountNameResponseContent": { "$ref": "#/components/schemas/Account" }, + "UpdateBoostRequestContent": { + "type": "object", + "properties": { + "content": { + "type": "string", + "maxLength": 16 + } + }, + "required": [ + "content" + ] + }, + "UpdateBoostResponseContent": { + "$ref": "#/components/schemas/Boost" + }, "UpdateCalendarRequestContent": { "type": "object", "properties": { @@ -39245,6 +39811,9 @@ } } }, + "UpdateLineupMarkerResponseContent": { + "$ref": "#/components/schemas/LineupMarker" + }, "UpdateMessageRequestContent": { "type": "object", "properties": { @@ -39454,13 +40023,11 @@ "type": "object", "properties": { "title": { - "type": "string" + "type": "string", + "description": "The question text. An update carries a title, a schedule or both." }, "schedule": { "$ref": "#/components/schemas/QuestionScheduleInput" - }, - "paused": { - "type": "boolean" } } }, @@ -40171,7 +40738,7 @@ }, "Wormhole": { "type": "object", - "description": "A wormhole links this card table to a column on another card table, enabling\ncards to move across projects. It carries the full recording representation\nplus the destination-linkage fields. The wormhole's own `url`/`app_url`/`parent`\npoint at the *source* board; `destination_url` is the only field identifying\nthe destination column.", + "description": "A wormhole is a standing shortcut from this card table to a column on another\ncard table, usually in another project, and the only way to move a card across\nprojects. It carries the full recording representation\nplus the destination-linkage fields. The wormhole's own `url`/`app_url`/`parent`\npoint at the *source* board; `destination_url` is the only field identifying\nthe destination column.", "properties": { "id": { "type": "integer", diff --git a/internal/mcpserver/testdata/catalog_snapshot.txt b/internal/mcpserver/testdata/catalog_snapshot.txt index 2b5b46cab..c0d833a68 100644 --- a/internal/mcpserver/testdata/catalog_snapshot.txt +++ b/internal/mcpserver/testdata/catalog_snapshot.txt @@ -60,8 +60,8 @@ ACTIONS (RO = read-only): - create_card: Create a card in a column - create_card_column: Create a column in a card table - create_card_step: Create a step on a card -- create_wormhole: Create a wormhole linking this card table to a column on another card table -- delete_wormhole: Delete a wormhole +- create_wormhole: Add a wormhole to a card table: a standing shortcut that sends cards to a column on another card table, usually in another project +- delete_wormhole: Permanently delete a card table's wormhole, the shortcut that sends cards to another card table - disable_card_column_on_hold: Disable on-hold section in a column - enable_card_column_on_hold: Enable on-hold section in a column - get_card (RO): Get a card by ID @@ -69,8 +69,8 @@ ACTIONS (RO = read-only): - get_card_step (RO): Get a step by ID - get_card_table (RO): Get a card table by ID - list_cards (RO, paginated): List cards in a column -- move_card: Move a card to a different column -- move_card_column: Move a column within a card table +- move_card: Move a card to any column in its project, including on another card table there, or through a linked wormhole to another project (queued: the card gets a new id, and position, if sent, must still be positive but doesn't set placement) +- move_card_column: Move a column to a slot within its card table - reposition_card_step: Reposition a step within a card - set_card_column_color: Set the color of a column - set_card_step_completion: Set card step completion status (PUT with completion: "on" to complete, "" to uncomplete) @@ -79,7 +79,7 @@ ACTIONS (RO = read-only): - update_card: Update an existing card - update_card_column: Update an existing column - update_card_step: Update an existing step -- update_wormhole: Update a wormhole's destination column +- update_wormhole: Point a card table's wormhole, its shortcut for sending cards to another card table, at a different destination column == basecamp_messages Message boards: messages, comments, message types (categories), and pinning. @@ -106,7 +106,7 @@ ACTIONS (RO = read-only): - update_message_type: Update an existing message type == basecamp_campfires -Campfire chat: rooms, chat lines, uploads, and chatbots. +Campfire chat: rooms, chat lines, uploads, and chatbots, and Pings: private conversations, direct or group, each held in its own Campfire. Gateway tool: call with {"action": "...", "params": {...}}. Call {"action": "describe", "params": {"action": "NAME"}} for an action's full parameter schema. @@ -114,20 +114,23 @@ Call {"action": "describe", "params": {"action": "NAME"}} for an action's full p ACTIONS (RO = read-only): - create_campfire_line: Create a new line (message) in a campfire - create_chatbot: Create a new chatbot for a campfire +- create_ping: Create a Ping with people, or return the one that already holds exactly them - delete_campfire_line: Delete a campfire line; allowed for the line's creator or an admin - delete_chatbot: Delete a chatbot - get_campfire (RO): Get a campfire by ID - get_campfire_line (RO): Get a campfire line by ID - get_chatbot (RO): Get a chatbot by ID +- get_ping (RO): Get a Ping by its id, which is its bucket's id - list_campfire_lines (RO, paginated): List all lines (messages) in a campfire - list_campfire_uploads (RO, paginated): List uploaded files in a campfire - list_campfires (RO, paginated): List all campfires across the account - list_chatbots (RO, paginated): List all chatbots for a campfire +- list_pings (RO): Get Pings: the caller's active Pings that have a line in them, most recently updated first, in one unpaginated response - update_campfire_line: Update an existing campfire line; the content is always treated as rich text (HTML) - update_chatbot: Update an existing chatbot == basecamp_boosts -Boosts (emoji reactions) on recordings and their events: list, get, create, and delete. +Boosts (emoji reactions) on recordings and their events: list, get, create, update, and delete. Gateway tool: call with {"action": "...", "params": {...}}. Call {"action": "describe", "params": {"action": "NAME"}} for an action's full parameter schema. @@ -139,6 +142,7 @@ ACTIONS (RO = read-only): - get_boost (RO): Get a single boost - list_event_boosts (RO, paginated): List boosts on a specific event within a recording - list_recording_boosts (RO, paginated): List boosts on a recording +- update_boost: Replace the content of a boost == basecamp_schedules Schedules and schedule entries (calendar events), timesheets and time tracking, and per-account calendars. @@ -283,7 +287,7 @@ ACTIONS (RO = read-only): - unspotlight_recording: Remove a recording from the spotlight area - update_answer: Update an existing answer - update_lineup_marker: Update an existing lineup marker -- update_question: Update an existing question +- update_question: Update an existing question's title, schedule or both - update_question_notification_settings: Update notification settings for a check-in question - update_template: Update an existing template - update_tool: Update (rename) an existing tool @@ -387,7 +391,7 @@ ACTIONS (RO = read-only): - get_my_completed_assignments (RO): Get the current user's completed assignments - get_my_due_assignments (RO): Get the current user's assignments filtered by due date scope - get_my_note (RO): Get the authenticated user's note — a per-person notebook singleton at /my/notes.json -- get_my_notifications (RO): Get the current user's notification inbox (the "Hey!" menu) +- get_my_notifications (RO): Get the current user's notifications (the "Hey!" menu in Basecamp 4) - list_gauge_needles (RO, paginated): List gauge needles for a project, ordered newest first - list_gauges (RO, paginated): List gauges across all projects the authenticated user has access to - list_my_bookmarks (RO, paginated): List the current user's bookmarks, most recently bookmarked first (paginated) diff --git a/internal/version/sdk-provenance.json b/internal/version/sdk-provenance.json index 850975797..fd3395d2b 100644 --- a/internal/version/sdk-provenance.json +++ b/internal/version/sdk-provenance.json @@ -1,13 +1,13 @@ { "sdk": { "module": "github.com/basecamp/basecamp-sdk/go", - "version": "v0.24.0", - "revision": "490e10dcea4a", - "updated_at": "2026-10-07T18:54:01Z" + "version": "v0.26.0", + "revision": "dfe90054e690", + "updated_at": "2026-10-20T00:38:27Z" }, "api": { "repo": "basecamp/bc3", - "revision": "66d387b62fb9ff968c5272b9cfec538b598ec05d", - "synced_at": "2026-09-15" + "revision": "4958288166a19b98894fce8d7deb5edbdd34424b", + "synced_at": "2026-10-19" } } diff --git a/nix/package.nix b/nix/package.nix index da749a31c..f547a368e 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-WHKt0cMO0Xnya3c5Kmt/Pnd/YquM3GovCHFrfFAGLmY="; + vendorHash = "sha256-l/kddGGXlnJXenhyAj5xRpEy0bJA+gTFFNreq+KY4kA="; subPackages = [ "cmd/basecamp" ]; From dc543f5665cd8763f78f613865801643dacd89cf Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Mon, 19 Oct 2026 11:50:06 -0400 Subject: [PATCH 2/7] Parse Ping URLs A Ping's web URL is circles/{ping} or circles/{ping}@{line}. The router sees the @ form as a type named after the whole segment, so urlarg now rewrites it the way it does chats/{room}@{line}: a Ping is its own bucket, so the form parses as a chat line in that bucket, with the Ping in the new PingID field. The Campfire is not in the URL; it comes from the Ping. --- internal/urlarg/urlarg.go | 43 ++++++++++++++++-- internal/urlarg/urlarg_ping_test.go | 67 +++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 4 deletions(-) create mode 100644 internal/urlarg/urlarg_ping_test.go diff --git a/internal/urlarg/urlarg.go b/internal/urlarg/urlarg.go index df10fd133..9f69a1474 100644 --- a/internal/urlarg/urlarg.go +++ b/internal/urlarg/urlarg.go @@ -13,6 +13,11 @@ import ( // and normalizes it to chats/{chatID}/lines/{lineID} before routing. var chatAtRe = regexp.MustCompile(`(/chats/\d+)@(\d+)`) +// pingAtRe matches the Basecamp web URL form circles/{pingID}@{lineID}, a chat +// line in a Ping. A Ping is its own bucket, so the form is parsed as a line +// whose bucket is the Ping; the Campfire is not in the URL. +var pingAtRe = regexp.MustCompile(`/circles/(\d+)@(\d+)`) + // Parsed represents components extracted from a Basecamp URL. type Parsed struct { AccountID string @@ -20,6 +25,7 @@ type Parsed struct { Type string // e.g., "todos", "messages", "cards" RecordingID string CampfireID string // parent campfire ID for chat-line URLs (chats/{campfireId}/lines/{lineId}) + PingID string // the Ping (a Circle bucket) named by circles/{pingId} and circles/{pingId}@{lineId} URLs CommentID string IsCollection bool // URL points to a list of items, not an individual resource OccurrenceDate string // date from schedule_entries/{id}/occurrences/{date} URLs @@ -31,10 +37,21 @@ var router = basecamp.DefaultRouter() // IsURL checks if the input looks like a Basecamp URL. // Returns true if the URL can be matched (either as an API route or structurally). func IsURL(input string) bool { - normalized := chatAtRe.ReplaceAllString(input, "${1}/lines/${2}") + normalized, _ := normalize(input) return router.Match(normalized) != nil } +// normalize rewrites the web @-forms into routable URLs. Chat lines become the +// slash form; a Ping line loses its @line, which the second result returns. +func normalize(input string) (normalized, pingLineID string) { + normalized = chatAtRe.ReplaceAllString(input, "${1}/lines/${2}") + if m := pingAtRe.FindStringSubmatch(normalized); m != nil { + pingLineID = m[2] + normalized = pingAtRe.ReplaceAllString(normalized, "/circles/${1}") + } + return normalized, pingLineID +} + // Parse extracts IDs from a Basecamp URL. // Returns nil if the input is not a valid Basecamp URL. // @@ -43,18 +60,35 @@ func IsURL(input string) bool { // - https://3.basecamp.com/{account}/buckets/{bucket}/{type}/{id}#__recording_{comment} // - https://3.basecamp.com/{account}/buckets/{bucket}/card_tables/cards/{id} // - https://3.basecamp.com/{account}/buckets/{bucket}/chats/{id}@{lineId} +// - https://3.basecamp.com/{account}/circles/{ping} +// - https://3.basecamp.com/{account}/circles/{ping}@{lineId} // - https://3.basecamp.com/{account}/buckets/{bucket}/schedule_entries/{id}/occurrences/{date} // - https://3.basecamp.com/{account}/projects/{project} func Parse(input string) *Parsed { - // Normalize the Basecamp web @-form for chat lines (chats/789@111) - // into the slash form (chats/789/lines/111) before routing. - normalized := chatAtRe.ReplaceAllString(input, "${1}/lines/${2}") + // Normalize the Basecamp web @-forms (chats/789@111, circles/456@111) + // into routable URLs before routing. + normalized, pingLineID := normalize(input) m := router.Match(normalized) if m == nil { return nil } + if pingLineID != "" { + pingID := m.Params["pingId"] + if pingID == "" { + return nil + } + return &Parsed{ + AccountID: m.AccountID, + ProjectID: pingID, + Type: "lines", + RecordingID: pingLineID, + PingID: pingID, + CommentID: m.CommentID, + } + } + pathType := m.PathType resourceID := m.ResourceID() @@ -102,6 +136,7 @@ func Parse(input string) *Parsed { Type: pathType, RecordingID: resourceID, CampfireID: campfireID, + PingID: m.Params["pingId"], CommentID: m.CommentID, IsCollection: isCollection, OccurrenceDate: occurrenceDate, diff --git a/internal/urlarg/urlarg_ping_test.go b/internal/urlarg/urlarg_ping_test.go new file mode 100644 index 000000000..05bf9f26e --- /dev/null +++ b/internal/urlarg/urlarg_ping_test.go @@ -0,0 +1,67 @@ +package urlarg + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestParsePingURLs(t *testing.T) { + tests := []struct { + name string + input string + want Parsed + }{ + { + name: "app host ping", + input: "https://app.basecamp.com/123/circles/456", + want: Parsed{AccountID: "123", Type: "circles", RecordingID: "456", PingID: "456"}, + }, + { + name: "web host ping", + input: "https://3.basecamp.com/123/circles/456", + want: Parsed{AccountID: "123", Type: "circles", RecordingID: "456", PingID: "456"}, + }, + { + name: "api host ping json", + input: "https://3.basecampapi.com/123/circles/456.json", + want: Parsed{AccountID: "123", Type: "circles", RecordingID: "456", PingID: "456"}, + }, + { + name: "app host ping line", + input: "https://app.basecamp.com/123/circles/456@789", + want: Parsed{AccountID: "123", ProjectID: "456", Type: "lines", RecordingID: "789", PingID: "456"}, + }, + { + name: "local dev ping line", + input: "http://3.basecamp.localhost:3001/123/circles/456@789", + want: Parsed{AccountID: "123", ProjectID: "456", Type: "lines", RecordingID: "789", PingID: "456"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.True(t, IsURL(tt.input)) + got := Parse(tt.input) + require.NotNil(t, got) + assert.Equal(t, tt.want, *got) + }) + } +} + +func TestParsePingLineURLNamesNoCampfire(t *testing.T) { + got := Parse("https://3.basecamp.com/123/circles/456@789") + require.NotNil(t, got) + assert.Empty(t, got.CampfireID, "the Campfire is not in a Ping URL; it comes from the Ping") +} + +func TestParseCircleCollectionsCarryNoPing(t *testing.T) { + for _, u := range []string{ + "https://3.basecamp.com/123/circles", + "https://3.basecamp.com/123/circles/people", + } { + got := Parse(u) + require.NotNil(t, got, u) + assert.Empty(t, got.PingID, u) + } +} From b8f06ee3e7b4720bac34182469f00eabefa9459e Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Mon, 19 Oct 2026 11:50:13 -0400 Subject: [PATCH 3/7] List a person's pings in the TUI from the Pings API The Pings view built its rooms from the account Campfire listing, which never returns a ping's Campfire, so it was always empty. Build them from Pings().List instead, with each ping's Campfire from its room URL; the ping is its own bucket, so it is the room's project. Pings can be groups, so the room carries a Title rather than a person's name, and the wording no longer calls them 1:1 threads. --- internal/tui/empty/messages.go | 6 +- internal/tui/workspace/actions.go | 2 +- internal/tui/workspace/data/hub_global.go | 100 ++++++++++-------- .../tui/workspace/data/ping_rooms_test.go | 64 +++++++++++ internal/tui/workspace/data/types.go | 5 +- .../tui/workspace/views/bonfire_sidebar.go | 4 +- internal/tui/workspace/views/pings.go | 10 +- internal/tui/workspace/views/sort_test.go | 12 +-- 8 files changed, 141 insertions(+), 62 deletions(-) create mode 100644 internal/tui/workspace/data/ping_rooms_test.go diff --git a/internal/tui/empty/messages.go b/internal/tui/empty/messages.go index 01cfdd403..9bdc779b5 100644 --- a/internal/tui/empty/messages.go +++ b/internal/tui/empty/messages.go @@ -165,11 +165,11 @@ func NoDocsFiles() Message { } } -// NoPings returns the empty state for no ping threads. +// NoPings returns the empty state for no pings. func NoPings() Message { return Message{ - Title: "No ping threads", - Body: "No active ping conversations.", + Title: "No pings", + Body: "A ping appears here once it has a message. Start one with: basecamp pings start ", } } diff --git a/internal/tui/workspace/actions.go b/internal/tui/workspace/actions.go index 2cbd87276..6cc5e1412 100644 --- a/internal/tui/workspace/actions.go +++ b/internal/tui/workspace/actions.go @@ -230,7 +230,7 @@ func DefaultActions() *Registry { r.Register(Action{ Name: ":pings", Aliases: []string{"dm", "direct messages"}, - Description: "Direct messages (pings)", + Description: "Pings: direct and group conversations", Category: "navigation", Scope: ScopeAny, Execute: func(s *Session) tea.Cmd { diff --git a/internal/tui/workspace/data/hub_global.go b/internal/tui/workspace/data/hub_global.go index 6e9754826..a37970e02 100644 --- a/internal/tui/workspace/data/hub_global.go +++ b/internal/tui/workspace/data/hub_global.go @@ -191,7 +191,8 @@ func (h *Hub) Assignments() *Pool[[]AssignmentInfo] { return p } -// PingRooms returns a global-scope pool of 1:1 chat threads. +// PingRooms returns a global-scope pool of the caller's pings, direct and +// group, most recently active first. func (h *Hub) PingRooms() *Pool[[]PingRoomInfo] { p := RealmPool(h.Global(), "ping-rooms", func() *Pool[[]PingRoomInfo] { return NewPool("ping-rooms", PoolConfig{ @@ -206,47 +207,7 @@ func (h *Hub) PingRooms() *Pool[[]PingRoomInfo] { results := FanOut[[]PingRoomInfo](ctx, h.multi, func(acct AccountInfo, client *basecamp.AccountClient) ([]PingRoomInfo, error) { - campfires, err := client.Campfires().List(ctx, nil) - if err != nil { - return nil, err - } - var rooms []PingRoomInfo - for _, cf := range campfires.Campfires { - if cf.Bucket != nil && cf.Bucket.Type == "Project" { - continue - } - var lastMsg, lastAt string - var lastAtTS int64 - lines, err := client.Campfires().ListLines(ctx, cf.ID, nil) - if err == nil && len(lines.Lines) > 0 { - last := lines.Lines[len(lines.Lines)-1] - if last.Creator != nil { - lastMsg = last.Creator.Name + ": " - } - content := last.Content - if r := []rune(content); len(r) > 40 { - content = string(r[:37]) + "…" - } - lastMsg += content - lastAt = last.CreatedAt.Format("Jan 2 3:04pm") - lastAtTS = last.CreatedAt.Unix() - } - var projectID int64 - if cf.Bucket != nil { - projectID = cf.Bucket.ID - } - rooms = append(rooms, PingRoomInfo{ - ChatID: cf.ID, - ProjectID: projectID, - PersonName: cf.Title, - Account: acct.Name, - AccountID: acct.ID, - LastMessage: lastMsg, - LastAt: lastAt, - LastAtTS: lastAtTS, - }) - } - return rooms, nil + return fetchAccountPingRooms(ctx, client, acct) }) var all []PingRoomInfo @@ -265,9 +226,62 @@ func (h *Hub) PingRooms() *Pool[[]PingRoomInfo] { return p } +// pingLastMessageLimit bounds how many pings get their latest line fetched per +// refresh: one request each, and the list arrives most recently updated first. +const pingLastMessageLimit = 20 + +// fetchAccountPingRooms lists an account's pings. A ping is a bucket of its +// own, so the room's project is the ping and its chat is the ping's Campfire. +func fetchAccountPingRooms(ctx context.Context, client *basecamp.AccountClient, acct AccountInfo) ([]PingRoomInfo, error) { + pings, err := client.Pings().List(ctx) + if err != nil { + return nil, err + } + + rooms := make([]PingRoomInfo, 0, len(pings)) + for i := range pings { + ping := &pings[i] + chatID, ok := ping.CampfireID() + if !ok { + continue + } + room := PingRoomInfo{ + ChatID: chatID, + ProjectID: ping.ID, + Title: ping.Title, + Account: acct.Name, + AccountID: acct.ID, + LastAt: ping.UpdatedAt.Format("Jan 2 3:04pm"), + LastAtTS: ping.UpdatedAt.Unix(), + } + if i < pingLastMessageLimit { + lines, err := client.Campfires().ListLines(ctx, chatID, &basecamp.CampfireLineListOptions{ + Sort: "created_at", + Direction: "desc", + Limit: 1, + }) + if err == nil && len(lines.Lines) > 0 { + last := lines.Lines[0] + if last.Creator != nil { + room.LastMessage = last.Creator.Name + ": " + } + content := last.Content + if r := []rune(content); len(r) > 40 { + content = string(r[:37]) + "…" + } + room.LastMessage += content + room.LastAt = last.CreatedAt.Format("Jan 2 3:04pm") + room.LastAtTS = last.CreatedAt.Unix() + } + } + rooms = append(rooms, room) + } + return rooms, nil +} + // BonfireRooms returns a global-scope pool of chat rooms from bookmarked // and recently visited projects. Uses FanOut like PingRooms but filters for -// project chats (not 1:1 pings). For accounts with no bookmarks, falls back +// project chats (not pings). For accounts with no bookmarks, falls back // to recently visited projects (from recents store). The RoomStore (if configured // via SetRoomStore) can further narrow or widen via explicit includes/excludes. func (h *Hub) BonfireRooms() *Pool[[]BonfireRoomConfig] { diff --git a/internal/tui/workspace/data/ping_rooms_test.go b/internal/tui/workspace/data/ping_rooms_test.go new file mode 100644 index 000000000..13ef47b04 --- /dev/null +++ b/internal/tui/workspace/data/ping_rooms_test.go @@ -0,0 +1,64 @@ +package data + +import ( + "context" + "io" + "net/http" + "strconv" + "strings" + "testing" + + "github.com/basecamp/basecamp-sdk/go/pkg/basecamp" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type pingRoomsTransport struct{} + +func (m *pingRoomsTransport) RoundTrip(req *http.Request) (*http.Response, error) { + header := make(http.Header) + header.Set("Content-Type", "application/json") + reply := func(status int, body string) (*http.Response, error) { + return &http.Response{StatusCode: status, Body: io.NopCloser(strings.NewReader(body)), Header: header, Request: req}, nil + } + + ping := func(id, campfire int, title string, named bool) string { + return `{"id":` + strconv.Itoa(id) + `,"created_at":"2026-10-19T15:00:00Z","updated_at":"2026-10-19T15:30:00Z","title":"` + title + `","named":` + strconv.FormatBool(named) + + `,"avatar_url":"x","url":"x","app_url":"x","bookmark_url":"x",` + + `"room_url":"https://3.basecampapi.com/99999/buckets/` + strconv.Itoa(id) + `/chats/` + strconv.Itoa(campfire) + `.json",` + + `"links":{"lines":"x","files":"x"},"people":[]}` + } + + switch { + case req.URL.Path == "/99999/circles.json": + return reply(200, "["+ping(500, 700, "Jason Fried", false)+","+ping(501, 701, "Jane, John", false)+"]") + case strings.HasSuffix(req.URL.Path, "/chats/700/lines.json"): + return reply(200, `[{"id":9,"content":"Lunch?","creator":{"name":"Jason Fried"},"created_at":"2026-10-19T15:45:00Z"}]`) + case strings.HasSuffix(req.URL.Path, "/chats/701/lines.json"): + return reply(200, `[]`) + } + return reply(404, `{"error":"Not Found"}`) +} + +func TestFetchAccountPingRoomsBuildsRoomsFromPings(t *testing.T) { + sdk := basecamp.NewClient(&basecamp.Config{BaseURL: "https://3.basecampapi.com"}, hubCheckinsTestTokenProvider{}, + basecamp.WithTransport(&pingRoomsTransport{}), + basecamp.WithMaxRetries(1), + ) + client := sdk.ForAccount("99999") + + rooms, err := fetchAccountPingRooms(context.Background(), client, AccountInfo{ID: "99999", Name: "Acme"}) + require.NoError(t, err) + require.Len(t, rooms, 2) + + assert.Equal(t, int64(700), rooms[0].ChatID) + assert.Equal(t, int64(500), rooms[0].ProjectID, "a ping is its own bucket") + assert.Equal(t, "Jason Fried", rooms[0].Title) + assert.Equal(t, "Jason Fried: Lunch?", rooms[0].LastMessage) + assert.Equal(t, "Acme", rooms[0].Account) + + assert.Equal(t, int64(701), rooms[1].ChatID) + assert.Equal(t, "Jane, John", rooms[1].Title, "group pings are listed too") + assert.Empty(t, rooms[1].LastMessage) + assert.NotZero(t, rooms[1].LastAtTS, "falls back to the ping's own activity time") +} diff --git a/internal/tui/workspace/data/types.go b/internal/tui/workspace/data/types.go index feec3554c..7fe226ebf 100644 --- a/internal/tui/workspace/data/types.go +++ b/internal/tui/workspace/data/types.go @@ -221,11 +221,12 @@ type ProjectInfo struct { Dock []DockToolInfo } -// PingRoomInfo represents a 1:1 chat thread. +// PingRoomInfo represents a ping, direct or group. A ping is its own bucket, so +// ProjectID is the ping's id and ChatID its Campfire. type PingRoomInfo struct { ChatID int64 ProjectID int64 - PersonName string + Title string Account string AccountID string LastMessage string diff --git a/internal/tui/workspace/views/bonfire_sidebar.go b/internal/tui/workspace/views/bonfire_sidebar.go index 068cc0628..ecb5d7396 100644 --- a/internal/tui/workspace/views/bonfire_sidebar.go +++ b/internal/tui/workspace/views/bonfire_sidebar.go @@ -16,7 +16,7 @@ import ( ) // BonfireSidebar is a compact live chat/ping panel designed for the sidebar. -// Shows chat rooms with activity and 1:1 pings, grouped and sorted by +// Shows chat rooms with activity and pings, grouped and sorted by // recency. Replaces the single-line ticker with a scannable, interactive list. type BonfireSidebar struct { session *workspace.Session @@ -329,7 +329,7 @@ func (b *BonfireSidebar) renderPingItem(p data.PingRoomInfo, selected bool, them } bullet := lipgloss.NewStyle().Foreground(theme.Secondary).Render("\u25cb") - name := truncateRunes(p.PersonName, maxW-8) + name := truncateRunes(p.Title, maxW-8) elapsed := relativeTime(p.LastAtTS) nameStyle := lipgloss.NewStyle() diff --git a/internal/tui/workspace/views/pings.go b/internal/tui/workspace/views/pings.go index aba25020d..6dae91c62 100644 --- a/internal/tui/workspace/views/pings.go +++ b/internal/tui/workspace/views/pings.go @@ -15,9 +15,9 @@ import ( "github.com/basecamp/basecamp-cli/internal/tui/workspace/widget" ) -// Pings shows 1:1 chat threads across all accounts. -// Discovery: list all chats per account, identify 1:1 rooms, -// fetch the latest line from each. +// Pings shows the caller's pings, direct and group, across all accounts. +// Discovery: list each account's pings, then fetch the latest line of the +// most recent ones. type Pings struct { session *workspace.Session pool *data.Pool[[]data.PingRoomInfo] @@ -184,7 +184,7 @@ func (v *Pings) syncRooms(rooms []workspace.PingRoomInfo) { v.roomMeta[id] = r items = append(items, widget.ListItem{ ID: id, - Title: r.PersonName, + Title: r.Title, Description: r.LastMessage, Extra: accountExtra(accounts, r.AccountID, r.LastAt), }) @@ -205,7 +205,7 @@ func (v *Pings) openSelected() tea.Cmd { return nil } - // Navigate to chat view for this 1:1 room + // Navigate to the chat view for this ping scope := v.session.Scope() scope.AccountID = meta.AccountID scope.ProjectID = meta.ProjectID diff --git a/internal/tui/workspace/views/sort_test.go b/internal/tui/workspace/views/sort_test.go index 6a48ae5ee..d5d8a2277 100644 --- a/internal/tui/workspace/views/sort_test.go +++ b/internal/tui/workspace/views/sort_test.go @@ -31,16 +31,16 @@ func TestPingRoomSortByTimestamp(t *testing.T) { // Verify that sorting uses numeric timestamps, not formatted strings. // "Dec 31 11:59pm" > "Jan 1 12:00am" lexicographically but Dec 31 is earlier. rooms := []workspace.PingRoomInfo{ - {ChatID: 1, PersonName: "Alice", LastAt: "Dec 31 11:59pm", LastAtTS: 1704067140}, - {ChatID: 2, PersonName: "Bob", LastAt: "Jan 1 12:00am", LastAtTS: 1704067200}, - {ChatID: 3, PersonName: "Carol", LastAt: "Jan 2 9:00am", LastAtTS: 1704186000}, + {ChatID: 1, Title: "Alice", LastAt: "Dec 31 11:59pm", LastAtTS: 1704067140}, + {ChatID: 2, Title: "Bob", LastAt: "Jan 1 12:00am", LastAtTS: 1704067200}, + {ChatID: 3, Title: "Carol", LastAt: "Jan 2 9:00am", LastAtTS: 1704186000}, } sort.Slice(rooms, func(i, j int) bool { return rooms[i].LastAtTS > rooms[j].LastAtTS }) - assert.Equal(t, "Carol", rooms[0].PersonName, "Jan 2 should be most recent") - assert.Equal(t, "Bob", rooms[1].PersonName, "Jan 1 should be second") - assert.Equal(t, "Alice", rooms[2].PersonName, "Dec 31 should be oldest") + assert.Equal(t, "Carol", rooms[0].Title, "Jan 2 should be most recent") + assert.Equal(t, "Bob", rooms[1].Title, "Jan 1 should be second") + assert.Equal(t, "Alice", rooms[2].Title, "Dec 31 should be oldest") } From 8938497263173c93ffdfa21ad92419cebf927cca Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Mon, 19 Oct 2026 11:50:13 -0400 Subject: [PATCH 4/7] Add the pings commands basecamp pings list, show, start (alias with), messages, post and upload cover a Ping: a private conversation, direct or group, held in a Circle bucket with one Campfire. A is an id, a ping URL, or a person; a bare number is always a ping id. start opens or reuses (the API returns the existing ping), reporting people Basecamp left out and the 403 and 422 refusals. messages, post and upload run through chat's own code on the ping's Campfire: chat's post, messages and upload now take a resolved Campfire, the commands that continue from it, and a mention resolver, so @mentions in a ping resolve among its people. show and chat line take ping URLs (circles/{id}, circles/{id}@{line}); a bucket with no dock that is a Ping now resolves to its Campfire, so chat messages --in works; chat list --all says pings are not in it; and the skill, README, API coverage, surface and smoke coverage follow. --- .surface | 396 +++++++++++++++++ API-COVERAGE.md | 9 +- README.md | 1 + e2e/pings.bats | 71 +++ e2e/smoke/smoke_aliases.bats | 8 + e2e/smoke/smoke_pings.bats | 51 +++ internal/cli/help.go | 2 +- internal/cli/root.go | 1 + internal/commands/chat.go | 267 ++++++++---- internal/commands/chat_test.go | 3 + internal/commands/commands.go | 1 + internal/commands/commands_test.go | 1 + internal/commands/helpers.go | 14 +- internal/commands/pings.go | 629 +++++++++++++++++++++++++++ internal/commands/pings_test.go | 564 ++++++++++++++++++++++++ internal/commands/show.go | 24 + internal/commands/show_test.go | 12 +- internal/names/mention.go | 19 + internal/names/ping_people_test.go | 87 ++++ internal/names/resolver.go | 27 ++ internal/presenter/schemas/ping.yaml | 52 +++ skills/basecamp/SKILL.md | 28 ++ 22 files changed, 2164 insertions(+), 103 deletions(-) create mode 100644 e2e/pings.bats create mode 100644 e2e/smoke/smoke_pings.bats create mode 100644 internal/commands/pings.go create mode 100644 internal/commands/pings_test.go create mode 100644 internal/names/ping_people_test.go create mode 100644 internal/presenter/schemas/ping.yaml diff --git a/.surface b/.surface index f305e3a15..d21d9d951 100644 --- a/.surface +++ b/.surface @@ -303,6 +303,22 @@ ARG basecamp people out-of-office 00 [me] ARG basecamp people remove 00 ... ARG basecamp people show 00 ARG basecamp people update 00 [me] +ARG basecamp ping messages 00 +ARG basecamp ping post 00 +ARG basecamp ping post 01 +ARG basecamp ping show 00 +ARG basecamp ping start 00 ... +ARG basecamp ping upload 00 +ARG basecamp ping upload 01 +ARG basecamp ping with 00 ... +ARG basecamp pings messages 00 +ARG basecamp pings post 00 +ARG basecamp pings post 01 +ARG basecamp pings show 00 +ARG basecamp pings start 00 ... +ARG basecamp pings upload 00 +ARG basecamp pings upload 01 +ARG basecamp pings with 00 ... ARG basecamp profile create 00 ARG basecamp profile delete 00 ARG basecamp profile set-default 00 @@ -970,6 +986,22 @@ CMD basecamp people pingable CMD basecamp people remove CMD basecamp people show CMD basecamp people update +CMD basecamp ping +CMD basecamp ping list +CMD basecamp ping messages +CMD basecamp ping post +CMD basecamp ping show +CMD basecamp ping start +CMD basecamp ping upload +CMD basecamp ping with +CMD basecamp pings +CMD basecamp pings list +CMD basecamp pings messages +CMD basecamp pings post +CMD basecamp pings show +CMD basecamp pings start +CMD basecamp pings upload +CMD basecamp pings with CMD basecamp profile CMD basecamp profile create CMD basecamp profile delete @@ -12443,6 +12475,354 @@ FLAG basecamp people update --time-zone type=string FLAG basecamp people update --title type=string FLAG basecamp people update --todolist type=string FLAG basecamp people update --verbose type=count +FLAG basecamp ping --account type=string +FLAG basecamp ping --agent type=bool +FLAG basecamp ping --cache-dir type=string +FLAG basecamp ping --count type=bool +FLAG basecamp ping --help type=bool +FLAG basecamp ping --hints type=bool +FLAG basecamp ping --ids-only type=bool +FLAG basecamp ping --in type=string +FLAG basecamp ping --jq type=string +FLAG basecamp ping --json type=bool +FLAG basecamp ping --markdown type=bool +FLAG basecamp ping --md type=bool +FLAG basecamp ping --no-hints type=bool +FLAG basecamp ping --no-stats type=bool +FLAG basecamp ping --profile type=string +FLAG basecamp ping --project type=string +FLAG basecamp ping --quiet type=bool +FLAG basecamp ping --stats type=bool +FLAG basecamp ping --styled type=bool +FLAG basecamp ping --todolist type=string +FLAG basecamp ping --verbose type=count +FLAG basecamp ping list --account type=string +FLAG basecamp ping list --agent type=bool +FLAG basecamp ping list --cache-dir type=string +FLAG basecamp ping list --count type=bool +FLAG basecamp ping list --help type=bool +FLAG basecamp ping list --hints type=bool +FLAG basecamp ping list --ids-only type=bool +FLAG basecamp ping list --in type=string +FLAG basecamp ping list --jq type=string +FLAG basecamp ping list --json type=bool +FLAG basecamp ping list --markdown type=bool +FLAG basecamp ping list --md type=bool +FLAG basecamp ping list --no-hints type=bool +FLAG basecamp ping list --no-stats type=bool +FLAG basecamp ping list --profile type=string +FLAG basecamp ping list --project type=string +FLAG basecamp ping list --quiet type=bool +FLAG basecamp ping list --stats type=bool +FLAG basecamp ping list --styled type=bool +FLAG basecamp ping list --todolist type=string +FLAG basecamp ping list --verbose type=count +FLAG basecamp ping messages --account type=string +FLAG basecamp ping messages --agent type=bool +FLAG basecamp ping messages --cache-dir type=string +FLAG basecamp ping messages --count type=bool +FLAG basecamp ping messages --help type=bool +FLAG basecamp ping messages --hints type=bool +FLAG basecamp ping messages --ids-only type=bool +FLAG basecamp ping messages --in type=string +FLAG basecamp ping messages --jq type=string +FLAG basecamp ping messages --json type=bool +FLAG basecamp ping messages --limit type=int +FLAG basecamp ping messages --markdown type=bool +FLAG basecamp ping messages --md type=bool +FLAG basecamp ping messages --no-hints type=bool +FLAG basecamp ping messages --no-stats type=bool +FLAG basecamp ping messages --profile type=string +FLAG basecamp ping messages --project type=string +FLAG basecamp ping messages --quiet type=bool +FLAG basecamp ping messages --stats type=bool +FLAG basecamp ping messages --styled type=bool +FLAG basecamp ping messages --todolist type=string +FLAG basecamp ping messages --verbose type=count +FLAG basecamp ping post --account type=string +FLAG basecamp ping post --agent type=bool +FLAG basecamp ping post --attach type=stringArray +FLAG basecamp ping post --cache-dir type=string +FLAG basecamp ping post --content type=string +FLAG basecamp ping post --content-type type=string +FLAG basecamp ping post --count type=bool +FLAG basecamp ping post --help type=bool +FLAG basecamp ping post --hints type=bool +FLAG basecamp ping post --ids-only type=bool +FLAG basecamp ping post --in type=string +FLAG basecamp ping post --jq type=string +FLAG basecamp ping post --json type=bool +FLAG basecamp ping post --markdown type=bool +FLAG basecamp ping post --md type=bool +FLAG basecamp ping post --no-hints type=bool +FLAG basecamp ping post --no-stats type=bool +FLAG basecamp ping post --profile type=string +FLAG basecamp ping post --project type=string +FLAG basecamp ping post --quiet type=bool +FLAG basecamp ping post --stats type=bool +FLAG basecamp ping post --styled type=bool +FLAG basecamp ping post --todolist type=string +FLAG basecamp ping post --verbose type=count +FLAG basecamp ping show --account type=string +FLAG basecamp ping show --agent type=bool +FLAG basecamp ping show --cache-dir type=string +FLAG basecamp ping show --count type=bool +FLAG basecamp ping show --help type=bool +FLAG basecamp ping show --hints type=bool +FLAG basecamp ping show --ids-only type=bool +FLAG basecamp ping show --in type=string +FLAG basecamp ping show --jq type=string +FLAG basecamp ping show --json type=bool +FLAG basecamp ping show --markdown type=bool +FLAG basecamp ping show --md type=bool +FLAG basecamp ping show --no-hints type=bool +FLAG basecamp ping show --no-stats type=bool +FLAG basecamp ping show --profile type=string +FLAG basecamp ping show --project type=string +FLAG basecamp ping show --quiet type=bool +FLAG basecamp ping show --stats type=bool +FLAG basecamp ping show --styled type=bool +FLAG basecamp ping show --todolist type=string +FLAG basecamp ping show --verbose type=count +FLAG basecamp ping start --account type=string +FLAG basecamp ping start --agent type=bool +FLAG basecamp ping start --cache-dir type=string +FLAG basecamp ping start --count type=bool +FLAG basecamp ping start --help type=bool +FLAG basecamp ping start --hints type=bool +FLAG basecamp ping start --ids-only type=bool +FLAG basecamp ping start --in type=string +FLAG basecamp ping start --jq type=string +FLAG basecamp ping start --json type=bool +FLAG basecamp ping start --markdown type=bool +FLAG basecamp ping start --md type=bool +FLAG basecamp ping start --new-group type=bool +FLAG basecamp ping start --no-hints type=bool +FLAG basecamp ping start --no-stats type=bool +FLAG basecamp ping start --profile type=string +FLAG basecamp ping start --project type=string +FLAG basecamp ping start --quiet type=bool +FLAG basecamp ping start --stats type=bool +FLAG basecamp ping start --styled type=bool +FLAG basecamp ping start --todolist type=string +FLAG basecamp ping start --verbose type=count +FLAG basecamp ping upload --account type=string +FLAG basecamp ping upload --agent type=bool +FLAG basecamp ping upload --cache-dir type=string +FLAG basecamp ping upload --count type=bool +FLAG basecamp ping upload --help type=bool +FLAG basecamp ping upload --hints type=bool +FLAG basecamp ping upload --ids-only type=bool +FLAG basecamp ping upload --in type=string +FLAG basecamp ping upload --jq type=string +FLAG basecamp ping upload --json type=bool +FLAG basecamp ping upload --markdown type=bool +FLAG basecamp ping upload --md type=bool +FLAG basecamp ping upload --no-hints type=bool +FLAG basecamp ping upload --no-stats type=bool +FLAG basecamp ping upload --profile type=string +FLAG basecamp ping upload --project type=string +FLAG basecamp ping upload --quiet type=bool +FLAG basecamp ping upload --stats type=bool +FLAG basecamp ping upload --styled type=bool +FLAG basecamp ping upload --todolist type=string +FLAG basecamp ping upload --verbose type=count +FLAG basecamp ping with --account type=string +FLAG basecamp ping with --agent type=bool +FLAG basecamp ping with --cache-dir type=string +FLAG basecamp ping with --count type=bool +FLAG basecamp ping with --help type=bool +FLAG basecamp ping with --hints type=bool +FLAG basecamp ping with --ids-only type=bool +FLAG basecamp ping with --in type=string +FLAG basecamp ping with --jq type=string +FLAG basecamp ping with --json type=bool +FLAG basecamp ping with --markdown type=bool +FLAG basecamp ping with --md type=bool +FLAG basecamp ping with --new-group type=bool +FLAG basecamp ping with --no-hints type=bool +FLAG basecamp ping with --no-stats type=bool +FLAG basecamp ping with --profile type=string +FLAG basecamp ping with --project type=string +FLAG basecamp ping with --quiet type=bool +FLAG basecamp ping with --stats type=bool +FLAG basecamp ping with --styled type=bool +FLAG basecamp ping with --todolist type=string +FLAG basecamp ping with --verbose type=count +FLAG basecamp pings --account type=string +FLAG basecamp pings --agent type=bool +FLAG basecamp pings --cache-dir type=string +FLAG basecamp pings --count type=bool +FLAG basecamp pings --help type=bool +FLAG basecamp pings --hints type=bool +FLAG basecamp pings --ids-only type=bool +FLAG basecamp pings --in type=string +FLAG basecamp pings --jq type=string +FLAG basecamp pings --json type=bool +FLAG basecamp pings --markdown type=bool +FLAG basecamp pings --md type=bool +FLAG basecamp pings --no-hints type=bool +FLAG basecamp pings --no-stats type=bool +FLAG basecamp pings --profile type=string +FLAG basecamp pings --project type=string +FLAG basecamp pings --quiet type=bool +FLAG basecamp pings --stats type=bool +FLAG basecamp pings --styled type=bool +FLAG basecamp pings --todolist type=string +FLAG basecamp pings --verbose type=count +FLAG basecamp pings list --account type=string +FLAG basecamp pings list --agent type=bool +FLAG basecamp pings list --cache-dir type=string +FLAG basecamp pings list --count type=bool +FLAG basecamp pings list --help type=bool +FLAG basecamp pings list --hints type=bool +FLAG basecamp pings list --ids-only type=bool +FLAG basecamp pings list --in type=string +FLAG basecamp pings list --jq type=string +FLAG basecamp pings list --json type=bool +FLAG basecamp pings list --markdown type=bool +FLAG basecamp pings list --md type=bool +FLAG basecamp pings list --no-hints type=bool +FLAG basecamp pings list --no-stats type=bool +FLAG basecamp pings list --profile type=string +FLAG basecamp pings list --project type=string +FLAG basecamp pings list --quiet type=bool +FLAG basecamp pings list --stats type=bool +FLAG basecamp pings list --styled type=bool +FLAG basecamp pings list --todolist type=string +FLAG basecamp pings list --verbose type=count +FLAG basecamp pings messages --account type=string +FLAG basecamp pings messages --agent type=bool +FLAG basecamp pings messages --cache-dir type=string +FLAG basecamp pings messages --count type=bool +FLAG basecamp pings messages --help type=bool +FLAG basecamp pings messages --hints type=bool +FLAG basecamp pings messages --ids-only type=bool +FLAG basecamp pings messages --in type=string +FLAG basecamp pings messages --jq type=string +FLAG basecamp pings messages --json type=bool +FLAG basecamp pings messages --limit type=int +FLAG basecamp pings messages --markdown type=bool +FLAG basecamp pings messages --md type=bool +FLAG basecamp pings messages --no-hints type=bool +FLAG basecamp pings messages --no-stats type=bool +FLAG basecamp pings messages --profile type=string +FLAG basecamp pings messages --project type=string +FLAG basecamp pings messages --quiet type=bool +FLAG basecamp pings messages --stats type=bool +FLAG basecamp pings messages --styled type=bool +FLAG basecamp pings messages --todolist type=string +FLAG basecamp pings messages --verbose type=count +FLAG basecamp pings post --account type=string +FLAG basecamp pings post --agent type=bool +FLAG basecamp pings post --attach type=stringArray +FLAG basecamp pings post --cache-dir type=string +FLAG basecamp pings post --content type=string +FLAG basecamp pings post --content-type type=string +FLAG basecamp pings post --count type=bool +FLAG basecamp pings post --help type=bool +FLAG basecamp pings post --hints type=bool +FLAG basecamp pings post --ids-only type=bool +FLAG basecamp pings post --in type=string +FLAG basecamp pings post --jq type=string +FLAG basecamp pings post --json type=bool +FLAG basecamp pings post --markdown type=bool +FLAG basecamp pings post --md type=bool +FLAG basecamp pings post --no-hints type=bool +FLAG basecamp pings post --no-stats type=bool +FLAG basecamp pings post --profile type=string +FLAG basecamp pings post --project type=string +FLAG basecamp pings post --quiet type=bool +FLAG basecamp pings post --stats type=bool +FLAG basecamp pings post --styled type=bool +FLAG basecamp pings post --todolist type=string +FLAG basecamp pings post --verbose type=count +FLAG basecamp pings show --account type=string +FLAG basecamp pings show --agent type=bool +FLAG basecamp pings show --cache-dir type=string +FLAG basecamp pings show --count type=bool +FLAG basecamp pings show --help type=bool +FLAG basecamp pings show --hints type=bool +FLAG basecamp pings show --ids-only type=bool +FLAG basecamp pings show --in type=string +FLAG basecamp pings show --jq type=string +FLAG basecamp pings show --json type=bool +FLAG basecamp pings show --markdown type=bool +FLAG basecamp pings show --md type=bool +FLAG basecamp pings show --no-hints type=bool +FLAG basecamp pings show --no-stats type=bool +FLAG basecamp pings show --profile type=string +FLAG basecamp pings show --project type=string +FLAG basecamp pings show --quiet type=bool +FLAG basecamp pings show --stats type=bool +FLAG basecamp pings show --styled type=bool +FLAG basecamp pings show --todolist type=string +FLAG basecamp pings show --verbose type=count +FLAG basecamp pings start --account type=string +FLAG basecamp pings start --agent type=bool +FLAG basecamp pings start --cache-dir type=string +FLAG basecamp pings start --count type=bool +FLAG basecamp pings start --help type=bool +FLAG basecamp pings start --hints type=bool +FLAG basecamp pings start --ids-only type=bool +FLAG basecamp pings start --in type=string +FLAG basecamp pings start --jq type=string +FLAG basecamp pings start --json type=bool +FLAG basecamp pings start --markdown type=bool +FLAG basecamp pings start --md type=bool +FLAG basecamp pings start --new-group type=bool +FLAG basecamp pings start --no-hints type=bool +FLAG basecamp pings start --no-stats type=bool +FLAG basecamp pings start --profile type=string +FLAG basecamp pings start --project type=string +FLAG basecamp pings start --quiet type=bool +FLAG basecamp pings start --stats type=bool +FLAG basecamp pings start --styled type=bool +FLAG basecamp pings start --todolist type=string +FLAG basecamp pings start --verbose type=count +FLAG basecamp pings upload --account type=string +FLAG basecamp pings upload --agent type=bool +FLAG basecamp pings upload --cache-dir type=string +FLAG basecamp pings upload --count type=bool +FLAG basecamp pings upload --help type=bool +FLAG basecamp pings upload --hints type=bool +FLAG basecamp pings upload --ids-only type=bool +FLAG basecamp pings upload --in type=string +FLAG basecamp pings upload --jq type=string +FLAG basecamp pings upload --json type=bool +FLAG basecamp pings upload --markdown type=bool +FLAG basecamp pings upload --md type=bool +FLAG basecamp pings upload --no-hints type=bool +FLAG basecamp pings upload --no-stats type=bool +FLAG basecamp pings upload --profile type=string +FLAG basecamp pings upload --project type=string +FLAG basecamp pings upload --quiet type=bool +FLAG basecamp pings upload --stats type=bool +FLAG basecamp pings upload --styled type=bool +FLAG basecamp pings upload --todolist type=string +FLAG basecamp pings upload --verbose type=count +FLAG basecamp pings with --account type=string +FLAG basecamp pings with --agent type=bool +FLAG basecamp pings with --cache-dir type=string +FLAG basecamp pings with --count type=bool +FLAG basecamp pings with --help type=bool +FLAG basecamp pings with --hints type=bool +FLAG basecamp pings with --ids-only type=bool +FLAG basecamp pings with --in type=string +FLAG basecamp pings with --jq type=string +FLAG basecamp pings with --json type=bool +FLAG basecamp pings with --markdown type=bool +FLAG basecamp pings with --md type=bool +FLAG basecamp pings with --new-group type=bool +FLAG basecamp pings with --no-hints type=bool +FLAG basecamp pings with --no-stats type=bool +FLAG basecamp pings with --profile type=string +FLAG basecamp pings with --project type=string +FLAG basecamp pings with --quiet type=bool +FLAG basecamp pings with --stats type=bool +FLAG basecamp pings with --styled type=bool +FLAG basecamp pings with --todolist type=string +FLAG basecamp pings with --verbose type=count FLAG basecamp profile --account type=string FLAG basecamp profile --agent type=bool FLAG basecamp profile --cache-dir type=string @@ -19294,6 +19674,22 @@ SUB basecamp people pingable SUB basecamp people remove SUB basecamp people show SUB basecamp people update +SUB basecamp ping +SUB basecamp ping list +SUB basecamp ping messages +SUB basecamp ping post +SUB basecamp ping show +SUB basecamp ping start +SUB basecamp ping upload +SUB basecamp ping with +SUB basecamp pings +SUB basecamp pings list +SUB basecamp pings messages +SUB basecamp pings post +SUB basecamp pings show +SUB basecamp pings start +SUB basecamp pings upload +SUB basecamp pings with SUB basecamp profile SUB basecamp profile create SUB basecamp profile delete diff --git a/API-COVERAGE.md b/API-COVERAGE.md index c7470924a..77af1740e 100644 --- a/API-COVERAGE.md +++ b/API-COVERAGE.md @@ -6,12 +6,14 @@ Coverage of Basecamp 3 API endpoints. Source: [bc3-api/sections](https://github. | Status | Sections | Endpoints | |--------|----------|-----------| -| ✅ Implemented | 52 | 204 | +| ✅ Implemented | 53 | 207 | | ⚠️ Blocked | 0 | 0 | | ⏭️ Out of scope | 4 | 12 | -| **Total tracked** | **56** | **216** | +| **Total tracked** | **57** | **219** | -**204 of 204 tracked in-scope endpoints.** Moving a document, upload or folder +**207 of 207 tracked in-scope endpoints.** The three Pings endpoints bc3#13943 +documented — `GET /circles.json`, `GET /circles/:id.json` and `POST +/circles.json` — land as `pings`. Moving a document, upload or folder between folders (`POST /recordings/:id/filing.json`, documented by bc3#13644 and modelled by basecamp/basecamp-sdk#960) lands as `files move`. The eight subtask endpoints bc3 #12659 documented — the flat `/recordings/:id/subtasks.json` and @@ -209,6 +211,7 @@ cannot faithfully cover at least one endpoint for a reason outside the CLI. A | message_boards | 3 | `messageboards` | ✅ | BC4 | - | Container, accessed via project dock | | message_types | 5 | `messagetypes` | ✅ | BC4 | - | list, show, create, update, delete. Bucket-scoped (`/buckets/{id}/categories…`); commands are project-scoped via `--in`/`--project` | | campfires | 14 | `chat` | ✅ | BC4 | - | list, messages, post, line show/update/delete. @mentions in content | +| pings | 3 | `pings` | ✅ | BC5 | - | list (`GET /circles.json`: active pings with at least one line, newest first, unpaginated), show (`GET /circles/:id.json`, archived included), start (`POST /circles.json`, which opens or reuses: an active direct ping is always reused, a group with the same people unless `--new-group`). A ping is a Circle bucket with one Campfire, so `messages`, `post` and `upload` run through the Campfire lines endpoints on the ping's `campfire_id`, with @mentions resolved among the ping's people. `` is an id, a ping URL, or a person (a direct ping: read from the list, or started when writing). `show` and `chat line` take ping URLs (`/circles/:id`, `/circles/:id@:line`). 403 `Pings have been disabled` and 422 (an agent starting a ping with nobody who can ping) surface as errors. Requires basecamp/bc3#13943 | | comments | 8 | `comment`, `comments` | ✅ | BC4 | - | list, show, thread, create, update. @mentions in content. `show` surfaces `reply_target` + paste-ready `mention` from its single Get (no new calls). `thread` composes Get + parent recording (via type endpoint) + List into a deterministic reply-ready context (no new endpoints) | | boosts | 6 | `boost`, `react` | ✅ | BC4 | - | list (recording + event), show, create (recording + event), delete. No account-wide listing — BC5 withdrew `/boosts.json` (basecamp/bc3#12464); temporary, returns via basecamp/bc3#12463 | | notifications | 2 | `notifications` | ✅ | BC4 | - | list, mark as read (BC5: `bubble_ups`/`scheduled_bubble_ups` sections; `memories` is BC4-only) | diff --git a/README.md b/README.md index 3ecf2f7be..c6d3c0b1a 100644 --- a/README.md +++ b/README.md @@ -127,6 +127,7 @@ basecamp search "authentication" # Search across projects basecamp files list --in 12345 # List docs & files basecamp cards list --in 12345 # List cards (Kanban) basecamp chat post "Hello" --in 12345 # Post to chat +basecamp pings post Jane "Lunch?" # Ping someone directly basecamp comments create 67890 "@Jane.Smith, done!" # Comment with @mention ``` diff --git a/e2e/pings.bats b/e2e/pings.bats new file mode 100644 index 000000000..fae488d61 --- /dev/null +++ b/e2e/pings.bats @@ -0,0 +1,71 @@ +#!/usr/bin/env bats +# pings.bats - Test pings command input handling + +load test_helper + + +# Help + +@test "pings without subcommand shows help" { + run basecamp pings + assert_success + assert_output_contains "COMMANDS" + assert_output_contains "start" +} + +@test "ping is an alias of pings" { + run basecamp ping + assert_success + assert_output_contains "COMMANDS" +} + +@test "pings list help says when a ping appears" { + run basecamp pings list --help + assert_success + assert_output_contains "once someone has posted" +} + +@test "chat list help points to pings" { + run basecamp chat list --help + assert_success + assert_output_contains "basecamp pings list" +} + + +# Input errors that need no network + +@test "pings start --new-group needs two people" { + create_credentials + create_global_config '{"account_id": 99999}' + + run basecamp pings start Jane --new-group + assert_failure + assert_output_contains "--new-group needs two or more people" +} + +@test "pings show rejects a URL that is not a ping" { + create_credentials + create_global_config '{"account_id": 99999}' + + run basecamp pings show https://3.basecamp.com/99999/buckets/1/todos/2 + assert_failure + assert_output_contains "expected a ping URL" +} + +@test "pings show rejects a URL on an untrusted host" { + create_credentials + create_global_config '{"account_id": 99999}' + + run basecamp pings show https://evil.example.com/99999/circles/1 + assert_failure + assert_output_contains "untrusted host" +} + +@test "pings messages --limit without value shows error" { + create_credentials + create_global_config '{"account_id": 99999}' + + run basecamp pings messages 1 --limit + assert_failure + assert_output_contains "--limit requires a value" +} diff --git a/e2e/smoke/smoke_aliases.bats b/e2e/smoke/smoke_aliases.bats index 9b37e0bfd..d60eea23b 100644 --- a/e2e/smoke/smoke_aliases.bats +++ b/e2e/smoke/smoke_aliases.bats @@ -35,6 +35,14 @@ load smoke_helper mark_out_of_scope "Alias for messages — tested via canonical form" } +@test "ping is out of scope" { + mark_out_of_scope "Alias for pings — tested via canonical form" +} + +@test "pings with is out of scope" { + mark_out_of_scope "Alias for pings start — tested via canonical form" +} + @test "project is out of scope" { mark_out_of_scope "Alias for projects — tested via canonical form" } diff --git a/e2e/smoke/smoke_pings.bats b/e2e/smoke/smoke_pings.bats new file mode 100644 index 000000000..fc8d78172 --- /dev/null +++ b/e2e/smoke/smoke_pings.bats @@ -0,0 +1,51 @@ +#!/usr/bin/env bats +# smoke_pings.bats - Level 0: Pings (private conversations) + +load smoke_helper + +setup_file() { + ensure_token || return 1 +} + +@test "pings list returns pings" { + run_smoke basecamp pings list --json + assert_success + assert_json_value '.ok' 'true' + + echo "$output" | jq -r '.data[0].id // empty' > "$BATS_FILE_TMPDIR/ping_id" +} + +@test "pings show returns a ping" { + local id_file="$BATS_FILE_TMPDIR/ping_id" + [[ -s "$id_file" ]] || mark_unverifiable "No pings to show" + local ping_id + ping_id=$(<"$id_file") + + run_smoke basecamp pings show "$ping_id" --json + assert_success + assert_json_value '.ok' 'true' + assert_json_not_null '.data.campfire_id' +} + +@test "pings messages returns lines" { + local id_file="$BATS_FILE_TMPDIR/ping_id" + [[ -s "$id_file" ]] || mark_unverifiable "No pings to read" + local ping_id + ping_id=$(<"$id_file") + + run_smoke basecamp pings messages "$ping_id" --limit 1 --json + assert_success + assert_json_value '.ok' 'true' +} + +@test "pings start is out of scope" { + mark_out_of_scope "Mutating - opens or creates a ping with a real person; covered by the live round-trip" +} + +@test "pings post is out of scope" { + mark_out_of_scope "Mutating - posts into a private conversation with a real person; covered by the live round-trip" +} + +@test "pings upload is out of scope" { + mark_out_of_scope "Mutating - uploads into a private conversation with a real person; covered by the live round-trip" +} diff --git a/internal/cli/help.go b/internal/cli/help.go index 5ea87c669..f31a7a3bd 100644 --- a/internal/cli/help.go +++ b/internal/cli/help.go @@ -99,7 +99,7 @@ var curatedCategories = []struct { { heading: "CORE COMMANDS", category: "Core Commands", - include: map[string]bool{"projects": true, "todos": true, "todolists": true, "messages": true, "chat": true, "cards": true, "files": true, "checkins": true, "schedule": true}, + include: map[string]bool{"projects": true, "todos": true, "todolists": true, "messages": true, "chat": true, "pings": true, "cards": true, "files": true, "checkins": true, "schedule": true}, }, { heading: "SHORTCUTS", diff --git a/internal/cli/root.go b/internal/cli/root.go index ecd83fff7..9c34bfd5e 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -325,6 +325,7 @@ func Execute() { cmd.AddCommand(commands.NewSearchCmd()) cmd.AddCommand(commands.NewRecordingsCmd()) cmd.AddCommand(commands.NewChatCmd()) + cmd.AddCommand(commands.NewPingsCmd()) cmd.AddCommand(commands.NewScheduleCmd()) cmd.AddCommand(commands.NewFilesCmd()) cmd.AddCommand(commands.NewVaultsCmd()) diff --git a/internal/commands/chat.go b/internal/commands/chat.go index bcc930e45..c0c060846 100644 --- a/internal/commands/chat.go +++ b/internal/commands/chat.go @@ -1,6 +1,7 @@ package commands import ( + "context" "errors" "fmt" "os" @@ -60,7 +61,10 @@ func newChatListCmd(project, chatID *string) *cobra.Command { cmd := &cobra.Command{ Use: "list", Short: "List chats", - Long: "List chats in a project or account-wide with --all.", + Long: `List chats in a project or account-wide with --all. + +These are project chats. Pings, private conversations with people, are not +chats of a project and are not listed here: use 'basecamp pings list'.`, RunE: func(cmd *cobra.Command, args []string) error { app := appctx.FromContext(cmd.Context()) if err := ensureAccount(cmd, app); err != nil { @@ -88,6 +92,7 @@ func runChatList(cmd *cobra.Command, app *appctx.App, project, chatID string, al return app.OK(chats, output.WithSummary(summary), + output.WithNotice("These are project chats. Pings are not listed here; see: basecamp pings list"), output.WithBreadcrumbs( output.Breadcrumb{ Action: "messages", @@ -257,12 +262,55 @@ func runChatMessages(cmd *cobra.Command, app *appctx.App, chatID, project string return output.ErrUsage("Invalid chat room ID") } + return showChatMessages(cmd, app, chatIDInt, limit, chatRoomNav(chatID, resolvedProjectID)) +} + +// chatNav names the commands that continue from a Campfire, so the commands +// that read and write one can send the reader on in their own vocabulary: a +// project chat continues through `chat`, a Ping through `pings`. +type chatNav struct { + messages func(limit int) string + post func(text string) string + // unresolved words the notice for @mentions left as text; nil uses the + // project chat's. + unresolved func(names []string) string +} + +func (n chatNav) unresolvedMentions(names []string) string { + if n.unresolved != nil { + return n.unresolved(names) + } + return unresolvedMentionWarning(names) +} + +// chatRoomNav continues a project chat with `chat --room`, adding --in when a +// project is known. +func chatRoomNav(roomID, projectID string) chatNav { + in := "" + if projectID != "" { + in = " --in " + projectID + } + return chatNav{ + messages: func(limit int) string { + if limit > 0 { + return fmt.Sprintf("basecamp chat messages --limit %d --room %s%s", limit, roomID, in) + } + return fmt.Sprintf("basecamp chat messages --room %s%s", roomID, in) + }, + post: func(text string) string { + return fmt.Sprintf("basecamp chat post %q --room %s%s", text, roomID, in) + }, + } +} + +// showChatMessages prints the most recent lines of a Campfire, oldest first. +func showChatMessages(cmd *cobra.Command, app *appctx.App, roomID int64, limit int, nav chatNav) error { if limit < 0 { return output.ErrUsage("--limit must not be negative") } // Get recent messages (lines) using SDK, newest first - result, err := app.Account().Campfires().ListLines(cmd.Context(), chatIDInt, &basecamp.CampfireLineListOptions{ + result, err := app.Account().Campfires().ListLines(cmd.Context(), roomID, &basecamp.CampfireLineListOptions{ Sort: "created_at", Direction: "desc", Limit: limit, @@ -284,12 +332,12 @@ func runChatMessages(cmd *cobra.Command, app *appctx.App, chatID, project string output.WithBreadcrumbs( output.Breadcrumb{ Action: "post", - Cmd: fmt.Sprintf("basecamp chat post \"message\" --room %s --in %s", chatID, resolvedProjectID), + Cmd: nav.post("message"), Description: "Post message", }, output.Breadcrumb{ Action: "more", - Cmd: fmt.Sprintf("basecamp chat messages --limit 50 --room %s --in %s", chatID, resolvedProjectID), + Cmd: nav.messages(50), Description: "Load more", }, ), @@ -317,53 +365,23 @@ Use - as the message argument to read the message from stdin: RunE: func(cmd *cobra.Command, args []string) error { app := appctx.FromContext(cmd.Context()) - // Validate user input first, before checking account. A - // positional and --content are the same input, so supplying both - // is rejected rather than one silently winning — with "-" in - // play, the losing source would discard piped content unread. - messageContent := content - argIndex, what := -1, "--content" - if len(args) > 0 { - if cmd.Flags().Changed("content") { - return output.ErrUsage("cannot combine a argument with --content") - } - messageContent = args[0] - argIndex, what = 0, "" - } - // An explicitly supplied --room must be numeric; when it is absent // the room is resolved from the project, which needs the network. - // Check the supplied case here so a malformed room does not cost - // the caller a drained pipe. Same for the content mode, which chat - // update already checks before its own read. - if *chatID != "" { - if _, err := strconv.ParseInt(*chatID, 10, 64); err != nil { - return output.ErrUsage("Invalid chat room ID") + // Check the supplied case before the pipe is drained. + checkRoom := func() error { + if *chatID != "" { + if _, err := strconv.ParseInt(*chatID, 10, 64); err != nil { + return output.ErrUsage("Invalid chat room ID") + } } - } - switch *contentType { - case "", "text/html", "text/plain": - default: - return output.ErrUsage(fmt.Sprintf("unsupported --content-type %q (expected text/html or text/plain)", *contentType)) - } - - // Attachment paths are readable or not regardless of the body, so - // check them before the pipe is drained. - if err := validateAttachPaths(attachFiles); err != nil { - return err + return nil } - var err error - messageContent, err = resolveContentValue(cmd, messageContent, argIndex, what) - if err != nil { + messageContent, err := readPostInput(cmd, args, 0, content, *contentType, attachFiles, checkRoom) + if err != nil || messageContent == "" { return err } - // Show help when invoked with no message content - if strings.TrimSpace(messageContent) == "" { - return missingArg(cmd, "") - } - if err := ensureAccount(cmd, app); err != nil { return err } @@ -381,6 +399,56 @@ Use - as the message argument to read the message from stdin: return cmd } +// readPostInput validates and reads the message for a chat post. msgArgs is +// the command's positional message, at argIndex among its arguments. Everything +// decidable from the arguments is checked before stdin is read, so a bad +// invocation does not cost the caller a drained pipe; checkRoom is the +// command's own check of an explicit room, run at the same point. +// +// A positional and --content are the same input, so supplying both is rejected +// rather than one silently winning: with "-" in play, the losing source would +// discard piped content unread. +// +// A missing message shows help in a terminal, which is not an error: the result +// is then empty with a nil error, and the caller is done. +func readPostInput(cmd *cobra.Command, msgArgs []string, argIndex int, content, contentType string, attachFiles []string, checkRoom func() error) (string, error) { + messageContent := content + index, what := -1, "--content" + if len(msgArgs) > 0 { + if cmd.Flags().Changed("content") { + return "", output.ErrUsage("cannot combine a argument with --content") + } + messageContent = msgArgs[0] + index, what = argIndex, "" + } + + if err := checkRoom(); err != nil { + return "", err + } + switch contentType { + case "", "text/html", "text/plain": + default: + return "", output.ErrUsage(fmt.Sprintf("unsupported --content-type %q (expected text/html or text/plain)", contentType)) + } + + // Attachment paths are readable or not regardless of the body, so + // check them before the pipe is drained. + if err := validateAttachPaths(attachFiles); err != nil { + return "", err + } + + messageContent, err := resolveContentValue(cmd, messageContent, index, what) + if err != nil { + return "", err + } + + // Show help when invoked with no message content + if strings.TrimSpace(messageContent) == "" { + return "", missingArg(cmd, "") + } + return messageContent, nil +} + func runChatPost(cmd *cobra.Command, app *appctx.App, chatID, project, content, contentType string, attachFiles []string) error { // Resolve project only when needed (chat ID not provided, or for breadcrumbs) var resolvedProjectID string @@ -416,6 +484,17 @@ func runChatPost(cmd *cobra.Command, app *appctx.App, chatID, project, content, return output.ErrUsage("Invalid chat room ID") } + mentions := func(ctx context.Context, html string) (richtext.MentionResult, error) { + return resolveMentions(ctx, app.Names, mentionScope(app, resolvedProjectID, project, app.Flags.Project), html) + } + return postToChat(cmd, app, chatIDInt, content, contentType, attachFiles, mentions, chatRoomNav(chatID, resolvedProjectID)) +} + +// mentionResolver turns the mention syntax in HTML into mention attachments. +type mentionResolver func(ctx context.Context, html string) (richtext.MentionResult, error) + +// postToChat posts text and attachments to a Campfire. +func postToChat(cmd *cobra.Command, app *appctx.App, chatIDInt int64, content, contentType string, attachFiles []string, resolve mentionResolver, nav chatNav) error { // Resolve @mentions — skip if user explicitly set a non-HTML content type. // When contentType is unset, convert Markdown to HTML first so the mention // resolver operates on HTML input. @@ -425,7 +504,7 @@ func runChatPost(cmd *cobra.Command, app *appctx.App, chatID, project, content, if contentType == "" { mentionInput = richtext.MarkdownToHTML(content) } - result, resolveErr := resolveMentions(cmd.Context(), app.Names, mentionScope(app, resolvedProjectID, project, app.Flags.Project), mentionInput) + result, resolveErr := resolve(cmd.Context(), mentionInput) if resolveErr != nil { return resolveErr } @@ -435,7 +514,7 @@ func runChatPost(cmd *cobra.Command, app *appctx.App, chatID, project, content, contentType = "text/html" } } - mentionNotice = unresolvedMentionWarning(result.Unresolved) + mentionNotice = nav.unresolvedMentions(result.Unresolved) } // Post message using SDK @@ -493,34 +572,9 @@ func runChatPost(cmd *cobra.Command, app *appctx.App, chatID, project, content, summary = fmt.Sprintf("Posted %d attachment(s)", len(uploadIDs)) } - // Build breadcrumbs — include project context if resolved - var breadcrumbs []output.Breadcrumb - if resolvedProjectID != "" { - breadcrumbs = append(breadcrumbs, - output.Breadcrumb{ - Action: "messages", - Cmd: fmt.Sprintf("basecamp chat messages --room %s --in %s", chatID, resolvedProjectID), - Description: "View messages", - }, - output.Breadcrumb{ - Action: "post", - Cmd: fmt.Sprintf("basecamp chat post \"reply\" --room %s --in %s", chatID, resolvedProjectID), - Description: "Post another", - }, - ) - } else { - breadcrumbs = append(breadcrumbs, - output.Breadcrumb{ - Action: "messages", - Cmd: fmt.Sprintf("basecamp chat messages --room %s", chatID), - Description: "View messages", - }, - output.Breadcrumb{ - Action: "post", - Cmd: fmt.Sprintf("basecamp chat post \"reply\" --room %s", chatID), - Description: "Post another", - }, - ) + breadcrumbs := []output.Breadcrumb{ + {Action: "messages", Cmd: nav.messages(0), Description: "View messages"}, + {Action: "post", Cmd: nav.post("reply"), Description: "Post another"}, } respOpts := []output.ResponseOption{ @@ -619,6 +673,11 @@ func runChatUpload(cmd *cobra.Command, app *appctx.App, chatID, project, filePat return output.ErrUsage("Invalid chat room ID") } + return uploadToChat(cmd, app, chatIDInt, filePath, chatRoomNav(chatID, resolvedProjectID)) +} + +// uploadToChat uploads a validated local file to a Campfire as a chat line. +func uploadToChat(cmd *cobra.Command, app *appctx.App, chatIDInt int64, filePath string, nav chatNav) error { contentType := richtext.DetectMIME(filePath) filename := filepath.Base(filePath) @@ -633,26 +692,6 @@ func runChatUpload(cmd *cobra.Command, app *appctx.App, chatID, project, filePat return convertSDKError(err) } - // Build breadcrumbs - var breadcrumbs []output.Breadcrumb - if resolvedProjectID != "" { - breadcrumbs = append(breadcrumbs, - output.Breadcrumb{ - Action: "messages", - Cmd: fmt.Sprintf("basecamp chat messages --room %s --in %s", chatID, resolvedProjectID), - Description: "View messages", - }, - ) - } else { - breadcrumbs = append(breadcrumbs, - output.Breadcrumb{ - Action: "messages", - Cmd: fmt.Sprintf("basecamp chat messages --room %s", chatID), - Description: "View messages", - }, - ) - } - // Build summary — prefer attachment filename from API response over local basename uploadName := filename if len(line.Attachments) > 0 && line.Attachments[0].Filename != "" { @@ -667,7 +706,11 @@ func runChatUpload(cmd *cobra.Command, app *appctx.App, chatID, project, filePat output.WithSummary(summary), output.WithEntity("chat_line"), output.WithDisplayData(chatLineDisplayData(line)), - output.WithBreadcrumbs(breadcrumbs...), + output.WithBreadcrumbs(output.Breadcrumb{ + Action: "messages", + Cmd: nav.messages(0), + Description: "View messages", + }), ) } @@ -1261,6 +1304,10 @@ func chatLineRef(app *appctx.App, arg string) (lineID, projectID, chatID string, return "", "", "", output.ErrUsage("refusing untrusted host in URL — expected a Basecamp URL") } parsed := urlarg.Parse(arg) + if parsed != nil && parsed.PingID != "" && parsed.Type != "lines" { + return "", "", "", output.ErrUsageHint("a ping URL names a ping, not a message", + fmt.Sprintf("Show the ping with: basecamp pings show %s; a message URL ends in @", parsed.PingID)) + } if parsed == nil || parsed.Type != "lines" || parsed.IsCollection { return "", "", "", output.ErrUsage("expected a chat-line ID or URL of the form /chats/{c}/lines/{l} or /chats/{c}@{l}") } @@ -1272,7 +1319,37 @@ func chatLineRef(app *appctx.App, arg string) (lineID, projectID, chatID string, // getChatID retrieves the chat ID from a project's dock, handling multi-dock projects. func getChatID(cmd *cobra.Command, app *appctx.App, projectID string) (string, error) { - return getDockToolID(cmd.Context(), app, projectID, "chat", "", "chat room", "room") + id, err := getDockToolID(cmd.Context(), app, projectID, "chat", "", "chat room", "room") + if err != nil { + if campfireID, ok := pingCampfireID(cmd.Context(), app, projectID, err); ok { + return campfireID, nil + } + return "", err + } + return id, nil +} + +// pingCampfireID finds the Campfire of a Ping named where a project was +// expected. A Ping is a bucket without a dock, so a project lookup that failed +// as not found is retried as a Ping; any other failure stands. +func pingCampfireID(ctx context.Context, app *appctx.App, bucketID string, lookupErr error) (string, bool) { + var e *output.Error + if !errors.As(lookupErr, &e) || e.Code != output.CodeNotFound { + return "", false + } + id, err := strconv.ParseInt(bucketID, 10, 64) + if err != nil { + return "", false + } + ping, err := app.Account().Pings().Get(ctx, id) + if err != nil { + return "", false + } + campfireID, ok := ping.CampfireID() + if !ok { + return "", false + } + return strconv.FormatInt(campfireID, 10), true } // chatLineDisplayContent produces human-readable content for a campfire line, diff --git a/internal/commands/chat_test.go b/internal/commands/chat_test.go index 0cd9024fb..fefbf213e 100644 --- a/internal/commands/chat_test.go +++ b/internal/commands/chat_test.go @@ -618,6 +618,7 @@ func TestChatListAllBreadcrumbSyntax(t *testing.T) { require.NoError(t, err) var envelope struct { + Notice string `json:"notice"` Breadcrumbs []struct { Cmd string `json:"cmd"` } `json:"breadcrumbs"` @@ -629,6 +630,8 @@ func TestChatListAllBreadcrumbSyntax(t *testing.T) { assert.Contains(t, bc.Cmd, "--room") assert.NotContains(t, bc.Cmd, "chat messages") } + assert.Contains(t, envelope.Notice, "Pings are not listed here") + assert.Contains(t, envelope.Notice, "basecamp pings list") } // mockChatMessagesTransport returns a fixed set of 5 campfire lines (newest-first) diff --git a/internal/commands/commands.go b/internal/commands/commands.go index 3064a0b43..f2c5f2a4d 100644 --- a/internal/commands/commands.go +++ b/internal/commands/commands.go @@ -44,6 +44,7 @@ func CommandCategories() []CommandCategory { {Name: "todolistgroups", Category: "core", Description: "Manage to-do list groups", Actions: []string{"list", "show", "create", "update", "position"}}, {Name: "messages", Category: "core", Description: "Manage messages", Actions: []string{"list", "show", "create", "update", "publish", "pin", "unpin", "trash", "archive", "restore"}}, {Name: "chat", Category: "core", Description: "Chat in real-time", Actions: []string{"list", "messages", "post", "upload", "line", "update", "delete"}}, + {Name: "pings", Category: "core", Description: "Talk to people directly", Actions: []string{"list", "show", "start", "messages", "post", "upload"}}, {Name: "cards", Category: "core", Description: "Manage Kanban cards", Actions: []string{"list", "show", "create", "update", "move", "done", "columns", "wormholes", "steps", "trash", "archive", "restore"}}, {Name: "files", Category: "core", Description: "Manage files, documents, and folders", Actions: []string{"list", "show", "versions", "replace", "download", "update", "move", "trash", "archive", "restore"}}, {Name: "checkins", Category: "core", Description: "View automatic check-ins", Actions: []string{"questions", "question", "answers", "answer", "reminders"}}, diff --git a/internal/commands/commands_test.go b/internal/commands/commands_test.go index 64c345418..1710ef1eb 100644 --- a/internal/commands/commands_test.go +++ b/internal/commands/commands_test.go @@ -76,6 +76,7 @@ func buildRootWithAllCommands() *cobra.Command { root.AddCommand(commands.NewSearchCmd()) root.AddCommand(commands.NewRecordingsCmd()) root.AddCommand(commands.NewChatCmd()) + root.AddCommand(commands.NewPingsCmd()) root.AddCommand(commands.NewScheduleCmd()) root.AddCommand(commands.NewFilesCmd()) root.AddCommand(commands.NewVaultsCmd()) diff --git a/internal/commands/helpers.go b/internal/commands/helpers.go index 7f0aa435f..8b0869249 100644 --- a/internal/commands/helpers.go +++ b/internal/commands/helpers.go @@ -650,9 +650,19 @@ func applySubscribeFlags(ctx context.Context, resolver *names.Resolver, subscrib // left as plain text; their names are returned in the Unresolved slice. // Deterministic syntaxes (mention:SGID, person:ID) still hard-fail on error. func resolveMentions(ctx context.Context, resolver *names.Resolver, scope names.ProjectScope, html string) (richtext.MentionResult, error) { + return resolveMentionsWith(html, + func(name string) (*names.Person, error) { return resolver.ResolveMentionByName(ctx, name, scope) }, + func(id int64) (*names.Person, error) { return resolver.ResolveMentionByID(ctx, id) }, + ) +} + +// resolveMentionsWith is resolveMentions over caller-supplied lookups, for a +// command whose mentions draw on a fixed set of people rather than the +// pingable set. +func resolveMentionsWith(html string, byName func(name string) (*names.Person, error), byID func(id int64) (*names.Person, error)) (richtext.MentionResult, error) { return richtext.ResolveMentions(html, func(name string) (string, string, error) { - person, err := resolver.ResolveMentionByName(ctx, name, scope) + person, err := byName(name) // A project that cannot be resolved is the command's error, not a // missed mention: never downgrade it to plain text. var scopeErr *names.ScopeError @@ -681,7 +691,7 @@ func resolveMentions(ctx context.Context, resolver *names.Resolver, scope names. if err != nil { return "", "", output.ErrUsage(fmt.Sprintf("invalid person ID %q — must be numeric", id)) } - person, err := resolver.ResolveMentionByID(ctx, personID) + person, err := byID(personID) if err != nil { return "", "", err } diff --git a/internal/commands/pings.go b/internal/commands/pings.go new file mode 100644 index 000000000..985fac983 --- /dev/null +++ b/internal/commands/pings.go @@ -0,0 +1,629 @@ +package commands + +import ( + "context" + "errors" + "fmt" + "slices" + "strconv" + "strings" + + "github.com/basecamp/basecamp-sdk/go/pkg/basecamp" + "github.com/spf13/cobra" + + "github.com/basecamp/basecamp-cli/internal/appctx" + "github.com/basecamp/basecamp-cli/internal/hostutil" + "github.com/basecamp/basecamp-cli/internal/names" + "github.com/basecamp/basecamp-cli/internal/output" + "github.com/basecamp/basecamp-cli/internal/richtext" + "github.com/basecamp/basecamp-cli/internal/urlarg" +) + +// NewPingsCmd creates the pings command for private conversations. +func NewPingsCmd() *cobra.Command { + var contentType string + + cmd := &cobra.Command{ + Use: "pings", + Aliases: []string{"ping"}, + Short: "Talk to people directly", + Long: `Pings are private conversations between people on an account, outside any project. + +A ping with one other person is a direct ping; with two or more it is a group +ping. Each ping has one chat, and its messages are the chat's lines. + +A is a ping ID, a ping URL (https://app.basecamp.com//circles/), +or a person: their name, email address or person ID names your direct ping with +them. A bare number is always a ping ID, never a person ID. + +Use 'basecamp pings list' to see your pings. +Use 'basecamp pings start ' to open one, or find the one you already have. +Use 'basecamp pings messages ' and 'basecamp pings post "message"' to talk.`, + Annotations: map[string]string{"agent_notes": "A ping appears in 'pings list' once it has a message; one just started is not listed yet, but 'pings start' prints it\n'pings start' reuses an existing direct ping (and an existing group with the same people unless --new-group), so it also opens your ping with someone\nA bare number is a ping ID; pass a person by name or email\n@mentions in a ping post resolve among the people in that ping; use --content-type text/plain to bypass\nPings are not project chats: 'chat list --all' does not show them\nIn the API a ping is a circle (/circles, bucket type Circle)"}, + } + + cmd.AddCommand( + newPingsListCmd(), + newPingsShowCmd(), + newPingsStartCmd(), + newPingsMessagesCmd(), + newPingsPostCmd(&contentType), + newPingsUploadCmd(), + ) + + return cmd +} + +// pingView is a Ping with the id of its Campfire, which the chat commands take. +type pingView struct { + basecamp.Ping + CampfireID int64 `json:"campfire_id,omitempty"` +} + +func newPingView(p *basecamp.Ping) pingView { + v := pingView{Ping: *p} + if id, ok := p.CampfireID(); ok { + v.CampfireID = id + } + return v +} + +func newPingViews(pings []basecamp.Ping) []pingView { + views := make([]pingView, len(pings)) + for i := range pings { + views[i] = newPingView(&pings[i]) + } + return views +} + +func newPingsListCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "list", + Short: "List your pings", + Long: `List your active pings, most recently updated first. + +A ping appears here once someone has posted in it. One you just started, with +nothing said yet, is not listed; 'basecamp pings start' prints it. Archived +pings are not listed either.`, + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, args []string) error { + app := appctx.FromContext(cmd.Context()) + if err := ensureAccount(cmd, app); err != nil { + return err + } + + pings, err := app.Account().Pings().List(cmd.Context()) + if err != nil { + return pingsError(err) + } + + views := newPingViews(pings) + opts := []output.ResponseOption{ + output.WithSummary(fmt.Sprintf("%d pings", len(pings))), + output.WithEntity("ping"), + } + if len(pings) == 0 { + opts = append(opts, + output.WithNotice("No pings yet. A ping appears here once it has a message."), + output.WithBreadcrumbs(output.Breadcrumb{ + Action: "start", + Cmd: "basecamp pings start ", + Description: "Start a ping", + }), + ) + } else { + opts = append(opts, output.WithBreadcrumbs( + output.Breadcrumb{ + Action: "messages", + Cmd: fmt.Sprintf("basecamp pings messages %d", pings[0].ID), + Description: "Read the latest ping", + }, + output.Breadcrumb{ + Action: "start", + Cmd: "basecamp pings start ", + Description: "Start or open a ping", + }, + )) + } + return app.OK(views, opts...) + }, + } + return cmd +} + +func newPingsShowCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "show ", + Short: "Show a ping", + Long: `Show a ping: who is in it and its chat. + + is a ping ID, a ping URL, or a person (your direct ping with them).`, + Example: ` basecamp pings show 49282878 + basecamp pings show https://app.basecamp.com/123/circles/49282878 + basecamp pings show "Jason Fried"`, + Args: cobra.MaximumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + app := appctx.FromContext(cmd.Context()) + if len(args) == 0 { + return missingArg(cmd, "") + } + target, err := parsePingTarget(app, args[0]) + if err != nil { + return err + } + if err := ensureAccount(cmd, app); err != nil { + return err + } + ping, err := resolvePing(cmd.Context(), app, target, pingLookupOnly) + if err != nil { + return err + } + return showPing(app, ping) + }, + } + return cmd +} + +func showPing(app *appctx.App, ping *basecamp.Ping) error { + view := newPingView(ping) + return app.OK(view, + output.WithSummary(pingSummary(ping)), + output.WithEntity("ping"), + output.WithBreadcrumbs(pingNextSteps(ping)...), + ) +} + +func newPingsStartCmd() *cobra.Command { + var newGroup bool + + cmd := &cobra.Command{ + Use: "start ...", + Aliases: []string{"with"}, + Short: "Start a ping, or open the one you have", + Long: `Start a ping with one or more people, or open the ping you already have with them. + +One person makes a direct ping, two or more a group. Starting a ping that +already exists returns it: you have one direct ping with each person, and one +group per set of people unless --new-group asks for another. So this is also +how you find your ping with someone. + +People are names, email addresses or person IDs among the people you can ping +(see 'basecamp people pingable').`, + Example: ` basecamp pings start Jason + basecamp pings with "Jason Fried" "Jane Smith" + basecamp pings start jane@example.com john@example.com --new-group`, + Args: cobra.MinimumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + app := appctx.FromContext(cmd.Context()) + if newGroup && len(args) < 2 { + return output.ErrUsageHint("--new-group needs two or more people", + "A direct ping is always the one you have with that person") + } + if err := ensureAccount(cmd, app); err != nil { + return err + } + + people, err := resolvePingPeople(cmd.Context(), app, args) + if err != nil { + return err + } + ping, err := startPing(cmd.Context(), app, people, newGroup) + if err != nil { + return err + } + + opts := []output.ResponseOption{ + output.WithSummary(pingSummary(ping)), + output.WithEntity("ping"), + output.WithBreadcrumbs(pingNextSteps(ping)...), + } + if left := peopleLeftOut(people, ping); len(left) > 0 { + opts = append(opts, output.WithDiagnostic("Left out because they cannot be pinged: "+strings.Join(left, ", "))) + } + return app.OK(newPingView(ping), opts...) + }, + } + + cmd.Flags().BoolVar(&newGroup, "new-group", false, "Start a new group even when you already have one with these people") + + return cmd +} + +func newPingsMessagesCmd() *cobra.Command { + var limit int + + cmd := &cobra.Command{ + Use: "messages ", + Short: "View recent messages in a ping", + Long: `View recent messages in a ping, oldest first. + + is a ping ID, a ping URL, or a person (your direct ping with them).`, + Example: ` basecamp pings messages 49282878 + basecamp pings messages Jason --limit 50`, + Args: cobra.MaximumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + app := appctx.FromContext(cmd.Context()) + if len(args) == 0 { + return missingArg(cmd, "") + } + target, err := parsePingTarget(app, args[0]) + if err != nil { + return err + } + if limit < 0 { + return output.ErrUsage("--limit must not be negative") + } + if err := ensureAccount(cmd, app); err != nil { + return err + } + ping, campfireID, err := resolvePingCampfire(cmd.Context(), app, target, pingLookupOnly) + if err != nil { + return err + } + return showChatMessages(cmd, app, campfireID, limit, pingNav(ping.ID)) + }, + } + + cmd.Flags().IntVarP(&limit, "limit", "n", 25, "Number of messages to show") + + return cmd +} + +func newPingsPostCmd(contentType *string) *cobra.Command { + var content string + var attachFiles []string + + cmd := &cobra.Command{ + Use: "post ", + Short: "Post a message to a ping", + Long: `Post a message to a ping. + + is a ping ID, a ping URL, or a person. A person names your direct ping +with them, which is started if you do not have one yet. + +By default, messages are sent as plain text. Use --content-type text/html +for rich text (HTML) messages. + +@mentions (@Name or @First.Last) resolve among the people in the ping and the +content type is promoted to text/html when mentions are present. + +Use - as the message argument to read the message from stdin: + printf 'Build is green' | basecamp pings post Jason -`, + Example: ` basecamp pings post 49282878 "Lunch?" + basecamp pings post Jason "Lunch?"`, + Args: cobra.MaximumNArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { + app := appctx.FromContext(cmd.Context()) + if len(args) == 0 { + return missingArg(cmd, "") + } + target, err := parsePingTarget(app, args[0]) + if err != nil { + return err + } + + messageContent, err := readPostInput(cmd, args[1:], 1, content, *contentType, attachFiles, func() error { return nil }) + if err != nil || messageContent == "" { + return err + } + if err := ensureAccount(cmd, app); err != nil { + return err + } + + ping, campfireID, err := resolvePingCampfire(cmd.Context(), app, target, pingStartIfMissing) + if err != nil { + return err + } + return postToChat(cmd, app, campfireID, messageContent, *contentType, attachFiles, pingMentionResolver(ping), pingNav(ping.ID)) + }, + } + + cmd.Flags().StringVar(&content, "content", "", "Message content; use - to read from stdin") + cmd.Flags().StringVar(contentType, "content-type", "", "Content type (text/html for rich text)") + cmd.Flags().StringArrayVar(&attachFiles, "attach", nil, "Attach file (repeatable)") + + allowDash(cmd, "arg:1", "flag:content") + + return cmd +} + +func newPingsUploadCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "upload ", + Short: "Upload a file to a ping", + Long: `Upload a file to a ping as a message with an attachment. + + is a ping ID, a ping URL, or a person. A person names your direct ping +with them, which is started if you do not have one yet.`, + Example: ` basecamp pings upload 49282878 ./screenshot.png`, + Args: cobra.ExactArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { + app := appctx.FromContext(cmd.Context()) + target, err := parsePingTarget(app, args[0]) + if err != nil { + return err + } + filePath := richtext.NormalizeDragPath(args[1]) + if err := richtext.ValidateFile(filePath); err != nil { + return fmt.Errorf("%s: %w", filePath, err) + } + if err := ensureAccount(cmd, app); err != nil { + return err + } + ping, campfireID, err := resolvePingCampfire(cmd.Context(), app, target, pingStartIfMissing) + if err != nil { + return err + } + return uploadToChat(cmd, app, campfireID, filePath, pingNav(ping.ID)) + }, + } + return cmd +} + +// pingNav continues a Ping through `pings`. +func pingNav(pingID int64) chatNav { + return chatNav{ + messages: func(limit int) string { + if limit > 0 { + return fmt.Sprintf("basecamp pings messages %d --limit %d", pingID, limit) + } + return fmt.Sprintf("basecamp pings messages %d", pingID) + }, + post: func(text string) string { + return fmt.Sprintf("basecamp pings post %d %q", pingID, text) + }, + unresolved: func(names []string) string { + if len(names) == 0 { + return "" + } + return "Unresolved mentions left as text: " + strings.Join(names, ", ") + + " (only people in this ping can be mentioned; [@Name](person:ID) names one by ID)" + }, + } +} + +func pingNextSteps(ping *basecamp.Ping) []output.Breadcrumb { + nav := pingNav(ping.ID) + return []output.Breadcrumb{ + {Action: "messages", Cmd: nav.messages(0), Description: "View messages"}, + {Action: "post", Cmd: nav.post("message"), Description: "Post message"}, + } +} + +func pingSummary(ping *basecamp.Ping) string { + return fmt.Sprintf("Ping #%d: %s", ping.ID, pingTitle(ping)) +} + +func pingTitle(ping *basecamp.Ping) string { + if ping.Title != "" { + return ping.Title + } + return "Ping" +} + +// pingsError turns the API's refusals of a Ping request into the CLI's +// vocabulary: Pings turned off for the company, and a Ping Basecamp would not +// start. +func pingsError(err error) error { + converted := convertSDKError(err) + var e *output.Error + if !errors.As(converted, &e) { + return converted + } + switch e.HTTPStatus { + case 403: + if strings.Contains(strings.ToLower(e.Message), "pings have been disabled") { + e.Hint = "Your company has turned Pings off, so no ping command works for you" + } + } + return e +} + +type pingLookup int + +const ( + // pingLookupOnly reads a Ping that exists: a person names a direct ping + // the caller already has, never one started for the occasion. + pingLookupOnly pingLookup = iota + // pingStartIfMissing starts the direct ping a person names when there is + // none, for commands that write to it. + pingStartIfMissing +) + +// pingTarget is what a argument names: a Ping by id, or the caller's +// direct Ping with a person. +type pingTarget struct { + id int64 + person string +} + +// parsePingTarget reads a argument without touching the network. A URL +// must be on a Basecamp host and in the configured account; a bare number is a +// Ping id; anything else names a person. +func parsePingTarget(app *appctx.App, arg string) (pingTarget, error) { + if urlarg.IsURL(arg) { + if !hostutil.IsTrustedBasecampHost(arg, app.Config.BaseURL) { + return pingTarget{}, output.ErrUsage("refusing untrusted host in URL — expected a Basecamp URL") + } + parsed := urlarg.Parse(arg) + if parsed == nil || parsed.PingID == "" { + return pingTarget{}, output.ErrUsageHint("expected a ping URL", + "Ping URLs look like https://app.basecamp.com//circles/") + } + if parsed.AccountID != "" && app.Config.AccountID != "" && parsed.AccountID != app.Config.AccountID { + return pingTarget{}, output.ErrUsage(fmt.Sprintf("URL account %s does not match the configured account %s", parsed.AccountID, app.Config.AccountID)) + } + id, err := strconv.ParseInt(parsed.PingID, 10, 64) + if err != nil { + return pingTarget{}, output.ErrUsage("Invalid ping ID") + } + return pingTarget{id: id}, nil + } + if isNumeric(arg) { + id, err := strconv.ParseInt(arg, 10, 64) + if err != nil { + return pingTarget{}, output.ErrUsage("Invalid ping ID") + } + return pingTarget{id: id}, nil + } + if strings.TrimSpace(arg) == "" { + return pingTarget{}, output.ErrUsage(" must not be empty") + } + return pingTarget{person: arg}, nil +} + +// resolvePing finds the Ping a target names. +func resolvePing(ctx context.Context, app *appctx.App, target pingTarget, mode pingLookup) (*basecamp.Ping, error) { + if target.id != 0 { + ping, err := app.Account().Pings().Get(ctx, target.id) + if err != nil { + return nil, pingsGetError(err, target.id) + } + return ping, nil + } + + people, err := resolvePingPeople(ctx, app, []string{target.person}) + if err != nil { + return nil, err + } + if mode == pingStartIfMissing { + return startPing(ctx, app, people, false) + } + + pings, err := app.Account().Pings().List(ctx) + if err != nil { + return nil, pingsError(err) + } + for i := range pings { + if isDirectPingWith(&pings[i], people[0].ID) { + return &pings[i], nil + } + } + return nil, output.ErrNotFoundHint("Ping", target.person, + fmt.Sprintf("No ping with %s has a message yet. Start one: basecamp pings start %s", people[0].Name, quoteArg(target.person))) +} + +func isDirectPingWith(ping *basecamp.Ping, personID int64) bool { + return !ping.Named && len(ping.People) == 1 && ping.People[0].ID == personID +} + +// resolvePingCampfire finds the Ping a target names and its Campfire. +func resolvePingCampfire(ctx context.Context, app *appctx.App, target pingTarget, mode pingLookup) (*basecamp.Ping, int64, error) { + ping, err := resolvePing(ctx, app, target, mode) + if err != nil { + return nil, 0, err + } + campfireID, ok := ping.CampfireID() + if !ok { + return nil, 0, output.ErrAPI(0, fmt.Sprintf("ping #%d did not name its chat", ping.ID)) + } + return ping, campfireID, nil +} + +func pingsGetError(err error, id int64) error { + converted := pingsError(err) + var e *output.Error + if errors.As(converted, &e) && e.HTTPStatus == 404 { + return output.ErrNotFoundHint("Ping", strconv.FormatInt(id, 10), + "A bare number is a ping ID, not a person ID; list yours with: basecamp pings list") + } + return converted +} + +// resolvePingPeople resolves the people to ping from their names, email +// addresses or ids among the people the caller can ping, once each. +func resolvePingPeople(ctx context.Context, app *appctx.App, args []string) ([]names.Person, error) { + var people []names.Person + for _, arg := range args { + if strings.EqualFold(strings.TrimSpace(arg), "me") { + return nil, output.ErrUsageHint("You are always in your own pings", "Name the people to ping with you") + } + person, err := app.Names.ResolvePingable(ctx, arg) + if err != nil { + return nil, err + } + if !slices.ContainsFunc(people, func(p names.Person) bool { return p.ID == person.ID }) { + people = append(people, *person) + } + } + return people, nil +} + +// startPing opens the Ping with people: the existing one when there is one. +func startPing(ctx context.Context, app *appctx.App, people []names.Person, newGroup bool) (*basecamp.Ping, error) { + ids := make([]int64, len(people)) + for i, p := range people { + ids[i] = p.ID + } + ping, err := app.Account().Pings().Create(ctx, &basecamp.CreatePingRequest{UserIDs: ids, NewGroup: newGroup}) + if err != nil { + return nil, pingsStartError(err) + } + if len(peopleLeftOut(people, ping)) == len(people) { + return nil, output.ErrUsageHint("None of those people can be pinged", + "See who you can ping: basecamp people pingable") + } + return ping, nil +} + +func pingsStartError(err error) error { + converted := pingsError(err) + var e *output.Error + if errors.As(converted, &e) && e.HTTPStatus == 422 { + return &output.Error{ + Code: e.Code, + Message: "Basecamp would not start this ping", + Hint: "An agent can only start a ping that includes someone who can use Pings", + HTTPStatus: e.HTTPStatus, + Cause: e.Cause, + } + } + return converted +} + +// peopleLeftOut names the requested people the Ping does not hold. Basecamp +// drops people the caller cannot reach rather than refusing them. +func peopleLeftOut(requested []names.Person, ping *basecamp.Ping) []string { + var left []string + for _, p := range requested { + if !slices.ContainsFunc(ping.People, func(m basecamp.Person) bool { return m.ID == p.ID }) { + left = append(left, p.Name) + } + } + return left +} + +func quoteArg(s string) string { + if strings.ContainsAny(s, " \t\"'") { + return strconv.Quote(s) + } + return s +} + +// pingMentionResolver resolves @mentions among the people in the Ping: only +// they can read what is said there. +func pingMentionResolver(ping *basecamp.Ping) mentionResolver { + people := make([]names.Person, len(ping.People)) + for i, p := range ping.People { + people[i] = names.Person{ + ID: p.ID, + AttachableSGID: p.AttachableSGID, + Name: p.Name, + Email: p.EmailAddress, + PersonableType: p.PersonableType, + } + } + return func(_ context.Context, html string) (richtext.MentionResult, error) { + return resolveMentionsWith(html, + func(name string) (*names.Person, error) { return names.ResolveAmong(name, people) }, + func(id int64) (*names.Person, error) { + for i := range people { + if people[i].ID == id { + return &people[i], nil + } + } + return nil, output.ErrNotFoundHint("Person", strconv.FormatInt(id, 10), "Not in this ping") + }, + ) + } +} diff --git a/internal/commands/pings_test.go b/internal/commands/pings_test.go new file mode 100644 index 000000000..ffd678095 --- /dev/null +++ b/internal/commands/pings_test.go @@ -0,0 +1,564 @@ +package commands + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "strings" + "sync" + "testing" + + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/basecamp/basecamp-cli/internal/appctx" + "github.com/basecamp/basecamp-cli/internal/output" +) + +const pingsTestAPI = "https://3.basecampapi.com/99999" + +func pingPersonJSON(id int, name, email string) string { + return fmt.Sprintf(`{"id":%d,"name":%q,"email_address":%q,"attachable_sgid":"sgid-%d","personable_type":"User","can_ping":true}`, id, name, email, id) +} + +// pingJSON is a Ping whose Campfire is campfireID. people are the other people. +func pingJSON(id, campfireID int, title string, named bool, people ...string) string { + return fmt.Sprintf(`{"id":%d,"created_at":"2026-10-19T15:15:25.688Z","updated_at":"2026-10-19T15:16:25.688Z",`+ + `"title":%q,"named":%t,"avatar_url":"https://x/avatar","url":"%s/circles/%d.json",`+ + `"app_url":"https://3.basecamp.com/99999/circles/%d","bookmark_url":"https://x/bm",`+ + `"room_url":"%s/buckets/%d/chats/%d.json",`+ + `"links":{"lines":"%s/buckets/%d/chats/%d/lines.json","files":"%s/buckets/%d/chats/%d/uploads.json"},`+ + `"people":[%s]}`, + id, title, named, pingsTestAPI, id, id, pingsTestAPI, id, campfireID, + pingsTestAPI, id, campfireID, pingsTestAPI, id, campfireID, strings.Join(people, ",")) +} + +type pingsReply struct { + status int + body string +} + +// pingsFake is a Basecamp that answers by "METHOD path" and records what it +// was asked, bodies included. +type pingsFake struct { + mu sync.Mutex + replies map[string]pingsReply + requests []string + bodies map[string][]byte +} + +func newPingsFake() *pingsFake { + return &pingsFake{replies: map[string]pingsReply{}, bodies: map[string][]byte{}} +} + +func (f *pingsFake) on(method, path string, status int, body string) { + f.replies[method+" "+path] = pingsReply{status, body} +} + +func (f *pingsFake) RoundTrip(req *http.Request) (*http.Response, error) { + key := req.Method + " " + req.URL.Path + f.mu.Lock() + f.requests = append(f.requests, key) + if req.Body != nil { + b, _ := io.ReadAll(req.Body) + req.Body.Close() + f.bodies[key] = b + } + reply, ok := f.replies[key] + f.mu.Unlock() + if !ok { + reply = pingsReply{404, `{"error":"no fake for ` + key + `"}`} + } + header := make(http.Header) + header.Set("Content-Type", "application/json") + return &http.Response{ + StatusCode: reply.status, + Body: io.NopCloser(strings.NewReader(reply.body)), + Header: header, + Request: req, + }, nil +} + +func (f *pingsFake) asked(key string) bool { + f.mu.Lock() + defer f.mu.Unlock() + for _, r := range f.requests { + if r == key { + return true + } + } + return false +} + +func (f *pingsFake) body(key string) map[string]any { + f.mu.Lock() + defer f.mu.Unlock() + var m map[string]any + _ = json.Unmarshal(f.bodies[key], &m) + return m +} + +// pingable is who the caller can ping in the tests. +const pingablePeople = `[` + + `{"id":1001,"name":"Jason Fried","email_address":"jason@example.com","attachable_sgid":"sgid-1001"},` + + `{"id":1002,"name":"Jane Smith","email_address":"jane@example.com","attachable_sgid":"sgid-1002"},` + + `{"id":1003,"name":"John Doe","email_address":"john@example.com","attachable_sgid":"sgid-1003"},` + + `{"id":1004,"name":"Jason Zimdars","email_address":"jz@example.com","attachable_sgid":"sgid-1004"}]` + +func newPingsFakeWithPeople() *pingsFake { + f := newPingsFake() + f.on("GET", "/99999/circles/people.json", 200, pingablePeople) + f.on("GET", "/99999/projects.json", 200, `[]`) + return f +} + +type pingsEnvelope struct { + Summary string `json:"summary"` + Notice string `json:"notice"` + Breadcrumbs []struct { + Cmd string `json:"cmd"` + } `json:"breadcrumbs"` + Data json.RawMessage `json:"data"` +} + +func (e pingsEnvelope) crumbs() []string { + cmds := make([]string, 0, len(e.Breadcrumbs)) + for _, b := range e.Breadcrumbs { + cmds = append(cmds, b.Cmd) + } + return cmds +} + +func runPings(t *testing.T, f *pingsFake, newCmd func() *cobra.Command, args ...string) (pingsEnvelope, error) { + t.Helper() + app, buf := newTestAppWithTransport(t, f) + app.Flags.Hints = true + cmd := newCmd() + cmd.SetArgs(args) + cmd.SetContext(appctx.WithApp(context.Background(), app)) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + err := cmd.Execute() + var env pingsEnvelope + if buf.Len() > 0 { + require.NoError(t, json.Unmarshal(buf.Bytes(), &env), buf.String()) + } + return env, err +} + +func runPingsCmd(t *testing.T, f *pingsFake, args ...string) (pingsEnvelope, error) { + t.Helper() + return runPings(t, f, NewPingsCmd, args...) +} + +func dataMap(t *testing.T, e pingsEnvelope) map[string]any { + t.Helper() + var m map[string]any + require.NoError(t, json.Unmarshal(e.Data, &m)) + return m +} + +func TestPingsListShowsPingsWithTheirCampfire(t *testing.T) { + f := newPingsFakeWithPeople() + f.on("GET", "/99999/circles.json", 200, "["+ + pingJSON(500, 700, "Jason Fried", false, pingPersonJSON(1001, "Jason Fried", "jason@example.com"))+","+ + pingJSON(501, 701, "Jane, John", false, pingPersonJSON(1002, "Jane Smith", "jane@example.com"), pingPersonJSON(1003, "John Doe", "john@example.com"))+"]") + + env, err := runPingsCmd(t, f, "list") + require.NoError(t, err) + + assert.Equal(t, "2 pings", env.Summary) + var data []map[string]any + require.NoError(t, json.Unmarshal(env.Data, &data)) + require.Len(t, data, 2) + assert.EqualValues(t, 500, data[0]["id"]) + assert.EqualValues(t, 700, data[0]["campfire_id"]) + assert.Equal(t, "Jane, John", data[1]["title"]) + assert.Contains(t, env.crumbs(), "basecamp pings messages 500") +} + +func TestPingsListEmptyExplainsWhenAPingAppears(t *testing.T) { + f := newPingsFakeWithPeople() + f.on("GET", "/99999/circles.json", 200, `[]`) + + env, err := runPingsCmd(t, f, "list") + require.NoError(t, err) + + assert.Equal(t, "0 pings", env.Summary) + assert.Contains(t, env.Notice, "appears here once it has a message") + assert.Contains(t, env.crumbs(), "basecamp pings start ") +} + +func TestPingsDisabledForTheCompanySaysSo(t *testing.T) { + f := newPingsFakeWithPeople() + f.on("GET", "/99999/circles.json", 403, `{"error":"Pings have been disabled"}`) + + _, err := runPingsCmd(t, f, "list") + require.Error(t, err) + assert.Contains(t, err.Error(), "Pings have been disabled") + assert.Contains(t, pingsErrorHint(t, err), "turned Pings off") +} + +func TestPingsShowByIDAndByURL(t *testing.T) { + for _, arg := range []string{ + "500", + "https://app.basecamp.com/99999/circles/500", + "https://3.basecamp.com/99999/circles/500", + } { + t.Run(arg, func(t *testing.T) { + f := newPingsFakeWithPeople() + f.on("GET", "/99999/circles/500", 200, pingJSON(500, 700, "Jason Fried", false, pingPersonJSON(1001, "Jason Fried", "jason@example.com"))) + + env, err := runPingsCmd(t, f, "show", arg) + require.NoError(t, err) + + assert.Equal(t, "Ping #500: Jason Fried", env.Summary) + assert.EqualValues(t, 700, dataMap(t, env)["campfire_id"]) + assert.Equal(t, []string{"basecamp pings messages 500", `basecamp pings post 500 "message"`}, env.crumbs()) + }) + } +} + +func TestPingsShowUnknownIDExplainsBareNumbersArePings(t *testing.T) { + f := newPingsFakeWithPeople() + + _, err := runPingsCmd(t, f, "show", "123") + require.Error(t, err) + assert.Contains(t, pingsErrorHint(t, err), "not a person ID") +} + +func TestPingsRejectURLsThatAreNotPings(t *testing.T) { + f := newPingsFakeWithPeople() + + _, err := runPingsCmd(t, f, "show", "https://3.basecamp.com/99999/buckets/456/todos/789") + require.Error(t, err) + assert.Contains(t, err.Error(), "expected a ping URL") + + _, err = runPingsCmd(t, f, "show", "https://evil.example.com/99999/circles/500") + require.Error(t, err) + assert.Contains(t, err.Error(), "untrusted host") + + _, err = runPingsCmd(t, f, "show", "https://3.basecamp.com/11111/circles/500") + require.Error(t, err) + assert.Contains(t, err.Error(), "does not match the configured account") + + assert.Empty(t, f.requests, "a bad reference must not reach the network") +} + +func TestPingsStartDirectPingByName(t *testing.T) { + f := newPingsFakeWithPeople() + f.on("POST", "/99999/circles.json", 201, pingJSON(500, 700, "Jane Smith", false, pingPersonJSON(1002, "Jane Smith", "jane@example.com"))) + + env, err := runPingsCmd(t, f, "start", "Jane") + require.NoError(t, err) + + body := f.body("POST /99999/circles.json") + assert.Equal(t, []any{float64(1002)}, body["user_ids"]) + assert.NotContains(t, body, "new_group") + assert.Equal(t, "Ping #500: Jane Smith", env.Summary) + assert.EqualValues(t, 700, dataMap(t, env)["campfire_id"]) + assert.Contains(t, env.crumbs(), "basecamp pings messages 500") +} + +func TestPingsStartTwiceReturnsTheSamePing(t *testing.T) { + f := newPingsFakeWithPeople() + f.on("POST", "/99999/circles.json", 201, pingJSON(500, 700, "Jane Smith", false, pingPersonJSON(1002, "Jane Smith", "jane@example.com"))) + + first, err := runPingsCmd(t, f, "start", "jane@example.com") + require.NoError(t, err) + second, err := runPingsCmd(t, f, "with", "1002") + require.NoError(t, err) + + assert.Equal(t, dataMap(t, first)["id"], dataMap(t, second)["id"]) +} + +func TestPingsStartGroupAndNewGroup(t *testing.T) { + people := []string{pingPersonJSON(1002, "Jane Smith", "jane@example.com"), pingPersonJSON(1003, "John Doe", "john@example.com")} + + f := newPingsFakeWithPeople() + f.on("POST", "/99999/circles.json", 201, pingJSON(501, 701, "Jane Smith, John Doe", false, people...)) + _, err := runPingsCmd(t, f, "start", "Jane", "john@example.com") + require.NoError(t, err) + body := f.body("POST /99999/circles.json") + assert.Equal(t, []any{float64(1002), float64(1003)}, body["user_ids"]) + assert.NotContains(t, body, "new_group") + + f = newPingsFakeWithPeople() + f.on("POST", "/99999/circles.json", 201, pingJSON(502, 702, "Jane Smith, John Doe", false, people...)) + _, err = runPingsCmd(t, f, "start", "Jane", "John", "--new-group") + require.NoError(t, err) + assert.Equal(t, true, f.body("POST /99999/circles.json")["new_group"]) +} + +func TestPingsStartDeduplicatesPeople(t *testing.T) { + f := newPingsFakeWithPeople() + f.on("POST", "/99999/circles.json", 201, pingJSON(500, 700, "Jane Smith", false, pingPersonJSON(1002, "Jane Smith", "jane@example.com"))) + + _, err := runPingsCmd(t, f, "start", "Jane", "jane@example.com", "1002") + require.NoError(t, err) + assert.Equal(t, []any{float64(1002)}, f.body("POST /99999/circles.json")["user_ids"]) +} + +func TestPingsStartNewGroupNeedsTwoPeople(t *testing.T) { + f := newPingsFakeWithPeople() + + _, err := runPingsCmd(t, f, "start", "Jane", "--new-group") + require.Error(t, err) + assert.Contains(t, err.Error(), "--new-group needs two or more people") + assert.False(t, f.asked("POST /99999/circles.json")) +} + +func TestPingsStartAmbiguousNameAsksWhichPerson(t *testing.T) { + f := newPingsFakeWithPeople() + + _, err := runPingsCmd(t, f, "start", "Jason") + require.Error(t, err) + assert.Contains(t, err.Error(), "Ambiguous") + assert.False(t, f.asked("POST /99999/circles.json")) +} + +func TestPingsStartUnknownPersonIsNotFound(t *testing.T) { + f := newPingsFakeWithPeople() + + _, err := runPingsCmd(t, f, "start", "Nobody Atall") + require.Error(t, err) + assert.Contains(t, err.Error(), "not found") + assert.False(t, f.asked("POST /99999/circles.json")) +} + +func TestPingsStartReportsPeopleBasecampLeftOut(t *testing.T) { + f := newPingsFakeWithPeople() + f.on("POST", "/99999/circles.json", 201, pingJSON(500, 700, "Jane Smith", false, pingPersonJSON(1002, "Jane Smith", "jane@example.com"))) + + env, err := runPingsCmd(t, f, "start", "Jane", "John") + require.NoError(t, err) + assert.Contains(t, env.Notice, "Left out because they cannot be pinged: John Doe") +} + +func TestPingsStartWithNobodyReachableFails(t *testing.T) { + f := newPingsFakeWithPeople() + f.on("POST", "/99999/circles.json", 201, pingJSON(500, 700, "Me", false)) + + _, err := runPingsCmd(t, f, "start", "Jane") + require.Error(t, err) + assert.Contains(t, err.Error(), "None of those people can be pinged") +} + +func TestPingsStartRefusedWith422ExplainsAgentRule(t *testing.T) { + f := newPingsFakeWithPeople() + f.on("POST", "/99999/circles.json", 422, ``) + + _, err := runPingsCmd(t, f, "start", "Jane") + require.Error(t, err) + assert.Contains(t, err.Error(), "would not start this ping") + assert.Contains(t, pingsErrorHint(t, err), "someone who can use Pings") +} + +func TestPingsStartDisabledWith403(t *testing.T) { + f := newPingsFakeWithPeople() + f.on("POST", "/99999/circles.json", 403, `{"error":"Pings have been disabled"}`) + + _, err := runPingsCmd(t, f, "start", "Jane") + require.Error(t, err) + assert.Contains(t, err.Error(), "Pings have been disabled") +} + +func TestPingsStartMeIsRefused(t *testing.T) { + f := newPingsFakeWithPeople() + + _, err := runPingsCmd(t, f, "start", "me") + require.Error(t, err) + assert.Contains(t, err.Error(), "always in your own pings") +} + +func TestPingsMessagesReadsTheCampfireOfThePing(t *testing.T) { + f := newPingsFakeWithPeople() + f.on("GET", "/99999/circles/500", 200, pingJSON(500, 700, "Jason Fried", false, pingPersonJSON(1001, "Jason Fried", "jason@example.com"))) + f.on("GET", "/99999/chats/700/lines.json", 200, `[{"id":9,"content":"hi","created_at":"2026-10-19T15:15:25.688Z"}]`) + + env, err := runPingsCmd(t, f, "messages", "https://app.basecamp.com/99999/circles/500") + require.NoError(t, err) + + assert.Equal(t, "1 messages", env.Summary) + assert.Equal(t, []string{`basecamp pings post 500 "message"`, "basecamp pings messages 500 --limit 50"}, env.crumbs()) +} + +func TestPingsMessagesByPersonUsesTheDirectPingFromTheList(t *testing.T) { + f := newPingsFakeWithPeople() + f.on("GET", "/99999/circles.json", 200, "["+ + pingJSON(501, 701, "Jane, John", false, pingPersonJSON(1002, "Jane Smith", "jane@example.com"), pingPersonJSON(1003, "John Doe", "john@example.com"))+","+ + pingJSON(502, 702, "Named with Jane", true, pingPersonJSON(1002, "Jane Smith", "jane@example.com"))+","+ + pingJSON(500, 700, "Jane Smith", false, pingPersonJSON(1002, "Jane Smith", "jane@example.com"))+"]") + f.on("GET", "/99999/chats/700/lines.json", 200, `[]`) + + env, err := runPingsCmd(t, f, "messages", "Jane") + require.NoError(t, err) + + assert.Equal(t, "0 messages", env.Summary) + assert.False(t, f.asked("POST /99999/circles.json"), "reading a ping must not start one") +} + +func TestPingsMessagesByPersonWithNoPingYetSaysHowToStartOne(t *testing.T) { + f := newPingsFakeWithPeople() + f.on("GET", "/99999/circles.json", 200, `[]`) + + _, err := runPingsCmd(t, f, "messages", "Jane") + require.Error(t, err) + assert.Contains(t, pingsErrorHint(t, err), "basecamp pings start Jane") + assert.False(t, f.asked("POST /99999/circles.json"), "reading a ping must not start one") +} + +func TestPingsPostWritesToThePingsCampfire(t *testing.T) { + f := newPingsFakeWithPeople() + f.on("GET", "/99999/circles/500", 200, pingJSON(500, 700, "Jason Fried", false, pingPersonJSON(1001, "Jason Fried", "jason@example.com"))) + f.on("POST", "/99999/chats/700/lines.json", 201, `{"id":9,"content":"Lunch?","created_at":"2026-10-19T15:15:25.688Z"}`) + + env, err := runPingsCmd(t, f, "post", "500", "Lunch?") + require.NoError(t, err) + + assert.Equal(t, "Posted message #9", env.Summary) + assert.Equal(t, "Lunch?", f.body("POST /99999/chats/700/lines.json")["content"]) + assert.Equal(t, []string{"basecamp pings messages 500", `basecamp pings post 500 "reply"`}, env.crumbs()) +} + +func TestPingsPostToAPersonStartsOrReusesTheDirectPing(t *testing.T) { + f := newPingsFakeWithPeople() + f.on("POST", "/99999/circles.json", 201, pingJSON(500, 700, "Jane Smith", false, pingPersonJSON(1002, "Jane Smith", "jane@example.com"))) + f.on("POST", "/99999/chats/700/lines.json", 201, `{"id":9,"content":"hi","created_at":"2026-10-19T15:15:25.688Z"}`) + + _, err := runPingsCmd(t, f, "post", "Jane", "hi") + require.NoError(t, err) + + assert.Equal(t, []any{float64(1002)}, f.body("POST /99999/circles.json")["user_ids"]) + assert.True(t, f.asked("POST /99999/chats/700/lines.json")) +} + +func TestPingsPostMentionsResolveAmongThePingsPeople(t *testing.T) { + f := newPingsFakeWithPeople() + f.on("GET", "/99999/circles/501", 200, pingJSON(501, 701, "Jane, John", false, + pingPersonJSON(1002, "Jane Smith", "jane@example.com"), pingPersonJSON(1003, "John Doe", "john@example.com"))) + f.on("POST", "/99999/chats/701/lines.json", 201, `{"id":9,"content":"x","created_at":"2026-10-19T15:15:25.688Z"}`) + + env, err := runPingsCmd(t, f, "post", "501", "Hey @John, and @Jason Fried too") + require.NoError(t, err) + + body := f.body("POST /99999/chats/701/lines.json") + assert.Equal(t, "text/html", body["content_type"]) + content := body["content"].(string) + assert.Contains(t, content, "sgid-1003", "John is in the ping") + assert.NotContains(t, content, "sgid-1001", "Jason Fried is pingable but not in this ping") + assert.Contains(t, env.Notice, "@Jason") + assert.Contains(t, env.Notice, "only people in this ping") +} + +func TestPingsPostPersonIDMentionOutsideThePingFails(t *testing.T) { + f := newPingsFakeWithPeople() + f.on("GET", "/99999/circles/501", 200, pingJSON(501, 701, "Jane Smith", false, pingPersonJSON(1002, "Jane Smith", "jane@example.com"))) + + _, err := runPingsCmd(t, f, "post", "501", "ask [@Jason](person:1001)") + require.Error(t, err) + assert.Contains(t, pingsErrorHint(t, err), "Not in this ping") + assert.False(t, f.asked("POST /99999/chats/701/lines.json")) +} + +func TestPingsPostPlainTextSkipsMentions(t *testing.T) { + f := newPingsFakeWithPeople() + f.on("GET", "/99999/circles/501", 200, pingJSON(501, 701, "Jane Smith", false, pingPersonJSON(1002, "Jane Smith", "jane@example.com"))) + f.on("POST", "/99999/chats/701/lines.json", 201, `{"id":9,"content":"x","created_at":"2026-10-19T15:15:25.688Z"}`) + + _, err := runPingsCmd(t, f, "post", "501", "hi @Jane", "--content-type", "text/plain") + require.NoError(t, err) + + body := f.body("POST /99999/chats/701/lines.json") + assert.Equal(t, "text/plain", body["content_type"]) + assert.Equal(t, "hi @Jane", body["content"]) +} + +func TestPingsPostWithoutMessageAsksForIt(t *testing.T) { + f := newPingsFakeWithPeople() + + _, err := runPingsCmd(t, f, "post", "500") + require.NoError(t, err, "a missing message shows help, as chat post does") + assert.Empty(t, f.requests) +} + +func TestPingsUploadRejectsMissingFileBeforeTheNetwork(t *testing.T) { + f := newPingsFakeWithPeople() + + _, err := runPingsCmd(t, f, "upload", "500", "/nonexistent/file.png") + require.Error(t, err) + assert.Empty(t, f.requests) +} + +func TestShowPingURL(t *testing.T) { + f := newPingsFakeWithPeople() + f.on("GET", "/99999/circles/500", 200, pingJSON(500, 700, "Jason Fried", false, pingPersonJSON(1001, "Jason Fried", "jason@example.com"))) + + env, err := runPings(t, f, NewShowCmd, "https://app.basecamp.com/99999/circles/500") + require.NoError(t, err) + + assert.Equal(t, "Ping #500: Jason Fried", env.Summary) + assert.EqualValues(t, 700, dataMap(t, env)["campfire_id"]) +} + +func TestShowPingLineURLReadsTheLineRecording(t *testing.T) { + f := newPingsFakeWithPeople() + f.on("GET", "/99999/recordings/111.json", 200, `{"id":111,"type":"Chat::Lines::Text","title":"hi","parent":{"id":700}}`) + f.on("GET", "/99999/chats/700/lines/111", 200, `{"id":111,"type":"Chat::Lines::Text","content":"hi"}`) + + _, err := runPings(t, f, NewShowCmd, "https://app.basecamp.com/99999/circles/500@111") + require.NoError(t, err) + + assert.True(t, f.asked("GET /99999/recordings/111.json")) +} + +func TestChatLineAcceptsAPingLineURL(t *testing.T) { + f := newPingsFakeWithPeople() + f.on("GET", "/99999/circles/500", 200, pingJSON(500, 700, "Jason Fried", false, pingPersonJSON(1001, "Jason Fried", "jason@example.com"))) + f.on("GET", "/99999/chats/700/lines/111", 200, `{"id":111,"content":"hi","creator":{"name":"Jason Fried"},"created_at":"2026-10-19T15:15:25.688Z"}`) + f.on("GET", "/99999/recordings/111/comments.json", 200, `[]`) + + env, err := runPings(t, f, NewChatCmd, "line", "https://app.basecamp.com/99999/circles/500@111") + require.NoError(t, err) + + assert.Equal(t, "Line #111 by Jason Fried", env.Summary) +} + +func TestChatLineRejectsAPingURLWithoutALineAndPointsToPings(t *testing.T) { + f := newPingsFakeWithPeople() + + _, err := runPings(t, f, NewChatCmd, "line", "https://app.basecamp.com/99999/circles/500") + require.Error(t, err) + assert.Contains(t, err.Error(), "a ping URL names a ping, not a message") + assert.Contains(t, pingsErrorHint(t, err), "basecamp pings show 500") +} + +func TestChatMessagesInAPingFindsItsCampfire(t *testing.T) { + f := newPingsFakeWithPeople() + f.on("GET", "/99999/circles/500", 200, pingJSON(500, 700, "Jason Fried", false, pingPersonJSON(1001, "Jason Fried", "jason@example.com"))) + f.on("GET", "/99999/chats/700/lines.json", 200, `[]`) + + _, err := runPings(t, f, NewChatCmd, "messages", "--in", "500") + require.NoError(t, err) + + assert.True(t, f.asked("GET /99999/chats/700/lines.json")) +} + +func TestChatMessagesInAMissingProjectKeepsTheProjectError(t *testing.T) { + f := newPingsFakeWithPeople() + + _, err := runPings(t, f, NewChatCmd, "messages", "--in", "777") + require.Error(t, err) + assert.Contains(t, err.Error(), "not found") +} + +func pingsErrorHint(t *testing.T, err error) string { + t.Helper() + var e *output.Error + require.ErrorAs(t, err, &e) + return e.Hint +} diff --git a/internal/commands/show.go b/internal/commands/show.go index 588cb155d..09957c595 100644 --- a/internal/commands/show.go +++ b/internal/commands/show.go @@ -6,6 +6,7 @@ import ( "encoding/json" "fmt" "net/http" + "strconv" "strings" "github.com/spf13/cobra" @@ -66,6 +67,9 @@ You can also pass a Basecamp URL directly: // Check if the id is a URL and extract components var occurrenceDate string if parsed := urlarg.Parse(id); parsed != nil { + if parsed.PingID != "" && parsed.Type == "circles" { + return showPingURL(cmd, app, parsed) + } if parsed.CommentID != "" { // Fragment URL (#__recording_N) — resolve the referenced // recording directly instead of the parent resource. @@ -344,6 +348,26 @@ You can also pass a Basecamp URL directly: return cmd } +// showPingURL shows the Ping a URL names. A Ping is not a recording, so it +// does not go through the recording endpoints. +func showPingURL(cmd *cobra.Command, app *appctx.App, parsed *urlarg.Parsed) error { + id, err := strconv.ParseInt(parsed.PingID, 10, 64) + if err != nil { + return output.ErrUsage("Invalid ping ID") + } + if err := ensureAccount(cmd, app); err != nil { + return err + } + if parsed.AccountID != "" && parsed.AccountID != app.Config.AccountID { + return output.ErrUsage(fmt.Sprintf("URL account %s does not match the configured account %s", parsed.AccountID, app.Config.AccountID)) + } + ping, err := resolvePing(cmd.Context(), app, pingTarget{id: id}, pingLookupOnly) + if err != nil { + return err + } + return showPing(app, ping) +} + // recordingTypeEndpoint maps a recording's canonical API "type" field to the // type-specific endpoint path. Type names are the namespaced forms returned by // the Basecamp API (e.g. "Kanban::Card", "Schedule::Entry"), matching the SDK diff --git a/internal/commands/show_test.go b/internal/commands/show_test.go index bb20ec39b..36a7b591c 100644 --- a/internal/commands/show_test.go +++ b/internal/commands/show_test.go @@ -817,9 +817,17 @@ func TestShowProjectURLReturnsHelpfulError(t *testing.T) { assert.Contains(t, err.Error(), "projects show") } -func TestShowCircleURLReturnsHelpfulError(t *testing.T) { +func TestShowPingLineURLShowsTheLine(t *testing.T) { transport := &showTrackingTransport{} - _, err := runShowCmd(t, transport, "https://3.basecamp.com/99999/circles/789@456") + reqs, err := runShowCmd(t, transport, "https://3.basecamp.com/99999/circles/789@456") + require.NoError(t, err) + require.GreaterOrEqual(t, len(reqs), 1) + assert.Contains(t, reqs[0], "/recordings/456.json") +} + +func TestShowCirclePeopleURLCannotBeShown(t *testing.T) { + transport := &showTrackingTransport{} + _, err := runShowCmd(t, transport, "https://3.basecamp.com/99999/circles/people") require.Error(t, err) assert.Contains(t, err.Error(), "cannot be shown") } diff --git a/internal/names/mention.go b/internal/names/mention.go index 0eae5c3c3..8812e35e7 100644 --- a/internal/names/mention.go +++ b/internal/names/mention.go @@ -232,3 +232,22 @@ func ParseProjectScope(projectID string) ProjectScope { } return func(context.Context) (int64, error) { return id, nil } } + +// ResolveAmong resolves a fuzzy @Name mention against a fixed set of people, +// such as the people in a Ping, where only they can be addressed. It applies +// the same exact, case-insensitive, then partial matching as everywhere else, +// and fails with the same not-found and ambiguous errors. +func ResolveAmong(input string, people []Person) (*Person, error) { + match, matches := resolve(input, people, personIDName) + if match != nil { + return match, nil + } + if len(matches) > 1 { + return nil, ambiguousPeople(matches) + } + suggestions := suggest(input, people, func(p Person) string { return p.Name }) + if len(suggestions) > 0 { + return nil, output.ErrNotFoundHint("Person", input, "Did you mean: "+strings.Join(suggestions, ", ")) + } + return nil, output.ErrNotFound("Person", input) +} diff --git a/internal/names/ping_people_test.go b/internal/names/ping_people_test.go new file mode 100644 index 000000000..2848afd0d --- /dev/null +++ b/internal/names/ping_people_test.go @@ -0,0 +1,87 @@ +package names + +import ( + "context" + "net/http/httptest" + "testing" + + "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" +) + +func newPingPeopleFixture(t *testing.T) *Resolver { + t.Helper() + jane := person(1, "Jane Smith", "sgid-jane", "User") + jane["email_address"] = "jane@example.com" + jason := person(2, "Jason Fried", "sgid-jason", "User") + jason["email_address"] = "jason@example.com" + jasper := person(3, "Jasper Jones", "sgid-jasper", "User") + s := &mentionServer{ + calls: map[string]int{}, + pingable: []map[string]any{jane, jason, jasper}, + } + server := httptest.NewServer(s) + t.Cleanup(server.Close) + + sdkClient := basecamp.NewClient(&basecamp.Config{BaseURL: server.URL}, testTokenProvider{}, basecamp.WithMaxRetries(1)) + return NewResolver(sdkClient, nil, "99999") +} + +func TestResolvePingableByIDEmailAndName(t *testing.T) { + r := newPingPeopleFixture(t) + ctx := context.Background() + + for _, input := range []string{"1", "jane@example.com", "JANE@EXAMPLE.COM", "Jane Smith", "jane smith", "Jane"} { + p, err := r.ResolvePingable(ctx, input) + require.NoError(t, err, input) + assert.Equal(t, int64(1), p.ID, input) + } +} + +func TestResolvePingableAmbiguousName(t *testing.T) { + r := newPingPeopleFixture(t) + + _, err := r.ResolvePingable(context.Background(), "Jas") + var e *output.Error + require.ErrorAs(t, err, &e) + assert.Equal(t, output.CodeAmbiguous, e.Code) +} + +func TestResolvePingableRefusesWhoCannotBePinged(t *testing.T) { + r := newPingPeopleFixture(t) + ctx := context.Background() + + _, err := r.ResolvePingable(ctx, "999") + var e *output.Error + require.ErrorAs(t, err, &e) + assert.Equal(t, output.CodeNotFound, e.Code) + assert.Contains(t, e.Hint, "basecamp people pingable") + + _, err = r.ResolvePingable(ctx, "Nobody Atall") + require.ErrorAs(t, err, &e) + assert.Equal(t, output.CodeNotFound, e.Code) +} + +func TestResolveAmongOnlyTheGivenPeople(t *testing.T) { + people := []Person{ + {ID: 1, Name: "Jane Smith", AttachableSGID: "sgid-jane"}, + {ID: 2, Name: "John Doe", AttachableSGID: "sgid-john"}, + } + + p, err := ResolveAmong("john", people) + require.NoError(t, err) + assert.Equal(t, "sgid-john", p.AttachableSGID) + + _, err = ResolveAmong("Jason", people) + var e *output.Error + require.ErrorAs(t, err, &e) + assert.Equal(t, output.CodeNotFound, e.Code) + + _, err = ResolveAmong("J", people) + require.ErrorAs(t, err, &e) + assert.Equal(t, output.CodeAmbiguous, e.Code) +} diff --git a/internal/names/resolver.go b/internal/names/resolver.go index 7191310af..84d752f71 100644 --- a/internal/names/resolver.go +++ b/internal/names/resolver.go @@ -290,6 +290,33 @@ func (r *Resolver) ResolvePersonByName(ctx context.Context, input string) (*Pers return nil, output.ErrNotFound("Person", input) } +// ResolvePingable resolves a person's ID, email address or name among the +// people the caller can ping, the only people a Ping can include. A name that +// matches more than one of them is ambiguous. +func (r *Resolver) ResolvePingable(ctx context.Context, input string) (*Person, error) { + pingable, err := r.getPingable(ctx) + if err != nil { + return nil, err + } + + if id, parseErr := strconv.ParseInt(input, 10, 64); parseErr == nil { + for i := range pingable { + if pingable[i].ID == id { + return &pingable[i], nil + } + } + return nil, output.ErrNotFoundHint("Person", input, "Not someone you can ping; see: basecamp people pingable") + } + + for i := range pingable { + if pingable[i].Email != "" && strings.EqualFold(pingable[i].Email, input) { + return &pingable[i], nil + } + } + + return ResolveAmong(input, pingable) +} + // ResolvePersonByID looks up a person by numeric ID in the pingable set. // Returns the Person record including AttachableSGID. // Returns not-found if the person is not in the pingable set. diff --git a/internal/presenter/schemas/ping.yaml b/internal/presenter/schemas/ping.yaml new file mode 100644 index 000000000..fdc15e392 --- /dev/null +++ b/internal/presenter/schemas/ping.yaml @@ -0,0 +1,52 @@ +entity: ping +kind: container + +identity: + label: title + id: id + +headline: + default: + template: "{{.title}}" + +fields: + title: + role: title + emphasis: primary + format: text + + people: + role: detail + format: people + + campfire_id: + role: meta + emphasis: muted + format: text + + updated_at: + role: meta + emphasis: muted + format: relative_time + + created_at: + role: meta + emphasis: muted + format: relative_time + + app_url: + role: meta + format: text + + id: + role: meta + emphasis: muted + +views: + list: + columns: [id, title, people, updated_at] + detail: + sections: + - fields: [title, people] + - heading: Metadata + fields: [id, campfire_id, app_url, created_at, updated_at] diff --git a/skills/basecamp/SKILL.md b/skills/basecamp/SKILL.md index 2ccf0d1bb..2f651c138 100644 --- a/skills/basecamp/SKILL.md +++ b/skills/basecamp/SKILL.md @@ -13,6 +13,8 @@ triggers: - basecamp cards - basecamp chat - basecamp campfire + - basecamp ping + - basecamp pings - basecamp messages - basecamp file - basecamp document @@ -188,6 +190,32 @@ basecamp comments thread "" --json Use `comments show` when only the cheap reply target and mention atoms are needed; use `comments thread` when surrounding discussion matters. +## Pings + +A ping is a private conversation between people on an account, outside any +project: direct with one other person, a group with more. It is not a project +chat, so `chat list --all` never shows one. Use the `pings` group. + +```bash +basecamp pings list --agent # your pings +basecamp pings start "Jane Smith" --agent # open your ping with Jane, or start it +basecamp pings post "Jane Smith" "Lunch?" --agent +basecamp pings messages "Jane Smith" --agent +``` + +- A `` is a ping ID, a ping URL (`/circles/` or `/circles/@`), + or a person. A bare number is always a ping ID, never a person ID. +- `pings list` shows a ping once it has a message. A ping you just started is + not listed yet; `pings start` prints it. +- `pings start` reuses what exists: one direct ping per person, and a group per + set of people unless `--new-group`. Use it to find the ping you already have. +- `pings messages` and `pings show` of a person read the direct ping you already + have and never start one; `pings post` and `pings upload` start it if missing. +- @mentions in `pings post` resolve among the people in that ping. +- A message URL (`/circles/@`) works with `basecamp show` and + `chat line`; edit or delete such a message with `chat update` or `chat delete` + on that URL. + ## Content and stdin Command help declares content arguments and stdin support. Titles are generally From bcb83eae29273663df59ddc250ce2962d2cf9fa5 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Mon, 19 Oct 2026 17:08:30 -0400 Subject: [PATCH 5/7] Accept the sidebar form of a ping URL Basecamp also links a ping as {account}/my/sidebar/circles/{id}, with circles/{id}@{line} under the same prefix. Drop the my/sidebar prefix before routing so pings, show and chat parse it like the plain form. --- internal/urlarg/urlarg.go | 6 ++++++ internal/urlarg/urlarg_ping_test.go | 20 ++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/internal/urlarg/urlarg.go b/internal/urlarg/urlarg.go index 9f69a1474..366cad4ca 100644 --- a/internal/urlarg/urlarg.go +++ b/internal/urlarg/urlarg.go @@ -18,6 +18,10 @@ var chatAtRe = regexp.MustCompile(`(/chats/\d+)@(\d+)`) // whose bucket is the Ping; the Campfire is not in the URL. var pingAtRe = regexp.MustCompile(`/circles/(\d+)@(\d+)`) +// pingSidebarRe matches the sidebar form my/sidebar/circles/{pingID}, which +// Basecamp links to for the same Ping, and drops the my/sidebar prefix. +var pingSidebarRe = regexp.MustCompile(`/my/sidebar(/circles/\d+)`) + // Parsed represents components extracted from a Basecamp URL. type Parsed struct { AccountID string @@ -45,6 +49,7 @@ func IsURL(input string) bool { // slash form; a Ping line loses its @line, which the second result returns. func normalize(input string) (normalized, pingLineID string) { normalized = chatAtRe.ReplaceAllString(input, "${1}/lines/${2}") + normalized = pingSidebarRe.ReplaceAllString(normalized, "${1}") if m := pingAtRe.FindStringSubmatch(normalized); m != nil { pingLineID = m[2] normalized = pingAtRe.ReplaceAllString(normalized, "/circles/${1}") @@ -62,6 +67,7 @@ func normalize(input string) (normalized, pingLineID string) { // - https://3.basecamp.com/{account}/buckets/{bucket}/chats/{id}@{lineId} // - https://3.basecamp.com/{account}/circles/{ping} // - https://3.basecamp.com/{account}/circles/{ping}@{lineId} +// - https://3.basecamp.com/{account}/my/sidebar/circles/{ping}, with or without @{lineId} // - https://3.basecamp.com/{account}/buckets/{bucket}/schedule_entries/{id}/occurrences/{date} // - https://3.basecamp.com/{account}/projects/{project} func Parse(input string) *Parsed { diff --git a/internal/urlarg/urlarg_ping_test.go b/internal/urlarg/urlarg_ping_test.go index 05bf9f26e..b60bc7caf 100644 --- a/internal/urlarg/urlarg_ping_test.go +++ b/internal/urlarg/urlarg_ping_test.go @@ -38,6 +38,26 @@ func TestParsePingURLs(t *testing.T) { input: "http://3.basecamp.localhost:3001/123/circles/456@789", want: Parsed{AccountID: "123", ProjectID: "456", Type: "lines", RecordingID: "789", PingID: "456"}, }, + { + name: "app host sidebar ping", + input: "https://app.basecamp.com/123/my/sidebar/circles/456", + want: Parsed{AccountID: "123", Type: "circles", RecordingID: "456", PingID: "456"}, + }, + { + name: "web host sidebar ping", + input: "https://3.basecamp.com/123/my/sidebar/circles/456", + want: Parsed{AccountID: "123", Type: "circles", RecordingID: "456", PingID: "456"}, + }, + { + name: "app host sidebar ping line", + input: "https://app.basecamp.com/123/my/sidebar/circles/456@789", + want: Parsed{AccountID: "123", ProjectID: "456", Type: "lines", RecordingID: "789", PingID: "456"}, + }, + { + name: "web host sidebar ping line", + input: "https://3.basecamp.com/123/my/sidebar/circles/456@789", + want: Parsed{AccountID: "123", ProjectID: "456", Type: "lines", RecordingID: "789", PingID: "456"}, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From 530c8a062bbcaf97d1fe0d5642605ebfbd17bca5 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Mon, 19 Oct 2026 21:28:03 -0400 Subject: [PATCH 6/7] Answer the review of the pings commands - Ping line URLs: chat line, update and delete resolve the Ping to its Campfire first, so the URL's room beats a stale --room, and the Ping stays the bucket whatever --in says. - getChatID's Ping fallback keeps the project error only when the Ping is not found either; 403, rate limits and transport errors now surface. - pings start --new-group counts the people after resolution, so two references to one person no longer pass for a group. - pings post and chat post accept --attach with no message; help still shows when there is neither. - pingTarget keeps the URL's account and resolvePing checks it after ensureAccount, before any Ping request (show's duplicate check goes). - The pings start hint quotes the person with richtext.ShellQuote. - policy.go names its loop variable limit, not cap, which the predeclared linter flags locally. --- internal/commands/chat.go | 94 ++++++++++++------ internal/commands/pings.go | 29 +++--- internal/commands/pings_test.go | 129 +++++++++++++++++++++++++ internal/commands/show.go | 5 +- internal/connector/admission/policy.go | 6 +- 5 files changed, 215 insertions(+), 48 deletions(-) diff --git a/internal/commands/chat.go b/internal/commands/chat.go index c0c060846..1df3b2cba 100644 --- a/internal/commands/chat.go +++ b/internal/commands/chat.go @@ -378,7 +378,7 @@ Use - as the message argument to read the message from stdin: } messageContent, err := readPostInput(cmd, args, 0, content, *contentType, attachFiles, checkRoom) - if err != nil || messageContent == "" { + if err != nil || (messageContent == "" && len(attachFiles) == 0) { return err } @@ -410,7 +410,8 @@ Use - as the message argument to read the message from stdin: // discard piped content unread. // // A missing message shows help in a terminal, which is not an error: the result -// is then empty with a nil error, and the caller is done. +// is then empty with a nil error, and the caller is done unless it has +// attachments, which post without text. func readPostInput(cmd *cobra.Command, msgArgs []string, argIndex int, content, contentType string, attachFiles []string, checkRoom func() error) (string, error) { messageContent := content index, what := -1, "--content" @@ -442,8 +443,11 @@ func readPostInput(cmd *cobra.Command, msgArgs []string, argIndex int, content, return "", err } - // Show help when invoked with no message content + // Show help when invoked with neither a message nor attachments if strings.TrimSpace(messageContent) == "" { + if len(attachFiles) > 0 { + return "", nil + } return "", missingArg(cmd, "") } return messageContent, nil @@ -734,14 +738,14 @@ You can pass either a line ID or a Basecamp line URL: } // Extract ID, project and room from URL if provided - lineID, urlProjectID, urlChatID, err := chatLineRef(app, args[0]) + lineID, urlProjectID, urlChatID, urlIsPing, err := chatLineRef(cmd.Context(), app, args[0]) if err != nil { return err } // Resolve project - use URL > flag > config, with interactive fallback projectID := *project - if projectID == "" && urlProjectID != "" { + if urlProjectID != "" && (projectID == "" || urlIsPing) { projectID = urlProjectID } if projectID == "" { @@ -1006,6 +1010,13 @@ edit to rich text.`, return output.ErrUsage(fmt.Sprintf("URL account %s does not match the configured account %s", parsedURL.AccountID, app.Config.AccountID)) } + if parsedURL != nil && parsedURL.PingID != "" { + var err error + if urlChatID, err = pingURLCampfireID(cmd.Context(), app, parsedURL.PingID); err != nil { + return err + } + } + // Resolve the chat (campfire) ID, and a project only when needed. The // PUT needs just the chat + line IDs; a project is required only to // discover the default room (no URL chat and no --room) and is @@ -1192,14 +1203,14 @@ You can pass either a line ID or a Basecamp line URL: } // Extract ID, project and room from URL if provided - lineID, urlProjectID, urlChatID, err := chatLineRef(app, args[0]) + lineID, urlProjectID, urlChatID, urlIsPing, err := chatLineRef(cmd.Context(), app, args[0]) if err != nil { return err } // Resolve project - use URL > flag > config, with interactive fallback projectID := *project - if projectID == "" && urlProjectID != "" { + if urlProjectID != "" && (projectID == "" || urlIsPing) { projectID = urlProjectID } if projectID == "" { @@ -1295,61 +1306,86 @@ You can pass either a line ID or a Basecamp line URL: // A URL is held to what chat update holds it to: a trusted Basecamp host, an // individual line (a room's /chats/{c}/lines parses with the room's ID where // the line's belongs, and would act on line {c}), and the configured account. -func chatLineRef(app *appctx.App, arg string) (lineID, projectID, chatID string, err error) { +// +// A Ping line URL names the Ping, not a room: its Campfire is looked up here +// and returned as the room, with isPing set so the caller keeps the Ping as the +// bucket whatever --in says. +func chatLineRef(ctx context.Context, app *appctx.App, arg string) (lineID, projectID, chatID string, isPing bool, err error) { if !urlarg.IsURL(arg) { lineID, projectID = extractWithProject(arg) - return lineID, projectID, "", nil + return lineID, projectID, "", false, nil } if !hostutil.IsTrustedBasecampHost(arg, app.Config.BaseURL) { - return "", "", "", output.ErrUsage("refusing untrusted host in URL — expected a Basecamp URL") + return "", "", "", false, output.ErrUsage("refusing untrusted host in URL — expected a Basecamp URL") } parsed := urlarg.Parse(arg) if parsed != nil && parsed.PingID != "" && parsed.Type != "lines" { - return "", "", "", output.ErrUsageHint("a ping URL names a ping, not a message", + return "", "", "", false, output.ErrUsageHint("a ping URL names a ping, not a message", fmt.Sprintf("Show the ping with: basecamp pings show %s; a message URL ends in @", parsed.PingID)) } if parsed == nil || parsed.Type != "lines" || parsed.IsCollection { - return "", "", "", output.ErrUsage("expected a chat-line ID or URL of the form /chats/{c}/lines/{l} or /chats/{c}@{l}") + return "", "", "", false, output.ErrUsage("expected a chat-line ID or URL of the form /chats/{c}/lines/{l} or /chats/{c}@{l}") } if parsed.AccountID != "" && app.Config.AccountID != "" && parsed.AccountID != app.Config.AccountID { - return "", "", "", output.ErrUsage(fmt.Sprintf("URL account %s does not match the configured account %s", parsed.AccountID, app.Config.AccountID)) + return "", "", "", false, output.ErrUsage(fmt.Sprintf("URL account %s does not match the configured account %s", parsed.AccountID, app.Config.AccountID)) + } + if parsed.PingID != "" { + chatID, err = pingURLCampfireID(ctx, app, parsed.PingID) + if err != nil { + return "", "", "", false, err + } + return parsed.RecordingID, parsed.ProjectID, chatID, true, nil + } + return parsed.RecordingID, parsed.ProjectID, parsed.CampfireID, false, nil +} + +// pingURLCampfireID finds the Campfire of the Ping a chat-line URL names. The +// URL carries the Ping and the line but not the Campfire, so the room the URL +// names has to be asked of the account, as a /chats/{c}@{l} URL names its own. +func pingURLCampfireID(ctx context.Context, app *appctx.App, pingID string) (string, error) { + id, err := strconv.ParseInt(pingID, 10, 64) + if err != nil { + return "", output.ErrUsage("Invalid ping ID") + } + _, campfireID, err := resolvePingCampfire(ctx, app, pingTarget{id: id}, pingLookupOnly) + if err != nil { + return "", err } - return parsed.RecordingID, parsed.ProjectID, parsed.CampfireID, nil + return strconv.FormatInt(campfireID, 10), nil } // getChatID retrieves the chat ID from a project's dock, handling multi-dock projects. func getChatID(cmd *cobra.Command, app *appctx.App, projectID string) (string, error) { id, err := getDockToolID(cmd.Context(), app, projectID, "chat", "", "chat room", "room") if err != nil { - if campfireID, ok := pingCampfireID(cmd.Context(), app, projectID, err); ok { - return campfireID, nil - } - return "", err + return pingCampfireID(cmd.Context(), app, projectID, err) } return id, nil } // pingCampfireID finds the Campfire of a Ping named where a project was // expected. A Ping is a bucket without a dock, so a project lookup that failed -// as not found is retried as a Ping; any other failure stands. -func pingCampfireID(ctx context.Context, app *appctx.App, bucketID string, lookupErr error) (string, bool) { +// as not found is retried as a Ping. The project's error stands when the Ping +// is not found either; a Ping lookup that failed any other way (Pings turned +// off, a rate limit, the network) is reported instead, since it left the answer +// unknown. +func pingCampfireID(ctx context.Context, app *appctx.App, bucketID string, lookupErr error) (string, error) { var e *output.Error if !errors.As(lookupErr, &e) || e.Code != output.CodeNotFound { - return "", false + return "", lookupErr } id, err := strconv.ParseInt(bucketID, 10, 64) if err != nil { - return "", false + return "", lookupErr } - ping, err := app.Account().Pings().Get(ctx, id) + _, campfireID, err := resolvePingCampfire(ctx, app, pingTarget{id: id}, pingLookupOnly) if err != nil { - return "", false - } - campfireID, ok := ping.CampfireID() - if !ok { - return "", false + if errors.As(err, &e) && e.Code == output.CodeNotFound { + return "", lookupErr + } + return "", err } - return strconv.FormatInt(campfireID, 10), true + return strconv.FormatInt(campfireID, 10), nil } // chatLineDisplayContent produces human-readable content for a campfire line, diff --git a/internal/commands/pings.go b/internal/commands/pings.go index 985fac983..4ffbdea04 100644 --- a/internal/commands/pings.go +++ b/internal/commands/pings.go @@ -196,8 +196,7 @@ People are names, email addresses or person IDs among the people you can ping RunE: func(cmd *cobra.Command, args []string) error { app := appctx.FromContext(cmd.Context()) if newGroup && len(args) < 2 { - return output.ErrUsageHint("--new-group needs two or more people", - "A direct ping is always the one you have with that person") + return newGroupNeedsPeople() } if err := ensureAccount(cmd, app); err != nil { return err @@ -207,6 +206,9 @@ People are names, email addresses or person IDs among the people you can ping if err != nil { return err } + if newGroup && len(people) < 2 { + return newGroupNeedsPeople() + } ping, err := startPing(cmd.Context(), app, people, newGroup) if err != nil { return err @@ -229,6 +231,11 @@ People are names, email addresses or person IDs among the people you can ping return cmd } +func newGroupNeedsPeople() error { + return output.ErrUsageHint("--new-group needs two or more people", + "A direct ping is always the one you have with that person") +} + func newPingsMessagesCmd() *cobra.Command { var limit int @@ -303,7 +310,7 @@ Use - as the message argument to read the message from stdin: } messageContent, err := readPostInput(cmd, args[1:], 1, content, *contentType, attachFiles, func() error { return nil }) - if err != nil || messageContent == "" { + if err != nil || (messageContent == "" && len(attachFiles) == 0) { return err } if err := ensureAccount(cmd, app); err != nil { @@ -435,6 +442,8 @@ const ( type pingTarget struct { id int64 person string + // accountID is the account a URL named, to hold against the resolved one. + accountID string } // parsePingTarget reads a argument without touching the network. A URL @@ -457,7 +466,7 @@ func parsePingTarget(app *appctx.App, arg string) (pingTarget, error) { if err != nil { return pingTarget{}, output.ErrUsage("Invalid ping ID") } - return pingTarget{id: id}, nil + return pingTarget{id: id, accountID: parsed.AccountID}, nil } if isNumeric(arg) { id, err := strconv.ParseInt(arg, 10, 64) @@ -474,6 +483,9 @@ func parsePingTarget(app *appctx.App, arg string) (pingTarget, error) { // resolvePing finds the Ping a target names. func resolvePing(ctx context.Context, app *appctx.App, target pingTarget, mode pingLookup) (*basecamp.Ping, error) { + if target.accountID != "" && target.accountID != app.Config.AccountID { + return nil, output.ErrUsage(fmt.Sprintf("URL account %s does not match the configured account %s", target.accountID, app.Config.AccountID)) + } if target.id != 0 { ping, err := app.Account().Pings().Get(ctx, target.id) if err != nil { @@ -500,7 +512,7 @@ func resolvePing(ctx context.Context, app *appctx.App, target pingTarget, mode p } } return nil, output.ErrNotFoundHint("Ping", target.person, - fmt.Sprintf("No ping with %s has a message yet. Start one: basecamp pings start %s", people[0].Name, quoteArg(target.person))) + fmt.Sprintf("No ping with %s has a message yet. Start one: basecamp pings start %s", people[0].Name, richtext.ShellQuote(target.person))) } func isDirectPingWith(ping *basecamp.Ping, personID int64) bool { @@ -593,13 +605,6 @@ func peopleLeftOut(requested []names.Person, ping *basecamp.Ping) []string { return left } -func quoteArg(s string) string { - if strings.ContainsAny(s, " \t\"'") { - return strconv.Quote(s) - } - return s -} - // pingMentionResolver resolves @mentions among the people in the Ping: only // they can read what is said there. func pingMentionResolver(ping *basecamp.Ping) mentionResolver { diff --git a/internal/commands/pings_test.go b/internal/commands/pings_test.go index ffd678095..758086004 100644 --- a/internal/commands/pings_test.go +++ b/internal/commands/pings_test.go @@ -7,6 +7,8 @@ import ( "fmt" "io" "net/http" + "os" + "path/filepath" "strings" "sync" "testing" @@ -562,3 +564,130 @@ func pingsErrorHint(t *testing.T, err error) string { require.ErrorAs(t, err, &e) return e.Hint } + +func (f *pingsFake) askedAbout(fragment string) bool { + f.mu.Lock() + defer f.mu.Unlock() + for _, r := range f.requests { + if strings.Contains(r, fragment) { + return true + } + } + return false +} + +func TestChatLineCommandsTakeThePingLineURLsRoomOverRoomAndIn(t *testing.T) { + const url = "https://app.basecamp.com/99999/circles/500@111" + for _, tc := range []struct { + name string + args []string + want string + }{ + {"line", []string{"line", url, "--room", "456", "--in", "777"}, "GET /99999/chats/700/lines/111"}, + {"update", []string{"update", url, "edited", "--room", "456", "--in", "777"}, "PUT /99999/chats/700/lines/111"}, + {"delete", []string{"delete", url, "--force", "--room", "456", "--in", "777"}, "DELETE /99999/chats/700/lines/111"}, + } { + t.Run(tc.name, func(t *testing.T) { + f := newPingsFakeWithPeople() + f.on("GET", "/99999/circles/500", 200, pingJSON(500, 700, "Jason Fried", false, pingPersonJSON(1001, "Jason Fried", "jason@example.com"))) + f.on("GET", "/99999/chats/700/lines/111", 200, `{"id":111,"content":"hi","creator":{"name":"Jason Fried"},"created_at":"2026-10-19T15:15:25.688Z"}`) + f.on("PUT", "/99999/chats/700/lines/111", 204, ``) + f.on("DELETE", "/99999/chats/700/lines/111", 204, ``) + f.on("GET", "/99999/recordings/111/comments.json", 200, `[]`) + + env, err := runPings(t, f, NewChatCmd, tc.args...) + require.NoError(t, err) + + assert.True(t, f.asked(tc.want), f.requests) + assert.False(t, f.askedAbout("/chats/456"), "the stale --room must not be used") + assert.False(t, f.askedAbout("/projects/777"), "--in must not replace the ping") + if tc.name != "update" { + assert.Contains(t, env.crumbs(), "basecamp chat messages --room 700 --in 500") + } + }) + } +} + +func TestChatLinePingURLReportsAFailedPingLookup(t *testing.T) { + f := newPingsFakeWithPeople() + f.on("GET", "/99999/circles/500", 403, `{"error":"Pings have been disabled"}`) + + _, err := runPings(t, f, NewChatCmd, "line", "https://app.basecamp.com/99999/circles/500@111", "--room", "456") + require.Error(t, err) + assert.Contains(t, pingsErrorHint(t, err), "turned Pings off") + assert.False(t, f.askedAbout("/chats/456")) +} + +func TestChatMessagesInAPingReportsAFailedPingLookup(t *testing.T) { + f := newPingsFakeWithPeople() + f.on("GET", "/99999/circles/500", 403, `{"error":"Pings have been disabled"}`) + + _, err := runPings(t, f, NewChatCmd, "messages", "--in", "500") + require.Error(t, err) + assert.Contains(t, pingsErrorHint(t, err), "turned Pings off") +} + +func TestPingsStartNewGroupNeedsTwoDistinctPeople(t *testing.T) { + f := newPingsFakeWithPeople() + + _, err := runPingsCmd(t, f, "start", "Jane", "jane@example.com", "--new-group") + require.Error(t, err) + assert.Contains(t, err.Error(), "--new-group needs two or more people") + assert.False(t, f.asked("POST /99999/circles.json")) +} + +func attachmentFile(t *testing.T) string { + t.Helper() + path := filepath.Join(t.TempDir(), "report.txt") + require.NoError(t, os.WriteFile(path, []byte("report"), 0o600)) + return path +} + +func TestPostsAnAttachmentWithoutAMessage(t *testing.T) { + for _, tc := range []struct { + name string + cmd func() *cobra.Command + args []string + }{ + {"pings post", NewPingsCmd, []string{"post", "500"}}, + {"chat post", NewChatCmd, []string{"post", "--room", "700"}}, + } { + t.Run(tc.name, func(t *testing.T) { + f := newPingsFakeWithPeople() + f.on("GET", "/99999/circles/500", 200, pingJSON(500, 700, "Jason Fried", false, pingPersonJSON(1001, "Jason Fried", "jason@example.com"))) + f.on("POST", "/99999/chats/700/uploads.json", 201, `{"id":888,"content":"","created_at":"2026-10-19T15:15:25.688Z"}`) + + env, err := runPings(t, f, tc.cmd, append(tc.args, "--attach", attachmentFile(t))...) + require.NoError(t, err) + + assert.True(t, f.asked("POST /99999/chats/700/uploads.json"), f.requests) + assert.False(t, f.asked("POST /99999/chats/700/lines.json"), "there is no text to post") + assert.Equal(t, "Posted 1 attachment(s)", env.Summary) + }) + } +} + +func TestPingsTargetKeepsTheURLAccountForTheResolvedOne(t *testing.T) { + f := newPingsFakeWithPeople() + app, _ := newTestAppWithTransport(t, f) + app.Config.AccountID = "" + + target, err := parsePingTarget(app, "https://app.basecamp.com/11111/circles/500") + require.NoError(t, err, "with no account configured the URL cannot be compared yet") + + app.Config.AccountID = "99999" // what ensureAccount settles on + _, err = resolvePing(context.Background(), app, target, pingLookupOnly) + require.Error(t, err) + assert.Contains(t, err.Error(), "URL account 11111 does not match the configured account 99999") + assert.Empty(t, f.requests) +} + +func TestPingsHintQuotesThePersonForTheShell(t *testing.T) { + f := newPingsFake() + f.on("GET", "/99999/circles/people.json", 200, `[{"id":1005,"name":"Sam $HOME","email_address":"sam@example.com","attachable_sgid":"sgid-1005"}]`) + f.on("GET", "/99999/circles.json", 200, `[]`) + + _, err := runPingsCmd(t, f, "messages", "Sam $HOME") + require.Error(t, err) + assert.Contains(t, pingsErrorHint(t, err), "basecamp pings start 'Sam $HOME'") +} diff --git a/internal/commands/show.go b/internal/commands/show.go index 09957c595..b86cd2341 100644 --- a/internal/commands/show.go +++ b/internal/commands/show.go @@ -358,10 +358,7 @@ func showPingURL(cmd *cobra.Command, app *appctx.App, parsed *urlarg.Parsed) err if err := ensureAccount(cmd, app); err != nil { return err } - if parsed.AccountID != "" && parsed.AccountID != app.Config.AccountID { - return output.ErrUsage(fmt.Sprintf("URL account %s does not match the configured account %s", parsed.AccountID, app.Config.AccountID)) - } - ping, err := resolvePing(cmd.Context(), app, pingTarget{id: id}, pingLookupOnly) + ping, err := resolvePing(cmd.Context(), app, pingTarget{id: id, accountID: parsed.AccountID}, pingLookupOnly) if err != nil { return err } diff --git a/internal/connector/admission/policy.go b/internal/connector/admission/policy.go index 99ca8927d..e0b9e6495 100644 --- a/internal/connector/admission/policy.go +++ b/internal/connector/admission/policy.go @@ -412,9 +412,9 @@ func (p Policy) validateAgents() error { return fmt.Errorf("admission: person %d is both a trusted person and an allowed agent", id) } } - for name, cap := range map[string]int{"agent_thread_cap": p.Trust.AgentThreadCap, "agent_daily_cap": p.Trust.AgentDailyCap} { - if cap < 0 || cap > MaxAgentCap { - return fmt.Errorf("admission: %s %d is outside 1 to %d", name, cap, MaxAgentCap) + for name, limit := range map[string]int{"agent_thread_cap": p.Trust.AgentThreadCap, "agent_daily_cap": p.Trust.AgentDailyCap} { + if limit < 0 || limit > MaxAgentCap { + return fmt.Errorf("admission: %s %d is outside 1 to %d", name, limit, MaxAgentCap) } } return nil From 0f177dc7f349adf1a58d6df0d3a086eb7cfee449 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Mon, 19 Oct 2026 21:41:10 -0400 Subject: [PATCH 7/7] Mention only a Ping's people when editing its line chat update on a Ping line URL resolved @mentions with the project resolver, scoped to the Ping's id as if it were a project: an unmatched name fetched /projects/{ping}/people.json and the 404 aborted the edit, and a namesake elsewhere in the account could be mentioned. The edit now keeps the Ping it looked up and resolves mentions among its people, with the Ping's unresolved-mention notice, as pings post does. --- internal/commands/chat.go | 29 +++++++++++++-------- internal/commands/pings_test.go | 45 +++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 10 deletions(-) diff --git a/internal/commands/chat.go b/internal/commands/chat.go index 1df3b2cba..48788c233 100644 --- a/internal/commands/chat.go +++ b/internal/commands/chat.go @@ -1010,9 +1010,10 @@ edit to rich text.`, return output.ErrUsage(fmt.Sprintf("URL account %s does not match the configured account %s", parsedURL.AccountID, app.Config.AccountID)) } + var ping *basecamp.Ping if parsedURL != nil && parsedURL.PingID != "" { var err error - if urlChatID, err = pingURLCampfireID(cmd.Context(), app, parsedURL.PingID); err != nil { + if ping, urlChatID, err = pingURLCampfire(cmd.Context(), app, parsedURL.PingID); err != nil { return err } } @@ -1087,12 +1088,20 @@ edit to rich text.`, if ct == "" { messageContent = richtext.MarkdownToHTML(messageContent) } - result, resolveErr := resolveMentions(cmd.Context(), app.Names, mentionScope(app, urlProjectID, resolvedProjectID, *project, app.Flags.Project), messageContent) + resolve := func(ctx context.Context, html string) (richtext.MentionResult, error) { + return resolveMentions(ctx, app.Names, mentionScope(app, urlProjectID, resolvedProjectID, *project, app.Flags.Project), html) + } + notice := unresolvedMentionWarning + if ping != nil { + // A Ping is no project, and only its people can read the line. + resolve, notice = pingMentionResolver(ping), pingNav(ping.ID).unresolvedMentions + } + result, resolveErr := resolve(cmd.Context(), messageContent) if resolveErr != nil { return resolveErr } messageContent = result.HTML - mentionNotice = unresolvedMentionWarning(result.Unresolved) + mentionNotice = notice(result.Unresolved) } if err := app.Account().Campfires().UpdateLine(cmd.Context(), chatIDInt, lineIDInt, messageContent); err != nil { @@ -1330,7 +1339,7 @@ func chatLineRef(ctx context.Context, app *appctx.App, arg string) (lineID, proj return "", "", "", false, output.ErrUsage(fmt.Sprintf("URL account %s does not match the configured account %s", parsed.AccountID, app.Config.AccountID)) } if parsed.PingID != "" { - chatID, err = pingURLCampfireID(ctx, app, parsed.PingID) + _, chatID, err = pingURLCampfire(ctx, app, parsed.PingID) if err != nil { return "", "", "", false, err } @@ -1339,19 +1348,19 @@ func chatLineRef(ctx context.Context, app *appctx.App, arg string) (lineID, proj return parsed.RecordingID, parsed.ProjectID, parsed.CampfireID, false, nil } -// pingURLCampfireID finds the Campfire of the Ping a chat-line URL names. The +// pingURLCampfire finds the Ping a chat-line URL names, and its Campfire. The // URL carries the Ping and the line but not the Campfire, so the room the URL // names has to be asked of the account, as a /chats/{c}@{l} URL names its own. -func pingURLCampfireID(ctx context.Context, app *appctx.App, pingID string) (string, error) { +func pingURLCampfire(ctx context.Context, app *appctx.App, pingID string) (*basecamp.Ping, string, error) { id, err := strconv.ParseInt(pingID, 10, 64) if err != nil { - return "", output.ErrUsage("Invalid ping ID") + return nil, "", output.ErrUsage("Invalid ping ID") } - _, campfireID, err := resolvePingCampfire(ctx, app, pingTarget{id: id}, pingLookupOnly) + ping, campfireID, err := resolvePingCampfire(ctx, app, pingTarget{id: id}, pingLookupOnly) if err != nil { - return "", err + return nil, "", err } - return strconv.FormatInt(campfireID, 10), nil + return ping, strconv.FormatInt(campfireID, 10), nil } // getChatID retrieves the chat ID from a project's dock, handling multi-dock projects. diff --git a/internal/commands/pings_test.go b/internal/commands/pings_test.go index 758086004..3bd87fd13 100644 --- a/internal/commands/pings_test.go +++ b/internal/commands/pings_test.go @@ -691,3 +691,48 @@ func TestPingsHintQuotesThePersonForTheShell(t *testing.T) { require.Error(t, err) assert.Contains(t, pingsErrorHint(t, err), "basecamp pings start 'Sam $HOME'") } + +// Editing a Ping line resolves @mentions among the Ping's people, as posting +// there does: a name nobody in the Ping has stays text, and the account's +// people are not asked about, since the Ping is no project. +func TestChatUpdateOnAPingLineMentionsOnlyThePingsPeople(t *testing.T) { + const url = "https://app.basecamp.com/99999/circles/500@111" + const put = "PUT /99999/chats/700/lines/111" + for _, tc := range []struct { + name string + message string + wantSGID string + wantNotice string + wantNoSGIDs []string + }{ + {"a name in the Ping", "thanks @Jason", "sgid-1001", "", []string{"sgid-1004"}}, + {"a name nobody has", "thanks @Nobody", "", "@Nobody", nil}, + {"a namesake outside the Ping", "thanks @Jane.Smith", "", "@Jane.Smith", []string{"sgid-1002"}}, + } { + t.Run(tc.name, func(t *testing.T) { + f := newPingsFakeWithPeople() + f.on("GET", "/99999/circles/500", 200, pingJSON(500, 700, "Jason Fried", false, pingPersonJSON(1001, "Jason Fried", "jason@example.com"))) + f.on("PUT", "/99999/chats/700/lines/111", 204, ``) + f.on("GET", "/99999/chats/700/lines/111", 200, `{"id":111,"content":"edited","creator":{"name":"Jason Fried"},"created_at":"2026-10-19T15:15:25.688Z"}`) + + env, err := runPings(t, f, NewChatCmd, "update", url, tc.message) + require.NoError(t, err) + + require.True(t, f.asked(put), f.requests) + content, _ := f.body(put)["content"].(string) + if tc.wantSGID != "" { + assert.Contains(t, content, tc.wantSGID) + } + for _, sgid := range tc.wantNoSGIDs { + assert.NotContains(t, content, sgid, "only the Ping's people can be mentioned") + } + if tc.wantNotice != "" { + assert.Contains(t, content, tc.wantNotice, "an unresolved mention stays text") + assert.Contains(t, env.Notice, tc.wantNotice) + assert.Contains(t, env.Notice, "only people in this ping") + } + assert.False(t, f.askedAbout("/projects/500"), "a Ping is no project") + assert.False(t, f.askedAbout("people.json"), "the account's people are not asked about") + }) + } +}