From fe9134c163c399228779a0b4fb086bd95f45d87a Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Wed, 16 Sep 2026 05:21:47 -0700 Subject: [PATCH 01/29] Pin the SDK to the main commit carrying the event-feed operations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The account event feed's three operations — poll events, poll the inbox, mint a stream ticket — are not in an SDK release yet, so the pin is a basecamp-sdk main pseudo-version until the next tag. Re-syncing the vendored MCP model brings the EventFeed tag with it, and an SDK tag no DomainSpec claims fails the catalog build. It gets its own domain rather than joining the account grab-bag: the feed is a resumable lane with its own contract — a position and a continuation URL instead of page numbers — and the blurb is where an MCP client learns that. --- go.mod | 2 +- go.sum | 4 +- internal/mcpserver/catalog_test.go | 4 +- internal/mcpserver/domains.go | 5 + internal/mcpserver/model/PROVENANCE.json | 4 +- internal/mcpserver/model/behavior-model.json | 44 +- internal/mcpserver/model/openapi.json | 761 +++++++++++++++++- internal/mcpserver/server_test.go | 2 +- .../mcpserver/testdata/catalog_snapshot.txt | 11 + internal/version/sdk-provenance.json | 6 +- 10 files changed, 787 insertions(+), 56 deletions(-) diff --git a/go.mod b/go.mod index fd42e4400..b956437aa 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( charm.land/bubbles/v2 v2.2.1 charm.land/bubbletea/v2 v2.0.9 charm.land/lipgloss/v2 v2.0.6 - github.com/basecamp/basecamp-sdk/go v0.18.1-0.20260916083520-5acb2f9aaca9 + github.com/basecamp/basecamp-sdk/go v0.18.1-0.20260916113525-e58b5823a8ca 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 883c265ea..069fd05e9 100644 --- a/go.sum +++ b/go.sum @@ -87,8 +87,8 @@ github.com/aymanbagabas/go-udiff v0.4.1 h1:OEIrQ8maEeDBXQDoGCbbTTXYJMYRCRO1fnodZ github.com/aymanbagabas/go-udiff v0.4.1/go.mod h1:0L9PGwj20lrtmEMeyw4WKJ/TMyDtvAoK9bf2u/mNo3w= github.com/aymerick/douceur v0.2.0 h1:Mv+mAeH1Q+n9Fr+oyamOlAkUNPWPlA8PPGR0QAaYuPk= github.com/aymerick/douceur v0.2.0/go.mod h1:wlT5vV2O3h55X9m7iVYN0TBM0NH/MmbLnd30/FjWUq4= -github.com/basecamp/basecamp-sdk/go v0.18.1-0.20260916083520-5acb2f9aaca9 h1:W0R9T9iy40bY+RvGIozG7Sz9e456yl3QDGOFDniCaH4= -github.com/basecamp/basecamp-sdk/go v0.18.1-0.20260916083520-5acb2f9aaca9/go.mod h1:kIBDYwPMMD59PadNGxpH0YTQuI+blFPZ8MelGI0RK5Q= +github.com/basecamp/basecamp-sdk/go v0.18.1-0.20260916113525-e58b5823a8ca h1:blH50bacuga0dFf/YAddx+bRL4ovMgqWhz/9308Jozk= +github.com/basecamp/basecamp-sdk/go v0.18.1-0.20260916113525-e58b5823a8ca/go.mod h1:kIBDYwPMMD59PadNGxpH0YTQuI+blFPZ8MelGI0RK5Q= github.com/basecamp/cli v0.2.2-0.20260828230226-767413fc712d h1:jAzDrCCzDpIwhbFT1xVVs0z2xpXoDEkomHfKB2bUUp8= github.com/basecamp/cli v0.2.2-0.20260828230226-767413fc712d/go.mod h1:iTBTaWvsPEFIcZfkxQHEfISyJ6sZ7036K6bNx0RY3EE= github.com/basecamp/mcp v0.0.0-20260828100356-2d6f44b51e9d h1:zEQVGq1x1nhKMZ2TudFAcSJ32CHT8richI1vQakIKz4= diff --git a/internal/mcpserver/catalog_test.go b/internal/mcpserver/catalog_test.go index 655286bdc..7dc07ce01 100644 --- a/internal/mcpserver/catalog_test.go +++ b/internal/mcpserver/catalog_test.go @@ -33,7 +33,7 @@ func TestCatalogServesCuratedDomains(t *testing.T) { "basecamp_projects", "basecamp_todos", "basecamp_cards", "basecamp_messages", "basecamp_campfires", "basecamp_boosts", "basecamp_schedules", "basecamp_files", "basecamp_people", "basecamp_automation", "basecamp_reports", "basecamp_everything", - "basecamp_clientside", "basecamp_forwards", "basecamp_account", + "basecamp_eventfeed", "basecamp_clientside", "basecamp_forwards", "basecamp_account", "basecamp_recordings", }, tools) } @@ -66,7 +66,7 @@ func TestCatalogExcludesBinaryUploads(t *testing.T) { assert.False(t, excluded[op.ID], "operation %q should be excluded from the vendored model", op.ID) } } - assert.Equal(t, 259, model, "served model operation count") + assert.Equal(t, 262, model, "served model operation count") assert.Equal(t, 1, composite, "served composite operation count") } diff --git a/internal/mcpserver/domains.go b/internal/mcpserver/domains.go index 07f16428d..cd69490e7 100644 --- a/internal/mcpserver/domains.go +++ b/internal/mcpserver/domains.go @@ -79,6 +79,11 @@ var DomainSpecs = []catalog.DomainSpec{ Tags: []string{"Everything"}, Blurb: "Account-wide feeds: every checkin, comment, file, forward, and message, and cards and todos filtered by state (open, completed, overdue, unassigned, no due date, not now).", }, + { + Key: "eventfeed", + Tags: []string{"EventFeed"}, + Blurb: "The account event feed: poll events from a resumable position, poll the agent inbox of addressed items, and mint a ticket for the live stream. Pages carry a position and a continuation URL rather than page numbers; deduplicate events by id and inbox items by addressing_id, and refetch the referenced recording before acting on it.", + }, { Key: "clientside", Tags: []string{"ClientFeatures"}, diff --git a/internal/mcpserver/model/PROVENANCE.json b/internal/mcpserver/model/PROVENANCE.json index 39f7332c3..22751eba4 100644 --- a/internal/mcpserver/model/PROVENANCE.json +++ b/internal/mcpserver/model/PROVENANCE.json @@ -1,7 +1,7 @@ { "source": "github.com/basecamp/basecamp-sdk", - "commit": "5acb2f9aaca98af831918c5d4c4a4b22843c9a54", - "ref": "go/v0.18.0-44-g5acb2f9a", + "commit": "e58b5823a8ca79d42bb008b281679d8fcbebebfc", + "ref": "go/v0.18.0-50-ge58b5823", "files": ["behavior-model.json", "openapi.json"], "synced_by": "scripts/sync-mcp-model.sh", "patches": "binary-upload operations dropped (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 b74aca9f7..92ad368f4 100644 --- a/internal/mcpserver/model/behavior-model.json +++ b/internal/mcpserver/model/behavior-model.json @@ -294,6 +294,18 @@ ] } }, + "CreateStreamTicket": { + "idempotent": true, + "retry": { + "max": 3, + "base_delay_ms": 1000, + "backoff": "exponential", + "retry_on": [ + 429, + 503 + ] + } + }, "CreateTemplate": { "retry": { "max": 2, @@ -2532,6 +2544,30 @@ ] } }, + "PollEvents": { + "readonly": true, + "retry": { + "max": 3, + "base_delay_ms": 1000, + "backoff": "exponential", + "retry_on": [ + 429, + 503 + ] + } + }, + "PollInbox": { + "readonly": true, + "retry": { + "max": 3, + "base_delay_ms": 1000, + "backoff": "exponential", + "retry_on": [ + 429, + 503 + ] + } + }, "PrioritizeAssignment": { "idempotent": true, "retry": { @@ -3356,6 +3392,10 @@ "$.title", "$.company_name" ], + "CreateStreamTicketOutput": [ + "$.ticket", + "$.url" + ], "MyAssignmentAssignee": [ "$.name", "$.avatar_url" @@ -3399,6 +3439,8 @@ "PersonBio", "PersonLocation", "PersonName", - "PersonTitle" + "PersonTitle", + "StreamTicket", + "StreamTicketUrl" ] } diff --git a/internal/mcpserver/model/openapi.json b/internal/mcpserver/model/openapi.json index 5bbe95700..0af60840a 100644 --- a/internal/mcpserver/model/openapi.json +++ b/internal/mcpserver/model/openapi.json @@ -8375,6 +8375,277 @@ } } }, + "/{accountId}/events.json": { + "get": { + "description": "Poll the account event feed for events after a position (oldest first, strict event-id order, up to 100 per page).\n\n**Entry.** With neither `since` nor `position` the feed begins at the present\n(equivalent to `since=now`). `since=` starts after that id and\n`since=0` replays all served history back to the feed's epoch; `since` is a\nsigned 64-bit integer written in decimal, or the literal `now`. `position`\nresumes from a token a previous page issued \u2014 signed, opaque, bound to the\naccount and the filter set.\n\n**Pagination**: the body envelope, not the Link header. `position` is the\ndurable cursor (persist it only after processing the page's events); `next`\nis an absolute continuation URL present only while this walk has more to\nserve. Not wired into the generic Link paginator \u2014 see the section note.\n\n**Errors.** 400 for a malformed position (resume with `since=`) or a malformed\nfilter (the body names the filter; a position reset will not help) \u2014 both the\nflat `{error}` body. 409 (FeedFilterMismatchError) when the position was\nminted for a different filter set. 410 (FeedPositionGoneError) when the\nposition predates the feed's epoch; follow its `resume` URL.", + "operationId": "PollEvents", + "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": "since", + "in": "query", + "description": "Entry point: a decimal event id (start after it; `0` replays served\nhistory back to the epoch), or the literal `now` (skip history). Mutually\nexclusive with `position` in practice; omit both to enter at the present.", + "schema": { + "type": "string", + "description": "Entry point: a decimal event id (start after it; `0` replays served\nhistory back to the epoch), or the literal `now` (skip history). Mutually\nexclusive with `position` in practice; omit both to enter at the present." + } + }, + { + "name": "position", + "in": "query", + "description": "Resume token from a previous page's `position`. Opaque and signed; never\nconstructed or parsed client-side.", + "schema": { + "type": "string", + "description": "Resume token from a previous page's `position`. Opaque and signed; never\nconstructed or parsed client-side." + } + }, + { + "name": "types", + "in": "query", + "description": "Comma-separated event types from the catalog (e.g. `message.created,comment.created`).", + "schema": { + "type": "string", + "description": "Comma-separated event types from the catalog (e.g. `message.created,comment.created`)." + } + }, + { + "name": "buckets", + "in": "query", + "description": "Comma-separated bucket (project) ids, at most 100.", + "schema": { + "type": "string", + "description": "Comma-separated bucket (project) ids, at most 100." + } + }, + { + "name": "creators", + "in": "query", + "description": "Comma-separated creator person ids, at most 100.", + "schema": { + "type": "string", + "description": "Comma-separated creator person ids, at most 100." + } + }, + { + "name": "performers", + "in": "query", + "description": "Comma-separated effective-performer ids (the agent on a delegated action,\nelse the creator), at most 100. The literal `self` means the request's own\neffective actor and is resolved server-side before filtering.", + "schema": { + "type": "string", + "description": "Comma-separated effective-performer ids (the agent on a delegated action,\nelse the creator), at most 100. The literal `self` means the request's own\neffective actor and is resolved server-side before filtering." + } + }, + { + "name": "exclude_performers", + "in": "query", + "description": "Comma-separated effective-performer ids to exclude, at most 100; `self`\nas on `performers`. `exclude_performers=self` is the loop guard for an\nagent that acts on what it hears.", + "schema": { + "type": "string", + "description": "Comma-separated effective-performer ids to exclude, at most 100; `self`\nas on `performers`. `exclude_performers=self` is the loop guard for an\nagent that acts on what it hears." + } + }, + { + "name": "actor_types", + "in": "query", + "description": "Comma-separated actor kinds: `agent`, `person`, or both. A filter, not a\ndefault \u2014 agent activity is real account activity.", + "schema": { + "type": "string", + "description": "Comma-separated actor kinds: `agent`, `person`, or both. A filter, not a\ndefault \u2014 agent activity is real account activity." + } + } + ], + "responses": { + "200": { + "description": "PollEvents 200 response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/PollEventsResponseContent" + } + } + } + }, + "400": { + "description": "BadRequestError 400 response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/BadRequestErrorResponseContent" + } + } + } + }, + "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" + } + } + } + }, + "409": { + "description": "FeedFilterMismatchError 409 response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/FeedFilterMismatchErrorResponseContent" + } + } + } + }, + "410": { + "description": "FeedPositionGoneError 410 response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/FeedPositionGoneErrorResponseContent" + } + } + } + }, + "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": [ + "EventFeed" + ], + "x-basecamp-retry": { + "maxAttempts": 3, + "baseDelayMs": 1000, + "backoff": "exponential", + "retryOn": [ + 429, + 503 + ] + } + } + }, + "/{accountId}/events/stream_ticket.json": { + "post": { + "description": "Mint a short-lived stream ticket and the exact WebSocket URL to open a live event stream with.\n\nThe ticket is signed and lives about two minutes; the response's `url` is\nthe one to connect to. Connect to `url` verbatim \u2014 never assemble the WebSocket URL (scheme, host,\npath, account prefix) client-side; the topology is the server's to change.\nMint a fresh ticket for every connection attempt: tickets expire and are not\nrefreshed by an open socket. The mint is a stateless signed capability with\nno server-side consumption, so a replayed POST is harmless and the operation\nis marked idempotent (safe to retry) \u2014 deliberately not a claim that two\nmints return the same ticket. The ticket is a replayable bearer credential\nwithin its window; `ticket` and `url` are marked sensitive (see StreamTicket).\n\nServes agent principals as well as people: an agent's client-credentials\ntoken can mint tickets for its own live stream. A ticket minted on a\ndelegated request carries its agent, so `self` on the socket it opens\nresolves to that agent.", + "operationId": "CreateStreamTicket", + "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": "CreateStreamTicket 200 response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/CreateStreamTicketResponseContent" + } + } + } + }, + "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": [ + "EventFeed" + ], + "x-basecamp-idempotent": { + "natural": true + }, + "x-basecamp-retry": { + "maxAttempts": 3, + "baseDelayMs": 1000, + "backoff": "exponential", + "retryOn": [ + 429, + 503 + ] + } + } + }, "/{accountId}/files.json": { "get": { "description": "Get every file recording across all accessible projects, newest-first (paginated).\nHeterogeneous: uploads and Basecamp documents carry their\nstandard recording shapes, while rich-text attachments are wrapped in a\nrecording envelope plus an `attachable_sgid` and blob metadata. Modeled as\nan optional-field superset (EverythingFile) so one element type decodes any\nvariant.", @@ -9008,37 +9279,192 @@ } } }, - "required": true - }, - "parameters": [ + "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": "googleDocumentId", + "in": "path", + "schema": { + "type": "integer", + "format": "int64" + }, + "required": true + } + ], + "responses": { + "200": { + "description": "UpdateGoogleDocument 200 response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/UpdateGoogleDocumentResponseContent" + } + } + } + }, + "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": "FieldValidationError 422 response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/FieldValidationErrorResponseContent" + } + } + } + }, + "500": { + "description": "InternalServerError 500 response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/InternalServerErrorResponseContent" + } + } + } + } + }, + "tags": [ + "Files" + ], + "x-basecamp-idempotent": { + "natural": true + }, + "x-basecamp-retry": { + "maxAttempts": 3, + "baseDelayMs": 1000, + "backoff": "exponential", + "retryOn": [ + 429, + 503 + ] + }, + "x-basecamp-write-semantics": { + "mode": "replace", + "clearsOmitted": true + } + } + }, + "/{accountId}/inbox.json": { + "get": { + "description": "Poll the authenticated agent's inbox for the items that addressed it (oldest first, strict item-id order); people receive 403.\n\nThe inbox is the low-noise \"someone addressed you\" lane as its own resource\nrather than a filter over the account feed. **Agents only for now**: any\nother principal receives 403.\n\nAn item is a first-class delivery with its own identity: one event can\naddress the same principal for several reasons, and each reason is its own\nitem. Deduplicate by `addressing_id`, never by event id. Items are never\nself-addressed, are kept for 30 days, and are dropped at read time when the\nevent is no longer readable.\n\n**Entry**: `since=0` replays the earliest retained items, `since=now` enters\nat the present, `position` resumes. Inbox positions are bound to the\naccount, the principal, and the filter set, and are never interchangeable\nwith feed positions.\n\n**Pagination**: the body envelope (`items`, `position`, `next`), exactly as\nPollEvents \u2014 not the Link header, and not the generic paginator.\n\n**Errors** follow PollEvents, except that 403 carries no body \u2014 the agent\nguard's bare `head :forbidden` (BareForbiddenError) \u2014 and that 410\n(FeedPositionGoneError) here means the position fell behind the retention\nwindow: `epoch_after_id` is absent and `resume` re-enters at `since=0`,\nthe earliest retained item.", + "operationId": "PollInbox", + "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": "since", + "in": "query", + "description": "Entry point: `0` (earliest retained), `now` (present), or a decimal item\nid to start after.", + "schema": { + "type": "string", + "description": "Entry point: `0` (earliest retained), `now` (present), or a decimal item\nid to start after." + } + }, + { + "name": "position", + "in": "query", + "description": "Resume token from a previous inbox page's `position`.", + "schema": { + "type": "string", + "description": "Resume token from a previous inbox page's `position`." + } + }, { - "name": "accountId", - "in": "path", - "description": "Basecamp account ID (numeric string)", + "name": "reasons", + "in": "query", + "description": "Comma-separated addressing reasons: `mentioned`, `assigned`, `subscribed`,\n`watched`, `pinged`, `boosted`.", "schema": { "type": "string", - "pattern": "^[0-9]+$", - "description": "Basecamp account ID (numeric string)" - }, - "required": true + "description": "Comma-separated addressing reasons: `mentioned`, `assigned`, `subscribed`,\n`watched`, `pinged`, `boosted`." + } }, { - "name": "googleDocumentId", - "in": "path", + "name": "types", + "in": "query", + "description": "Comma-separated event types, as a narrowing filter.", "schema": { - "type": "integer", - "format": "int64" - }, - "required": true + "type": "string", + "description": "Comma-separated event types, as a narrowing filter." + } + }, + { + "name": "buckets", + "in": "query", + "description": "Comma-separated bucket ids, as a narrowing filter (at most 100).", + "schema": { + "type": "string", + "description": "Comma-separated bucket ids, as a narrowing filter (at most 100)." + } } ], "responses": { "200": { - "description": "UpdateGoogleDocument 200 response", + "description": "PollInbox 200 response", "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/UpdateGoogleDocumentResponseContent" + "$ref": "#/components/schemas/PollInboxResponseContent" + } + } + } + }, + "400": { + "description": "BadRequestError 400 response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/BadRequestErrorResponseContent" } } } @@ -9054,31 +9480,34 @@ } }, "403": { - "description": "ForbiddenError 403 response", + "description": "BareForbiddenError 403 response" + }, + "409": { + "description": "FeedFilterMismatchError 409 response", "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/ForbiddenErrorResponseContent" + "$ref": "#/components/schemas/FeedFilterMismatchErrorResponseContent" } } } }, - "404": { - "description": "NotFoundError 404 response", + "410": { + "description": "FeedPositionGoneError 410 response", "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/NotFoundErrorResponseContent" + "$ref": "#/components/schemas/FeedPositionGoneErrorResponseContent" } } } }, - "422": { - "description": "FieldValidationError 422 response", + "429": { + "description": "RateLimitError 429 response", "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/FieldValidationErrorResponseContent" + "$ref": "#/components/schemas/RateLimitErrorResponseContent" } } } @@ -9095,11 +9524,8 @@ } }, "tags": [ - "Files" + "EventFeed" ], - "x-basecamp-idempotent": { - "natural": true - }, "x-basecamp-retry": { "maxAttempts": 3, "baseDelayMs": 1000, @@ -9108,10 +9534,6 @@ 429, 503 ] - }, - "x-basecamp-write-semantics": { - "mode": "replace", - "clearsOmitted": true } } }, @@ -29987,6 +30409,40 @@ "CreateScheduleEntryResponseContent": { "$ref": "#/components/schemas/ScheduleEntry" }, + "CreateStreamTicketResponseContent": { + "type": "object", + "description": "A minted stream ticket. `url` already carries the ticket in its query\nstring; connect to it verbatim.", + "properties": { + "ticket": { + "type": "string", + "description": "The signed ticket \u2014 an opaque bearer credential, never parsed or logged.", + "format": "password", + "x-basecamp-sensitive": { + "category": "credential", + "redact": true + } + }, + "expires_in": { + "type": "integer", + "description": "Ticket lifetime in seconds (about 120). Server-owned: expiry is arbitrated\nby the server, so mint fresh per connection rather than scheduling on it.", + "format": "int32" + }, + "url": { + "type": "string", + "description": "The exact WebSocket URL to connect to, ticket included. Sensitive because\nit embeds the ticket.", + "format": "password", + "x-basecamp-sensitive": { + "category": "credential", + "redact": true + } + } + }, + "required": [ + "expires_in", + "ticket", + "url" + ] + }, "CreateTemplateLibraryCopyRequestContent": { "type": "object", "properties": { @@ -30654,6 +31110,9 @@ "creator": { "$ref": "#/components/schemas/Person" }, + "performed_by": { + "$ref": "#/components/schemas/Person" + }, "boosts_count": { "type": "integer", "format": "int32" @@ -30842,6 +31301,118 @@ } } }, + "FeedEvent": { + "type": "object", + "description": "One feed row: a thin pointer, not resource state. Refetch the referenced\nrecording through the canonical resource APIs before acting on it. The same\nshape rides the inbox (`InboxItem.event`) and, with two transport-only\nextras, the live stream.", + "properties": { + "id": { + "type": "integer", + "description": "Feed-global event id; the poll lane serves ids in strict ascending order.", + "format": "int64" + }, + "kind": { + "type": "string", + "description": "Event kind (e.g. `message_created`)." + }, + "action": { + "type": "string", + "description": "The action that produced the event (e.g. `created`)." + }, + "event_type": { + "type": "string", + "description": "Cataloged event type (e.g. `message.created`). Only cataloged types are served." + }, + "bucket_id": { + "type": "integer", + "description": "The bucket (project or circle) the recording lives in.", + "format": "int64" + }, + "creator_id": { + "type": "integer", + "description": "The person the action is attributed to.", + "format": "int64" + }, + "performed_by_id": { + "type": [ + "integer", + "null" + ], + "description": "The agent that carried out a delegated action; `null` on the wire when the\naction was performed directly. The effective performer \u2014 used by the\n`performers`/`exclude_performers` filters \u2014 is this when present, else\n`creator_id`.", + "format": "int64", + "x-go-type": "int64" + }, + "recording_id": { + "type": "integer", + "description": "The recording the event references.", + "format": "int64" + }, + "created_at": { + "type": "string", + "x-go-type": "time.Time", + "x-go-type-import": { + "path": "time" + } + }, + "details": { + "description": "Type-specific details, carried verbatim as a JSON document \u2014 present only\nfor the types that publish one, absent (not empty) for every other type.\n`boost.created` publishes `boost_id`, plus `boosted_event_id` and\n`boosted_event_type`, both `null` for a boost on the recording itself\n(`boosted_event_type` also `null` when the boosted event's kind is not\ncataloged); `card.moved` publishes `column_id` and `previous_column_id`,\nthe containing columns (going on hold within a column reports the same\ncolumn twice). Verbatim on purpose: the connector's push lane delivers\nthe same object, and a typed projection would drop explicit nulls and\nany member a newly cataloged type adds, making the two lanes disagree.\nGo carries it as `json.RawMessage`; TypeScript `unknown`; Python `Any`;\nRuby a Hash; Kotlin `JsonElement`; Rust `serde_json::Value`; Swift the\nSDK's `JSONValue` (numbers as `Double`).", + "x-go-type": "json.RawMessage" + } + }, + "required": [ + "action", + "bucket_id", + "created_at", + "creator_id", + "event_type", + "id", + "kind", + "recording_id" + ] + }, + "FeedFilterMismatchErrorResponseContent": { + "type": "object", + "description": "409 from the event feed's poll lanes (PollEvents, PollInbox): the held\n`position` was minted for a different filter set than the request presented.\nPositions are bound to their filter set; re-enter with `since=` or\n`since=now` to acknowledge the filter change. Both digests are the bare\n16-lowercase-hex `srv2` filter digest BC3 publishes in\ndoc/api/sections/event_feed.md (\"Filter digests\").", + "properties": { + "error": { + "type": "string" + }, + "position_digest": { + "type": "string", + "description": "The digest of the filter set the position was minted for." + }, + "filters_digest": { + "type": "string", + "description": "The digest of the filter set this request presented." + } + }, + "required": [ + "error", + "filters_digest", + "position_digest" + ] + }, + "FeedPositionGoneErrorResponseContent": { + "type": "object", + "description": "410 from the event feed's poll lanes: the held `position` predates what the\nlane can still serve. On PollEvents that is the feed's epoch \u2014 an operational\nfence that can be raised \u2014 and `epoch_after_id` names it; on PollInbox it is\nthe inbox's 30-day retention window, and `epoch_after_id` is absent. Either\nway `resume` is an absolute URL that re-enters the same lane with the\nrequest's canonical filters preserved: at the epoch (`since=`)\nfor the feed, at the earliest retained item (`since=0`) for the inbox.\nConsumers validate the URL (same origin as the API base, no scheme downgrade)\nbefore following it \u2014 SPEC.md \u00a723 \"Continuation and Resume URL Validation\".", + "properties": { + "error": { + "type": "string" + }, + "epoch_after_id": { + "type": "integer", + "description": "The feed's epoch: the event id after which history is servable. Feed only.", + "format": "int64" + }, + "resume": { + "type": "string", + "description": "Absolute re-entry URL for the same lane, filters preserved." + } + }, + "required": [ + "error", + "resume" + ] + }, "FieldErrorMap": { "type": "object", "additionalProperties": { @@ -32235,6 +32806,36 @@ "visible_to_clients" ] }, + "InboxItem": { + "type": "object", + "description": "One addressed delivery. `addressing_id` is the item's own identity \u2014 the\ndedupe key \u2014 since one event can address the same principal for several\nreasons.", + "properties": { + "addressing_id": { + "type": "integer", + "format": "int64" + }, + "reason": { + "type": "string", + "description": "Why the principal was addressed: `mentioned`, `assigned`, `subscribed`,\n`watched`, `pinged`, or `boosted`." + }, + "addressed_at": { + "type": "string", + "x-go-type": "time.Time", + "x-go-type-import": { + "path": "time" + } + }, + "event": { + "$ref": "#/components/schemas/FeedEvent" + } + }, + "required": [ + "addressed_at", + "addressing_id", + "event", + "reason" + ] + }, "InternalServerErrorResponseContent": { "type": "object", "properties": { @@ -33267,48 +33868,68 @@ } }, "email_address": { - "type": "string", + "type": [ + "string", + "null" + ], "format": "password", "x-basecamp-sensitive": { "category": "pii", "redact": true - } + }, + "x-go-type": "string" }, "personable_type": { "type": "string" }, "title": { - "type": "string", + "type": [ + "string", + "null" + ], "format": "password", "x-basecamp-sensitive": { "category": "pii", "redact": false - } + }, + "x-go-type": "string" }, "bio": { - "type": "string", + "type": [ + "string", + "null" + ], "format": "password", "x-basecamp-sensitive": { "category": "pii", "redact": false - } + }, + "x-go-type": "string" }, "tagline": { - "type": "string", + "type": [ + "string", + "null" + ], "description": "Alias of `bio` introduced in BC5. BC3 emits both keys with identical content;\nolder BC4 responses may omit `tagline`. Prefer `bio` for cross-version reads.", "format": "password", "x-basecamp-sensitive": { "category": "pii", "redact": false - } + }, + "x-go-type": "string" }, "location": { - "type": "string", + "type": [ + "string", + "null" + ], "format": "password", "x-basecamp-sensitive": { "category": "pii", "redact": false - } + }, + "x-go-type": "string" }, "created_at": { "type": "string", @@ -33388,6 +34009,55 @@ "name" ] }, + "PollEventsResponseContent": { + "type": "object", + "description": "The poll envelope. Every 200 carries `events` and `position`; `next` only\nwhile the walk continues.", + "properties": { + "events": { + "type": "array", + "items": { + "$ref": "#/components/schemas/FeedEvent" + }, + "description": "Up to 100 events, oldest first, strict event-id order. A page may be empty\nwhile the walk crosses history the filters exclude \u2014 keep following `next`." + }, + "position": { + "type": "string", + "description": "Durable position token for resuming later. Persist only after the page's\nevents have been processed." + }, + "next": { + "type": "string", + "description": "Absolute continuation URL, present only while the current walk has more\nto serve; absent means the walk reached its (frozen) head \u2014 poll again\nlater from `position`." + } + }, + "required": [ + "events", + "position" + ] + }, + "PollInboxResponseContent": { + "type": "object", + "description": "The inbox envelope: `items` and `position` on every 200, `next` while the\nwalk continues.", + "properties": { + "items": { + "type": "array", + "items": { + "$ref": "#/components/schemas/InboxItem" + } + }, + "position": { + "type": "string", + "description": "Durable inbox position. Persist only after the page's items have been\nprocessed." + }, + "next": { + "type": "string", + "description": "Absolute continuation URL, present only while the walk has more to serve." + } + }, + "required": [ + "items", + "position" + ] + }, "Preferences": { "type": "object", "properties": { @@ -37726,6 +38396,9 @@ "creator": { "$ref": "#/components/schemas/Person" }, + "performed_by": { + "$ref": "#/components/schemas/Person" + }, "copy": { "$ref": "#/components/schemas/WebhookCopy" } diff --git a/internal/mcpserver/server_test.go b/internal/mcpserver/server_test.go index cfdeceee8..71bbf0673 100644 --- a/internal/mcpserver/server_test.go +++ b/internal/mcpserver/server_test.go @@ -41,7 +41,7 @@ func TestServerListsDomainTools(t *testing.T) { session := mcptest.Connect(t, srv.BuildMCPServer(slog.New(slog.DiscardHandler))) tools := mcptest.ListTools(t, session) - assert.Len(t, tools, 16) + assert.Len(t, tools, 17) require.Contains(t, tools, "basecamp_projects") projects := tools["basecamp_projects"] assert.Contains(t, projects.Description, "list_projects") diff --git a/internal/mcpserver/testdata/catalog_snapshot.txt b/internal/mcpserver/testdata/catalog_snapshot.txt index f513c1cc3..82eea3a47 100644 --- a/internal/mcpserver/testdata/catalog_snapshot.txt +++ b/internal/mcpserver/testdata/catalog_snapshot.txt @@ -313,6 +313,17 @@ ACTIONS (RO = read-only): - get_everything_unassigned_cards (RO, paginated): Open, unassigned cards across all accessible projects, grouped by project (paginated) - get_everything_unassigned_todos (RO, paginated): Open, unassigned to-dos across all accessible projects, grouped by project (paginated) +== basecamp_eventfeed +The account event feed: poll events from a resumable position, poll the agent inbox of addressed items, and mint a ticket for the live stream. Pages carry a position and a continuation URL rather than page numbers; deduplicate events by id and inbox items by addressing_id, and refetch the referenced recording before acting on it. + +Gateway tool: call with {"action": "...", "params": {...}}. +Call {"action": "describe", "params": {"action": "NAME"}} for an action's full parameter schema. + +ACTIONS (RO = read-only): +- create_stream_ticket: Mint a short-lived stream ticket and the exact WebSocket URL to open a live event stream with +- poll_events (RO): Poll the account event feed for events after a position (oldest first, strict event-id order, up to 100 per page) +- poll_inbox (RO): Poll the authenticated agent's inbox for the items that addressed it (oldest first, strict item-id order); people receive 403 + == basecamp_clientside The Clientside: client approvals, correspondences, replies, and client visibility of recordings. diff --git a/internal/version/sdk-provenance.json b/internal/version/sdk-provenance.json index a7cfdaa57..3ed0615f5 100644 --- a/internal/version/sdk-provenance.json +++ b/internal/version/sdk-provenance.json @@ -1,9 +1,9 @@ { "sdk": { "module": "github.com/basecamp/basecamp-sdk/go", - "version": "v0.18.1-0.20260916083520-5acb2f9aaca9", - "revision": "5acb2f9aaca9", - "updated_at": "2026-09-16T08:35:20Z" + "version": "v0.18.1-0.20260916113525-e58b5823a8ca", + "revision": "e58b5823a8ca", + "updated_at": "2026-09-16T11:35:25Z" }, "api": { "repo": "basecamp/bc3", From 4fb6a920d7f0c9dcc65ea9a9704d238c433ed783 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Wed, 16 Sep 2026 05:21:59 -0700 Subject: [PATCH 02/29] Add events poll, inbox, and stream-ticket commands on the account event feed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The account-wide event feed is how an agent hears about activity it did not cause. It is a different resource from `basecamp events `, which is one recording's audit trail, so it lands as subcommands under the same noun: `events poll` for the catch-up poll lane and `events ticket` to mint a live-stream credential, plus `inbox` at the top level for the addressed-items lane, which is its own endpoint rather than a filter over the feed. Pagination here is the response body, not the Link-header page walk every other listing in this CLI uses: a page carries a durable position to resume from and a continuation URL while the current walk has more to serve. One call is one page; --all walks the continuation to the end of that walk, bounded by --max-pages, and says so when it stops at the bound rather than at the end. That is not a live tail — the live lane is a WebSocket the ticket opens, and connecting it would mean taking a WebSocket dependency the CLI does not have today. The three documented failures get the taxonomy they deserve. A 409 is the caller's to fix, so it exits as usage and names both filter digests. A 410 is not, so it exits as not-found carrying the lane's own re-entry point: the feed re-enters at the epoch the body names, the inbox at the earliest retained item. A 400 keeps the server's message, because a malformed position and a malformed filter have different fixes and only the body knows which. The inbox's 403 says what it means — the lane is served to agent principals only for now. The ticket and its URL are redacted unless --show-secret: the URL embeds the ticket, both are bearers for the ~2 minute window, and a mint that printed them by default would put a credential in every terminal scrollback and log. Bare `basecamp events` now shows the group's help instead of refusing for a missing id, so the subcommands are discoverable. Passing an id still does what it did. --- .surface | 89 ++++- .surface-breaking | 1 + API-COVERAGE.md | 10 +- e2e/event_feed.bats | 62 ++++ e2e/events.bats | 14 +- e2e/smoke/smoke_event_feed.bats | 42 +++ internal/cli/root.go | 1 + internal/commands/commands.go | 3 +- internal/commands/commands_test.go | 1 + internal/commands/events.go | 27 +- internal/commands/feed.go | 525 ++++++++++++++++++++++++++++ internal/commands/feed_test.go | 483 +++++++++++++++++++++++++ internal/commands/inbox.go | 153 ++++++++ internal/commands/mcp_test.go | 2 +- internal/commands/recording_test.go | 34 +- scripts/check-bare-groups.sh | 1 + skills/basecamp/SKILL.md | 47 ++- 17 files changed, 1480 insertions(+), 15 deletions(-) create mode 100644 e2e/event_feed.bats create mode 100644 e2e/smoke/smoke_event_feed.bats create mode 100644 internal/commands/feed.go create mode 100644 internal/commands/feed_test.go create mode 100644 internal/commands/inbox.go diff --git a/.surface b/.surface index 55823b81c..c9d2b0b8e 100644 --- a/.surface +++ b/.surface @@ -176,7 +176,7 @@ ARG basecamp documents uploads create 00 ARG basecamp documents vault create 00 ARG basecamp documents vaults create 00 ARG basecamp documents versions 00 -ARG basecamp events 00 +ARG basecamp events 00 [id|url] ARG basecamp file archive 00 ARG basecamp file doc create 00 ARG basecamp file doc create 01 [content] @@ -746,6 +746,8 @@ CMD basecamp documents versions CMD basecamp drafts CMD basecamp drafts list CMD basecamp events +CMD basecamp events poll +CMD basecamp events ticket CMD basecamp file CMD basecamp file archive CMD basecamp file doc @@ -880,6 +882,7 @@ CMD basecamp hillcharts CMD basecamp hillcharts show CMD basecamp hillcharts track CMD basecamp hillcharts untrack +CMD basecamp inbox CMD basecamp lineup CMD basecamp lineup create CMD basecamp lineup delete @@ -7317,6 +7320,59 @@ FLAG basecamp events --stats type=bool FLAG basecamp events --styled type=bool FLAG basecamp events --todolist type=string FLAG basecamp events --verbose type=count +FLAG basecamp events poll --account type=string +FLAG basecamp events poll --actor-types type=stringSlice +FLAG basecamp events poll --agent type=bool +FLAG basecamp events poll --all type=bool +FLAG basecamp events poll --buckets type=stringSlice +FLAG basecamp events poll --cache-dir type=string +FLAG basecamp events poll --count type=bool +FLAG basecamp events poll --creators type=stringSlice +FLAG basecamp events poll --exclude-performers type=stringSlice +FLAG basecamp events poll --help type=bool +FLAG basecamp events poll --hints type=bool +FLAG basecamp events poll --ids-only type=bool +FLAG basecamp events poll --in type=string +FLAG basecamp events poll --jq type=string +FLAG basecamp events poll --json type=bool +FLAG basecamp events poll --markdown type=bool +FLAG basecamp events poll --max-pages type=int +FLAG basecamp events poll --md type=bool +FLAG basecamp events poll --no-hints type=bool +FLAG basecamp events poll --no-stats type=bool +FLAG basecamp events poll --performers type=stringSlice +FLAG basecamp events poll --position type=string +FLAG basecamp events poll --profile type=string +FLAG basecamp events poll --project type=string +FLAG basecamp events poll --quiet type=bool +FLAG basecamp events poll --since type=string +FLAG basecamp events poll --stats type=bool +FLAG basecamp events poll --styled type=bool +FLAG basecamp events poll --todolist type=string +FLAG basecamp events poll --types type=stringSlice +FLAG basecamp events poll --verbose type=count +FLAG basecamp events ticket --account type=string +FLAG basecamp events ticket --agent type=bool +FLAG basecamp events ticket --cache-dir type=string +FLAG basecamp events ticket --count type=bool +FLAG basecamp events ticket --help type=bool +FLAG basecamp events ticket --hints type=bool +FLAG basecamp events ticket --ids-only type=bool +FLAG basecamp events ticket --in type=string +FLAG basecamp events ticket --jq type=string +FLAG basecamp events ticket --json type=bool +FLAG basecamp events ticket --markdown type=bool +FLAG basecamp events ticket --md type=bool +FLAG basecamp events ticket --no-hints type=bool +FLAG basecamp events ticket --no-stats type=bool +FLAG basecamp events ticket --profile type=string +FLAG basecamp events ticket --project type=string +FLAG basecamp events ticket --quiet type=bool +FLAG basecamp events ticket --show-secret type=bool +FLAG basecamp events ticket --stats type=bool +FLAG basecamp events ticket --styled type=bool +FLAG basecamp events ticket --todolist type=string +FLAG basecamp events ticket --verbose type=count FLAG basecamp file --account type=string FLAG basecamp file --agent type=bool FLAG basecamp file --cache-dir type=string @@ -10669,6 +10725,34 @@ FLAG basecamp hillcharts untrack --styled type=bool FLAG basecamp hillcharts untrack --todolist type=string FLAG basecamp hillcharts untrack --todoset type=string FLAG basecamp hillcharts untrack --verbose type=count +FLAG basecamp inbox --account type=string +FLAG basecamp inbox --agent type=bool +FLAG basecamp inbox --all type=bool +FLAG basecamp inbox --buckets type=stringSlice +FLAG basecamp inbox --cache-dir type=string +FLAG basecamp inbox --count type=bool +FLAG basecamp inbox --help type=bool +FLAG basecamp inbox --hints type=bool +FLAG basecamp inbox --ids-only type=bool +FLAG basecamp inbox --in type=string +FLAG basecamp inbox --jq type=string +FLAG basecamp inbox --json type=bool +FLAG basecamp inbox --markdown type=bool +FLAG basecamp inbox --max-pages type=int +FLAG basecamp inbox --md type=bool +FLAG basecamp inbox --no-hints type=bool +FLAG basecamp inbox --no-stats type=bool +FLAG basecamp inbox --position type=string +FLAG basecamp inbox --profile type=string +FLAG basecamp inbox --project type=string +FLAG basecamp inbox --quiet type=bool +FLAG basecamp inbox --reasons type=stringSlice +FLAG basecamp inbox --since type=string +FLAG basecamp inbox --stats type=bool +FLAG basecamp inbox --styled type=bool +FLAG basecamp inbox --todolist type=string +FLAG basecamp inbox --types type=stringSlice +FLAG basecamp inbox --verbose type=count FLAG basecamp lineup --account type=string FLAG basecamp lineup --agent type=bool FLAG basecamp lineup --cache-dir type=string @@ -18462,6 +18546,8 @@ SUB basecamp documents versions SUB basecamp drafts SUB basecamp drafts list SUB basecamp events +SUB basecamp events poll +SUB basecamp events ticket SUB basecamp file SUB basecamp file archive SUB basecamp file doc @@ -18596,6 +18682,7 @@ SUB basecamp hillcharts SUB basecamp hillcharts show SUB basecamp hillcharts track SUB basecamp hillcharts untrack +SUB basecamp inbox SUB basecamp lineup SUB basecamp lineup create SUB basecamp lineup delete diff --git a/.surface-breaking b/.surface-breaking index 840004f60..ff2d93791 100644 --- a/.surface-breaking +++ b/.surface-breaking @@ -13,6 +13,7 @@ ARG basecamp comment 00 <id|url> ARG basecamp comment 01 <content> ARG basecamp comments list 00 <id|url> ARG basecamp done 00 <id|url>... +ARG basecamp events 00 <id|url> ARG basecamp message 00 <title> ARG basecamp message 01 [body] ARG basecamp react 00 <content> diff --git a/API-COVERAGE.md b/API-COVERAGE.md index 886568735..3d6397a0e 100644 --- a/API-COVERAGE.md +++ b/API-COVERAGE.md @@ -6,12 +6,12 @@ Coverage of Basecamp 3 API endpoints. Source: [bc3-api/sections](https://github. | Status | Sections | Endpoints | |--------|----------|-----------| -| ✅ Implemented | 50 | 192 | +| ✅ Implemented | 51 | 195 | | ⚠️ Blocked | 0 | 0 | | ⏭️ Out of scope | 4 | 12 | -| **Total tracked** | **54** | **204** | +| **Total tracked** | **55** | **207** | -**192 of 192 tracked in-scope endpoints.** The client-admission endpoints +**195 of 195 tracked in-scope endpoints.** The client-admission endpoints basecamp/bc3#13098 added — `PUT /projects/:id/people/client_users.json` and `POST`/`DELETE /projects/:id/client_enablement.json` — land as `people clients`. SDK v0.16.0 adds the three to-do @@ -50,6 +50,8 @@ Out-of-scope sections are excluded from parity totals and scripts: chatbots (dif **SDK version:** v0.16.0 (adds the to-do list template library and asynchronous copy operations; `internal/version/sdk-provenance.json` is authoritative). The +event-feed operations are unreleased, so the pin is a basecamp-sdk `main` +pseudo-version until the next SDK release tag. The command surface below largely dates to the v0.12.0 bump, which added 20 exported Go methods over 13 new backend operations; the extra seven wrapped endpoints that already existed but were reachable only through the raw generated client, @@ -229,6 +231,8 @@ cannot faithfully cover at least one endpoint for a reason outside the CLI. A | schedules | 2 | `schedule` | ✅ | BC4 | - | Schedule container + settings | | schedule_entries | 5 | `schedule` | ✅ | BC4 | - | list, show, create, update, occurrences. Create supports `--subscribe`/`--no-subscribe` | | events | 1 | `events` | ✅ | BC4 | - | Recording change audit trail | +| **Event Feed** | +| event_feed | 3 | `events poll`, `events ticket`, `inbox` | ✅ | BC5 | - | The account-wide event feed: `GET /events.json` (`events poll`), `GET /inbox.json` (`inbox`, agent principals only), `POST /events/stream_ticket.json` (`events ticket`). Pagination is the body envelope — a durable `position` and a `next` continuation URL — not the Link-header page walk, so `--all` walks `next` rather than page numbers. The live WebSocket lane the ticket opens is out of scope for the CLI: the SDK's cable client carries a WebSocket dependency the CLI does not take | | **Webhooks** | | webhooks | 7 | `webhooks` | ✅ | BC4 | - | list, show, create, update, delete | | **Templates** | diff --git a/e2e/event_feed.bats b/e2e/event_feed.bats new file mode 100644 index 000000000..d518f7106 --- /dev/null +++ b/e2e/event_feed.bats @@ -0,0 +1,62 @@ +#!/usr/bin/env bats +# event_feed.bats - Account event feed: entry-point and filter validation +# +# Every case here is refused before a request leaves the process, so the suite +# needs no network and no live feed. + +load test_helper + +setup_extra() { + create_credentials + create_global_config '{"account_id": 99999}' +} + +@test "events poll rejects --since and --position together" { + run basecamp events poll --since now --position abc + assert_failure + assert_output_contains "mutually exclusive" +} + +@test "events poll rejects a --since that is not an entry point" { + run basecamp events poll --since 2026-07-14 + assert_failure + assert_output_contains "Invalid --since" +} + +@test "events poll rejects a non-numeric bucket filter" { + run basecamp events poll --buckets one + assert_failure + assert_output_contains "Invalid --buckets id" +} + +@test "events poll rejects an actor type outside agent and person" { + run basecamp events poll --actor-types robot + assert_failure + assert_output_contains "Invalid --actor-types" +} + +@test "events poll --agent --help documents position handling" { + run basecamp events poll --agent --help + assert_success + assert_output_contains "position" + assert_output_contains "Deduplicate by event id" +} + +@test "events ticket --agent --help warns against logging the ticket" { + run basecamp events ticket --agent --help + assert_success + assert_output_contains "show-secret" +} + +@test "inbox rejects --since and --position together" { + run basecamp inbox --since now --position abc + assert_failure + assert_output_contains "mutually exclusive" +} + +@test "inbox --agent --help documents addressing_id dedupe" { + run basecamp inbox --agent --help + assert_success + assert_output_contains "addressing_id" + assert_output_contains "agent" +} diff --git a/e2e/events.bats b/e2e/events.bats index e80ee31e8..3f74a61ba 100644 --- a/e2e/events.bats +++ b/e2e/events.bats @@ -6,13 +6,23 @@ load test_helper # Missing context errors -@test "events without recording id shows error" { +@test "events without recording id shows the group's help" { create_credentials create_global_config '{"account_id": 99999}' run basecamp events + assert_success + assert_output_contains "poll" + assert_output_contains "ticket" +} + +@test "events with a non-numeric recording id shows error" { + create_credentials + create_global_config '{"account_id": 99999}' + + run basecamp events not-an-id assert_failure - assert_output_contains "ID required" + assert_output_contains "Invalid ID" } # Help diff --git a/e2e/smoke/smoke_event_feed.bats b/e2e/smoke/smoke_event_feed.bats new file mode 100644 index 000000000..be3d968f4 --- /dev/null +++ b/e2e/smoke/smoke_event_feed.bats @@ -0,0 +1,42 @@ +#!/usr/bin/env bats +# smoke_event_feed.bats - Level 0: Account event feed operations +# +# Nothing here prints a position, a ticket or a cable URL: the first is a +# signed resume token and the other two are bearers, and a smoke trace is a +# file somebody keeps. Entering at the present (--since now) also keeps these +# reads bounded — a since=0 replay would walk the account's served history. + +load smoke_helper + +setup_file() { + ensure_token || return 1 +} + +@test "events poll enters the feed at the present" { + run_smoke basecamp events poll --since now --json + assert_success + assert_json_value '.ok' 'true' +} + +@test "events poll rejects a since that is not an entry point" { + run_smoke basecamp events poll --since yesterday --json + assert_failure + assert_output_contains "Invalid --since" +} + +@test "events ticket mints a redacted ticket" { + run_smoke basecamp events ticket --json + assert_success + assert_json_value '.ok' 'true' + assert_json_value '.data.ticket' '[REDACTED]' +} + +@test "inbox polls addressed items" { + run_smoke basecamp inbox --since now --json + # The inbox is served to agent principals only; a person's token gets 403. + if [[ "$status" -ne 0 ]]; then + assert_output_contains "agent principals only" + mark_unverifiable "Inbox is agents-only and this token is not an agent's" + fi + assert_json_value '.ok' 'true' +} diff --git a/internal/cli/root.go b/internal/cli/root.go index d1b03d187..3cfd19869 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -333,6 +333,7 @@ func Execute() { cmd.AddCommand(commands.NewCheckinsCmd()) cmd.AddCommand(commands.NewWebhooksCmd()) cmd.AddCommand(commands.NewEventsCmd()) + cmd.AddCommand(commands.NewInboxCmd()) cmd.AddCommand(commands.NewSubscriptionsCmd()) cmd.AddCommand(commands.NewForwardsCmd()) cmd.AddCommand(commands.NewMessageboardsCmd()) diff --git a/internal/commands/commands.go b/internal/commands/commands.go index 904216535..e32b86d1b 100644 --- a/internal/commands/commands.go +++ b/internal/commands/commands.go @@ -105,6 +105,7 @@ func CommandCategories() []CommandCategory { {Name: "comments", Category: "communication", Description: "Manage comments", Actions: []string{"create", "list", "show", "thread", "update", "trash", "archive", "restore"}}, {Name: "boost", Category: "communication", Description: "Manage boosts (reactions)", Actions: []string{"list", "show", "create", "delete"}}, {Name: "notifications", Category: "communication", Description: "View and manage notifications", Actions: []string{"list", "read", "bubbleups"}}, + {Name: "inbox", Category: "communication", Description: "Poll addressed items from the account event feed (agents only)"}, }, }, { @@ -113,7 +114,7 @@ func CommandCategories() []CommandCategory { {Name: "search", Category: "search", Description: "Search across projects"}, {Name: "recordings", Category: "search", Description: "Browse content by type across projects", Actions: []string{"list", "trash", "archive", "restore", "visibility"}}, {Name: "show", Category: "search", Description: "Show any item by ID"}, - {Name: "events", Category: "search", Description: "View change history"}, + {Name: "events", Category: "search", Description: "View change history and poll the account event feed", Actions: []string{"poll", "ticket"}}, {Name: "url", Category: "search", Description: "Parse Basecamp URLs"}, }, }, diff --git a/internal/commands/commands_test.go b/internal/commands/commands_test.go index d295c3353..1bea37acf 100644 --- a/internal/commands/commands_test.go +++ b/internal/commands/commands_test.go @@ -81,6 +81,7 @@ func buildRootWithAllCommands() *cobra.Command { root.AddCommand(commands.NewCheckinsCmd()) root.AddCommand(commands.NewWebhooksCmd()) root.AddCommand(commands.NewEventsCmd()) + root.AddCommand(commands.NewInboxCmd()) root.AddCommand(commands.NewSubscriptionsCmd()) root.AddCommand(commands.NewForwardsCmd()) root.AddCommand(commands.NewMessageboardsCmd()) diff --git a/internal/commands/events.go b/internal/commands/events.go index e89b37efc..9771cfb31 100644 --- a/internal/commands/events.go +++ b/internal/commands/events.go @@ -19,7 +19,7 @@ func NewEventsCmd() *cobra.Command { var all bool cmd := &cobra.Command{ - Use: "events <id|url>", + Use: "events [id|url]", Short: "View change history", Long: `View the event history (audit trail) for any item. @@ -34,10 +34,24 @@ Events track all changes to an item. Common event actions: - assignment_changed - Assignees were added/removed - content_changed - Content was edited - archived/unarchived - Status changed -- commented_on - A comment was added`, - Annotations: map[string]string{"agent_notes": "Events show change history for a specific item"}, - Args: cobra.ExactArgs(1), +- commented_on - A comment was added + +The account-wide event feed is a different resource, reached through the +subcommands: 'events poll' for the feed's catch-up poll lane and +'events ticket' to mint a live-stream ticket. 'basecamp inbox' is the +addressed-items lane of the same feed.`, + Annotations: map[string]string{ + "agent_notes": "Events show change history for a specific item.\n" + + "For the account-wide event feed use 'basecamp events poll'; this command is one recording's history.", + }, + Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { + // Bare 'events' is the group's help: the recording history needs + // an id, and the account feed lives under 'poll'. + if len(args) == 0 { + return cmd.Help() + } + app := appctx.FromContext(cmd.Context()) // Validate flag combinations @@ -104,5 +118,10 @@ Events track all changes to an item. Common event actions: cmd.Flags().BoolVar(&all, "all", false, "Fetch all events (no limit)") cmd.Flags().IntVar(&page, "page", 0, "Fetch a single page (use --all for everything)") + cmd.AddCommand( + newEventsPollCmd(), + newEventsTicketCmd(), + ) + return cmd } diff --git a/internal/commands/feed.go b/internal/commands/feed.go new file mode 100644 index 000000000..89ab33aca --- /dev/null +++ b/internal/commands/feed.go @@ -0,0 +1,525 @@ +package commands + +import ( + "errors" + "fmt" + "net/http" + "net/url" + "strconv" + "strings" + + "github.com/spf13/cobra" + + "github.com/basecamp/basecamp-sdk/go/pkg/basecamp" + + "github.com/basecamp/basecamp-cli/internal/appctx" + "github.com/basecamp/basecamp-cli/internal/output" +) + +// The account event feed: the catch-up poll lane (`events poll`), the agent +// inbox (`inbox`), and stream-ticket minting for the live lane +// (`events ticket`). +// +// Pagination here is the response body, not the Link-header page walk every +// other listing in this CLI uses: a page carries a durable `position` to +// resume from later and a `next` continuation URL while the current walk has +// more to serve. One call is one page; --all walks `next` to the end of the +// current walk, which is not the same as following the feed forever. +// +// The feed is a notification lane, not an audit log. Events are thin +// pointers: deduplicate by event id (inbox items by addressing_id, since one +// event addresses a principal once per reason) and refetch the referenced +// recording before acting on it. + +// feedDefaultMaxPages bounds an --all walk. The server serves up to 100 +// events per page, so the default admits 5,000 events before the walk reports +// itself capped rather than complete. It is a runaway backstop for a +// since=0 replay, not a tuning knob for ordinary polling: an ordinary poll +// reaches the end of its walk in a handful of pages. +const feedDefaultMaxPages = 50 + +// feedLane is the per-lane text the shared machinery cannot derive: which +// command to re-run, and how that lane re-enters after its position stops +// being servable. The feed re-enters at the epoch the 410 body names; the +// inbox re-enters at since=0, the earliest item still retained. +type feedLane struct { + name string + pollCmd string + resumeSince string + forbiddenMsg string + forbiddenHint string +} + +var ( + eventsLane = feedLane{ + name: "event", + pollCmd: "basecamp events poll", + } + inboxLane = feedLane{ + name: "inbox item", + pollCmd: "basecamp inbox", + resumeSince: basecamp.SinceEpoch, + forbiddenMsg: "The inbox is served to agent principals only", + forbiddenHint: "Authenticate as an agent, or use 'basecamp events poll' for the account-wide feed", + } +) + +// feedEntry is the entry point a poll presents: an explicit since, a held +// position, or neither (which enters at the present). +type feedEntry struct { + since string + position string +} + +// validate rejects the two entry points the server would reject anyway, with +// a message that names the fix. The shape of --since is checked, never its +// value: an event id the feed has never served is the server's verdict to +// give, not ours. +func (e feedEntry) validate() error { + switch { + case e.since != "" && e.position != "": + return output.ErrUsage("--since and --position are mutually exclusive") + case e.since == "", e.since == basecamp.SinceNow, e.since == basecamp.SinceEpoch: + return nil + } + if _, err := strconv.ParseInt(e.since, 10, 64); err != nil { + return output.ErrUsageHint( + fmt.Sprintf("Invalid --since: %s", e.since), + "Pass an event id to start after, 'now' to enter at the present, or 0 to replay served history", + ) + } + return nil +} + +// feedIDs parses a comma-separated id filter. Only the shape is enforced: +// the server owns how many ids a filter may carry, and a cap written here +// would go stale as a rejection of a request the server would have served. +func feedIDs(values []string, flag string) ([]int64, error) { + if len(values) == 0 { + return nil, nil + } + ids := make([]int64, 0, len(values)) + for _, value := range values { + id, err := strconv.ParseInt(strings.TrimSpace(value), 10, 64) + if err != nil { + return nil, output.ErrUsage(fmt.Sprintf("Invalid %s id: %s", flag, value)) + } + ids = append(ids, id) + } + return ids, nil +} + +// feedPerformers parses a performer filter, which takes ids or the literal +// "self" — the request's own effective actor, resolved server-side. +// --exclude-performers self is the loop guard for an agent that acts on what +// it hears. +func feedPerformers(values []string, flag string) ([]string, error) { + if len(values) == 0 { + return nil, nil + } + performers := make([]string, 0, len(values)) + for _, value := range values { + value = strings.TrimSpace(value) + if value != "self" { + if _, err := strconv.ParseInt(value, 10, 64); err != nil { + return nil, output.ErrUsage(fmt.Sprintf("Invalid %s: %s (expected a person id or 'self')", flag, value)) + } + } + performers = append(performers, value) + } + return performers, nil +} + +// feedActorTypes parses the actor-kind filter. Unlike types and reasons, +// whose catalogs grow, this dimension is the closed pair the contract names. +func feedActorTypes(values []string) ([]string, error) { + if len(values) == 0 { + return nil, nil + } + actorTypes := make([]string, 0, len(values)) + for _, value := range values { + value = strings.TrimSpace(value) + if value != "agent" && value != "person" { + return nil, output.ErrUsage(fmt.Sprintf("Invalid --actor-types: %s (expected 'agent' or 'person')", value)) + } + actorTypes = append(actorTypes, value) + } + return actorTypes, nil +} + +// trimFilter drops empty entries a trailing comma leaves behind, so +// --types "message.created," is the one-type filter it reads as rather than a +// filter carrying an empty type the server rejects. +func trimFilter(values []string) []string { + if len(values) == 0 { + return nil + } + trimmed := make([]string, 0, len(values)) + for _, value := range values { + if value = strings.TrimSpace(value); value != "" { + trimmed = append(trimmed, value) + } + } + if len(trimmed) == 0 { + return nil + } + return trimmed +} + +// feedError maps the poll lanes' documented failures onto the CLI's error +// taxonomy. Every branch is an errors.As on a typed SDK error, so the split +// the SDK is adding — a per-lane 410 and a reason-carrying 400 — lands as two +// more arms here and nowhere else. +// +// The mapping: +// +// 409 → usage. The held position was minted for different filters; the fix +// is the caller's, and both digests name which side is which. +// 410 → not found. The position is no longer servable. Not the caller's +// mistake, so the message carries the lane's own re-entry point. +// 403 → forbidden, with the lane's documented reason (the inbox is +// agents-only for now). +// 400 → whatever the server said. A malformed position and a malformed +// filter have different fixes and the body says which; guessing +// between them would send the caller to the wrong one. +func feedError(lane feedLane, err error) error { + var mismatch *basecamp.FeedFilterMismatchError + if errors.As(err, &mismatch) { + return &output.Error{ + Code: output.CodeUsage, + Message: fmt.Sprintf( + "Position was minted for a different filter set (position digest %s, this request's filters digest %s)", + mismatch.PositionDigest, mismatch.FiltersDigest), + Hint: fmt.Sprintf("Re-enter with --since to acknowledge the filter change: %s --since now", lane.pollCmd), + HTTPStatus: http.StatusConflict, + Cause: err, + } + } + + var gone *basecamp.FeedPositionGoneError + if errors.As(err, &gone) { + since := lane.resumeSince + if gone.EpochAfterID != nil { + since = strconv.FormatInt(*gone.EpochAfterID, 10) + } + hint := fmt.Sprintf("Re-enter with: %s --since %s", lane.pollCmd, since) + if since == "" { + hint = fmt.Sprintf("Re-enter with: %s --since now", lane.pollCmd) + } + message := fmt.Sprintf("Position is no longer servable for the %s feed", lane.name) + if gone.Resume != "" { + message = fmt.Sprintf("%s. Resume URL: %s", message, gone.Resume) + } + return &output.Error{ + Code: output.CodeNotFound, + Message: message, + Hint: hint, + HTTPStatus: http.StatusGone, + Cause: err, + } + } + + var sdkErr *basecamp.Error + if errors.As(err, &sdkErr) && sdkErr.HTTPStatus == http.StatusForbidden && lane.forbiddenMsg != "" { + return &output.Error{ + Code: output.CodeForbidden, + Message: lane.forbiddenMsg, + Hint: lane.forbiddenHint, + HTTPStatus: http.StatusForbidden, + Cause: err, + } + } + + return convertSDKError(err) +} + +// checkContinuation is the caller-side obligation the SDK's +// Poll*OptionsFromURL documents: a continuation URL arrives in a server +// response, so its origin is checked against the configured API base before +// the walk follows it. Only the query is ever re-issued — the SDK builds the +// next request against the configured base URL, so a foreign origin cannot +// receive the bearer — but a server that could redirect a walk's filters +// still has no business doing so. +// +// An unconfigured base URL (the SDK's own default) is the default host. +func checkContinuation(baseURL, raw string) error { + base, err := url.Parse(baseURL) + if err != nil || base.Host == "" { + base = &url.URL{Scheme: "https", Host: "3.basecampapi.com"} + } + next, err := url.Parse(raw) + if err != nil { + return &output.Error{ + Code: output.CodeAPI, + Message: "Continuation URL is not a URL", + } + } + if next.Scheme != base.Scheme || next.Host != base.Host { + return &output.Error{ + Code: output.CodeAPI, + Message: "Continuation URL points outside the configured Basecamp API host", + Hint: "Re-enter with --since rather than following this walk", + } + } + return nil +} + +// feedWalkNotice reports a walk that stopped at its page cap rather than at +// the end of the walk, so a caller reads "more remains" instead of "done". +func feedWalkNotice(capped bool, maxPages int) string { + if !capped { + return "" + } + return fmt.Sprintf("Stopped at --max-pages %d; more remains. Poll again from the position above.", maxPages) +} + +// newEventsPollCmd builds `basecamp events poll`. +func newEventsPollCmd() *cobra.Command { + var ( + entry feedEntry + types []string + buckets []string + creators []string + performers []string + excludePerformers []string + actorTypes []string + all bool + maxPages int + ) + + cmd := &cobra.Command{ + Use: "poll", + Short: "Poll the account event feed", + Long: `Poll the account-wide event feed. + +One call is one page. Each page carries a durable 'position' to resume from +and, while the current walk has more to serve, a 'next' continuation URL. +With no --since or --position the feed begins at the present. + + basecamp events poll --since now + basecamp events poll --position "$POSITION" + basecamp events poll --since 0 --types message.created,comment.created --all + basecamp events poll --exclude-performers self + +The feed is a notification lane, not an audit log: events are thin pointers, +delivery is best effort, and the same event can arrive twice. Deduplicate by +event id and refetch the referenced recording before acting on it.`, + Annotations: map[string]string{ + "agent_notes": "Account-wide — no --in <project> needed.\n" + + "Returns {events, position, next}. Persist 'position' only after processing the page's events, then resume with --position.\n" + + "--since takes an event id (int64), 'now' to enter at the present, or 0 to replay served history back to the feed's epoch.\n" + + "Deduplicate by event id: polls repeat events the live stream already delivered, and events can appear up to ~30s after they happen.\n" + + "Events are thin pointers — refetch the referenced recording with 'basecamp show <recording_id>' before acting on it.\n" + + "An agent that acts on what it hears should pass --exclude-performers self to avoid reacting to its own activity.\n" + + "Changing filters invalidates a held position (exit 1, HTTP 409): re-enter with --since.\n" + + "A position that predates the feed's epoch exits 2 (HTTP 410) and the hint names the --since to re-enter with.\n" + + "--all walks 'next' to the end of the current walk; it is not a live tail.", + }, + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, args []string) error { + app := appctx.FromContext(cmd.Context()) + + if err := entry.validate(); err != nil { + return err + } + if maxPages < 1 { + return output.ErrUsage("--max-pages must be at least 1") + } + + bucketIDs, err := feedIDs(trimFilter(buckets), "--buckets") + if err != nil { + return err + } + creatorIDs, err := feedIDs(trimFilter(creators), "--creators") + if err != nil { + return err + } + performerIDs, err := feedPerformers(trimFilter(performers), "--performers") + if err != nil { + return err + } + excludeIDs, err := feedPerformers(trimFilter(excludePerformers), "--exclude-performers") + if err != nil { + return err + } + kinds, err := feedActorTypes(trimFilter(actorTypes)) + if err != nil { + return err + } + + if err := ensureAccount(cmd, app); err != nil { + return err + } + + opts := &basecamp.PollEventsOptions{ + Since: entry.since, + Position: entry.position, + Types: trimFilter(types), + Buckets: bucketIDs, + Creators: creatorIDs, + Performers: performerIDs, + ExcludePerformers: excludeIDs, + ActorTypes: kinds, + } + + events := make([]basecamp.FeedEvent, 0) + var position, next string + pages, capped := 0, false + service := app.Account().EventFeed() + + for { + page, err := service.PollEvents(cmd.Context(), opts) + if err != nil { + return feedError(eventsLane, err) + } + pages++ + events = append(events, page.Events...) + position, next = page.Position, page.Next + + if !all || next == "" { + break + } + if pages >= maxPages { + capped = true + break + } + if err := checkContinuation(app.Config.BaseURL, next); err != nil { + return err + } + if opts, err = basecamp.PollEventsOptionsFromURL(next); err != nil { + return convertSDKError(err) + } + } + + summary := fmt.Sprintf("%d event(s)", len(events)) + if pages > 1 { + summary = fmt.Sprintf("%s over %d pages", summary, pages) + } + if next != "" && !capped { + summary += "; more to serve" + } + + respOpts := []output.ResponseOption{ + output.WithSummary(summary), + output.WithBreadcrumbs( + output.Breadcrumb{ + Action: "resume", + Cmd: "basecamp events poll --position <position>", + Description: "Poll again from this page's position", + }, + output.Breadcrumb{ + Action: "show", + Cmd: "basecamp show <recording_id>", + Description: "Fetch a referenced recording", + }, + ), + } + if notice := feedWalkNotice(capped, maxPages); notice != "" { + respOpts = append(respOpts, output.WithNotice(notice)) + } + + return app.OK(map[string]any{ + "events": events, + "position": position, + "next": next, + }, respOpts...) + }, + } + + cmd.Flags().StringVar(&entry.since, "since", "", "Enter the feed: an event id to start after, 'now', or 0 to replay served history") + cmd.Flags().StringVar(&entry.position, "position", "", "Resume from a position token a previous page issued") + cmd.Flags().StringSliceVar(&types, "types", nil, "Filter by event type (comma-separated, e.g. message.created)") + cmd.Flags().StringSliceVar(&buckets, "buckets", nil, "Filter by bucket (project) id (comma-separated)") + cmd.Flags().StringSliceVar(&creators, "creators", nil, "Filter by creator person id (comma-separated)") + cmd.Flags().StringSliceVar(&performers, "performers", nil, "Filter by effective performer: person ids or 'self' (comma-separated)") + cmd.Flags().StringSliceVar(&excludePerformers, "exclude-performers", nil, "Exclude effective performers: person ids or 'self' (comma-separated)") + cmd.Flags().StringSliceVar(&actorTypes, "actor-types", nil, "Filter by actor kind: agent, person (comma-separated)") + cmd.Flags().BoolVar(&all, "all", false, "Walk 'next' to the end of the current walk (not a live tail)") + cmd.Flags().IntVar(&maxPages, "max-pages", feedDefaultMaxPages, "Maximum pages to fetch with --all") + + return cmd +} + +// newEventsTicketCmd builds `basecamp events ticket`. +func newEventsTicketCmd() *cobra.Command { + var showSecret bool + + cmd := &cobra.Command{ + Use: "ticket", + Short: "Mint a live event stream ticket", + Long: `Mint a short-lived ticket for the live event stream over WebSocket. + +The response carries the exact URL to connect to — connect to it verbatim +rather than assembling cable topology yourself. The ticket is a replayable +bearer for its ~2 minute window and the URL embeds it, so both are redacted +unless --show-secret is passed. Mint a fresh ticket for every connection +attempt; an open socket does not refresh one. + + basecamp events ticket + basecamp events ticket --show-secret`, + Annotations: map[string]string{ + "agent_notes": "Account-wide — no --in <project> needed.\n" + + "Returns {ticket, expires_in, url}. Both ticket and url are redacted unless --show-secret is passed; the url embeds the ticket.\n" + + "Never log either value. Mint one ticket per connection attempt — tickets expire in about 2 minutes and are not refreshed by an open socket.\n" + + "Live frames never advance a durable position; only polls do. Keep polling from your position to repair what push missed.", + }, + 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 + } + + ticket, err := app.Account().EventFeed().CreateStreamTicket(cmd.Context()) + if err != nil { + return convertSDKError(err) + } + + data := map[string]any{ + "ticket": ticket.Ticket, + "expires_in": ticket.ExpiresIn, + "url": ticket.URL, + } + if !showSecret { + data["ticket"] = redactedTicket + data["url"] = redactTicketURL(ticket.URL) + } + + respOpts := []output.ResponseOption{ + output.WithSummary(fmt.Sprintf("Stream ticket expires in %ds", ticket.ExpiresIn)), + } + if !showSecret { + respOpts = append(respOpts, output.WithNotice("Ticket and URL redacted. Pass --show-secret to reveal them; never log either.")) + } + + return app.OK(data, respOpts...) + }, + } + + cmd.Flags().BoolVar(&showSecret, "show-secret", false, "Print the ticket and its URL verbatim instead of redacting them") + + return cmd +} + +// redactedTicket stands in for a ticket the caller did not ask to see. It is +// deliberately not ticket-shaped: a placeholder that still looked like a +// credential invites a consumer to present it. +const redactedTicket = "[REDACTED]" + +// redactTicketURL renders a cable URL without its credential. A URL that +// cannot be parsed is withheld whole rather than echoed: the ticket is +// opaque, so any component of an unparseable URL can be carrying it. +func redactTicketURL(raw string) string { + parsed, err := url.Parse(raw) + if err != nil { + return redactedTicket + } + query := parsed.Query() + if !query.Has("ticket") { + return redactedTicket + } + query.Set("ticket", redactedTicket) + parsed.RawQuery = query.Encode() + return parsed.String() +} diff --git a/internal/commands/feed_test.go b/internal/commands/feed_test.go new file mode 100644 index 000000000..25683cd7d --- /dev/null +++ b/internal/commands/feed_test.go @@ -0,0 +1,483 @@ +package commands + +import ( + "bytes" + "encoding/json" + "fmt" + "net/http" + "net/url" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/basecamp/basecamp-cli/internal/appctx" + "github.com/basecamp/basecamp-cli/internal/output" +) + +// The positions, tickets and cable URLs in these fixtures are invented +// strings. Real ones are credentials — a position is a signed resume token, a +// ticket is a replayable bearer — and neither belongs in a repository. + +const ( + feedEventsPath = "/99999/events.json" + feedInboxPath = "/99999/inbox.json" + feedTicketPath = "/99999/events/stream_ticket.json" + feedBaseURL = "https://3.basecampapi.com" +) + +func feedEventJSON(id int64) string { + return fmt.Sprintf(`{ + "id": %d, + "kind": "message_created", + "action": "created", + "event_type": "message.created", + "bucket_id": 2085958499, + "creator_id": 1049715945, + "performed_by_id": null, + "recording_id": 1069479766, + "created_at": "2026-07-14T06:10:00.159Z" + }`, id) +} + +func feedPageJSON(position, next string, ids ...int64) string { + events := make([]string, 0, len(ids)) + for _, id := range ids { + events = append(events, feedEventJSON(id)) + } + body := fmt.Sprintf(`{"events":[%s],"position":%q`, joinJSON(events), position) + if next != "" { + body += fmt.Sprintf(`,"next":%q`, next) + } + return body + "}" +} + +func inboxPageJSON(position, next string, addressingIDs ...int64) string { + items := make([]string, 0, len(addressingIDs)) + for _, id := range addressingIDs { + items = append(items, fmt.Sprintf( + `{"addressing_id":%d,"reason":"mentioned","addressed_at":"2026-07-14T06:10:00.159Z","event":%s}`, + id, feedEventJSON(1071915468))) + } + body := fmt.Sprintf(`{"items":[%s],"position":%q`, joinJSON(items), position) + if next != "" { + body += fmt.Sprintf(`,"next":%q`, next) + } + return body + "}" +} + +func joinJSON(parts []string) string { + out := "" + for i, part := range parts { + if i > 0 { + out += "," + } + out += part + } + return out +} + +// setupFeedApp is the recording harness with a JSON envelope writer and the +// configured API base the continuation check reads. +func setupFeedApp(t *testing.T, routes ...stubRoute) (*appctx.App, *recordingTransport, *bytes.Buffer) { + t.Helper() + app, transport := setupRecordingTestApp(t, routes...) + app.Config.BaseURL = feedBaseURL + out := &bytes.Buffer{} + app.Output = output.New(output.Options{Format: output.FormatJSON, Writer: out}) + return app, transport, out +} + +// feedEnvelope is the slice of the success envelope these tests read. +type feedEnvelope struct { + OK bool `json:"ok"` + Data json.RawMessage `json:"data"` + Summary string `json:"summary"` + Notice string `json:"notice"` +} + +func decodeFeedEnvelope(t *testing.T, out *bytes.Buffer) feedEnvelope { + t.Helper() + var envelope feedEnvelope + require.NoError(t, json.Unmarshal(out.Bytes(), &envelope)) + return envelope +} + +type feedPollData struct { + Events []struct { + ID int64 `json:"id"` + } `json:"events"` + Position string `json:"position"` + Next string `json:"next"` +} + +type inboxPollData struct { + Items []struct { + AddressingID int64 `json:"addressing_id"` + } `json:"items"` + Position string `json:"position"` + Next string `json:"next"` +} + +func requireFeedError(t *testing.T, err error) *output.Error { + t.Helper() + require.Error(t, err) + cliErr := output.AsError(err) + require.NotNil(t, cliErr) + return cliErr +} + +func eventsRoute(status int, bodies ...string) stubRoute { + return stubRoute{method: http.MethodGet, path: feedEventsPath, status: status, bodies: bodies} +} + +func inboxRoute(status int, bodies ...string) stubRoute { + return stubRoute{method: http.MethodGet, path: feedInboxPath, status: status, bodies: bodies} +} + +func TestEventsPollServesOnePageAndItsPosition(t *testing.T) { + app, transport, out := setupFeedApp(t, eventsRoute(http.StatusOK, feedPageJSON("pos-1", "", 11, 12))) + + err := executeRecordingCommand(NewEventsCmd(), app, "poll", "--since", "now") + require.NoError(t, err) + + envelope := decodeFeedEnvelope(t, out) + var data feedPollData + require.NoError(t, json.Unmarshal(envelope.Data, &data)) + assert.Equal(t, []int64{11, 12}, []int64{data.Events[0].ID, data.Events[1].ID}) + assert.Equal(t, "pos-1", data.Position) + assert.Empty(t, data.Next) + assert.Equal(t, "2 event(s)", envelope.Summary) + + assert.Equal(t, []string{"since=now"}, transport.queriesFor(feedEventsPath)) +} + +func TestEventsPollSendsEveryFilterCommaJoined(t *testing.T) { + app, transport, _ := setupFeedApp(t, eventsRoute(http.StatusOK, feedPageJSON("pos-1", ""))) + + err := executeRecordingCommand(NewEventsCmd(), app, "poll", + "--types", "message.created,comment.created", + "--buckets", "1,2", + "--creators", "9", + "--performers", "self,7", + "--exclude-performers", "self", + "--actor-types", "agent,person", + ) + require.NoError(t, err) + + query, err := url.ParseQuery(transport.last(t).Query) + require.NoError(t, err) + assert.Equal(t, "message.created,comment.created", query.Get("types")) + assert.Equal(t, "1,2", query.Get("buckets")) + assert.Equal(t, "9", query.Get("creators")) + assert.Equal(t, "self,7", query.Get("performers")) + assert.Equal(t, "self", query.Get("exclude_performers")) + assert.Equal(t, "agent,person", query.Get("actor_types")) +} + +// An empty page renders as [] rather than null, and next is always present — +// empty means the walk is done — so a consumer iterating .data.events or +// testing .data.next never has to handle a missing key. +func TestEventsPollEmptyPageKeepsTheEnvelopesShape(t *testing.T) { + app, _, out := setupFeedApp(t, eventsRoute(http.StatusOK, feedPageJSON("pos-1", ""))) + + require.NoError(t, executeRecordingCommand(NewEventsCmd(), app, "poll", "--since", "now")) + + var envelope struct { + Data struct { + Events *[]json.RawMessage `json:"events"` + Next *string `json:"next"` + Position string `json:"position"` + } `json:"data"` + } + require.NoError(t, json.Unmarshal(out.Bytes(), &envelope)) + require.NotNil(t, envelope.Data.Events) + assert.Empty(t, *envelope.Data.Events) + require.NotNil(t, envelope.Data.Next) + assert.Empty(t, *envelope.Data.Next) + assert.Equal(t, "pos-1", envelope.Data.Position) +} + +func TestEventsPollWithoutAllStopsAtOnePageAndReportsNext(t *testing.T) { + next := feedBaseURL + feedEventsPath + "?position=pos-1" + app, transport, out := setupFeedApp(t, eventsRoute(http.StatusOK, + feedPageJSON("pos-1", next, 11), + feedPageJSON("pos-2", "", 12), + )) + + require.NoError(t, executeRecordingCommand(NewEventsCmd(), app, "poll", "--since", "0")) + + envelope := decodeFeedEnvelope(t, out) + var data feedPollData + require.NoError(t, json.Unmarshal(envelope.Data, &data)) + assert.Len(t, data.Events, 1) + assert.Equal(t, next, data.Next) + assert.Contains(t, envelope.Summary, "more to serve") + assert.Len(t, transport.queriesFor(feedEventsPath), 1) +} + +func TestEventsPollAllFollowsNextUntilItIsAbsent(t *testing.T) { + next := feedBaseURL + feedEventsPath + "?position=pos-1&types=message.created" + app, transport, out := setupFeedApp(t, eventsRoute(http.StatusOK, + feedPageJSON("pos-1", next, 11), + feedPageJSON("pos-2", "", 12, 13), + )) + + require.NoError(t, executeRecordingCommand(NewEventsCmd(), app, "poll", "--since", "0", "--all")) + + envelope := decodeFeedEnvelope(t, out) + var data feedPollData + require.NoError(t, json.Unmarshal(envelope.Data, &data)) + assert.Len(t, data.Events, 3) + assert.Equal(t, "pos-2", data.Position) + assert.Empty(t, data.Next) + assert.Empty(t, envelope.Notice) + + queries := transport.queriesFor(feedEventsPath) + require.Len(t, queries, 2) + assert.Equal(t, "since=0", queries[0]) + // The second request re-issues the continuation's own query, filters + // included, rather than the flags the first call was given. + second, err := url.ParseQuery(queries[1]) + require.NoError(t, err) + assert.Equal(t, "pos-1", second.Get("position")) + assert.Equal(t, "message.created", second.Get("types")) + assert.Empty(t, second.Get("since")) +} + +func TestEventsPollAllStopsAtMaxPagesAndSaysSo(t *testing.T) { + next := feedBaseURL + feedEventsPath + "?position=pos-1" + app, transport, out := setupFeedApp(t, eventsRoute(http.StatusOK, + feedPageJSON("pos-1", next, 11), + feedPageJSON("pos-2", next, 12), + )) + + require.NoError(t, executeRecordingCommand(NewEventsCmd(), app, "poll", "--since", "0", "--all", "--max-pages", "1")) + + envelope := decodeFeedEnvelope(t, out) + assert.Contains(t, envelope.Notice, "--max-pages 1") + assert.NotContains(t, envelope.Summary, "more to serve") + assert.Len(t, transport.queriesFor(feedEventsPath), 1) +} + +func TestEventsPollRefusesACrossOriginContinuation(t *testing.T) { + app, transport, _ := setupFeedApp(t, eventsRoute(http.StatusOK, + feedPageJSON("pos-1", "https://evil.example/99999/events.json?position=pos-1", 11), + )) + + err := executeRecordingCommand(NewEventsCmd(), app, "poll", "--since", "0", "--all") + + cliErr := requireFeedError(t, err) + assert.Equal(t, output.CodeAPI, cliErr.Code) + assert.Contains(t, cliErr.Message, "outside the configured Basecamp API host") + assert.Len(t, transport.queriesFor(feedEventsPath), 1) +} + +func TestEventsPollFilterMismatchNamesBothDigests(t *testing.T) { + app, _, _ := setupFeedApp(t, eventsRoute(http.StatusConflict, + `{"error":"position was minted for a different filter set","position_digest":"44136fa355b3678a","filters_digest":"38b223c13c89dc89"}`)) + + err := executeRecordingCommand(NewEventsCmd(), app, "poll", "--position", "stale") + + cliErr := requireFeedError(t, err) + assert.Equal(t, output.CodeUsage, cliErr.Code) + assert.Equal(t, output.ExitUsage, cliErr.ExitCode()) + assert.Equal(t, http.StatusConflict, cliErr.HTTPStatus) + assert.Contains(t, cliErr.Message, "44136fa355b3678a") + assert.Contains(t, cliErr.Message, "38b223c13c89dc89") + assert.Contains(t, cliErr.Hint, "--since") +} + +func TestEventsPollPositionGoneCarriesTheResumeURLAndTheEpoch(t *testing.T) { + resume := feedBaseURL + feedEventsPath + "?since=900" + app, _, _ := setupFeedApp(t, eventsRoute(http.StatusGone, + fmt.Sprintf(`{"error":"position predates the feed epoch","epoch_after_id":900,"resume":%q}`, resume))) + + err := executeRecordingCommand(NewEventsCmd(), app, "poll", "--position", "ancient") + + cliErr := requireFeedError(t, err) + assert.Equal(t, output.CodeNotFound, cliErr.Code) + assert.Equal(t, output.ExitNotFound, cliErr.ExitCode()) + assert.Equal(t, http.StatusGone, cliErr.HTTPStatus) + assert.Contains(t, cliErr.Message, resume) + assert.Equal(t, "Re-enter with: basecamp events poll --since 900", cliErr.Hint) +} + +func TestEventsPollBadRequestKeepsTheServersMessage(t *testing.T) { + app, _, _ := setupFeedApp(t, eventsRoute(http.StatusBadRequest, + `{"error":"unknown type: message.exploded"}`)) + + err := executeRecordingCommand(NewEventsCmd(), app, "poll", "--types", "message.exploded") + + cliErr := requireFeedError(t, err) + assert.Contains(t, cliErr.Error(), "message.exploded") + assert.Equal(t, http.StatusBadRequest, cliErr.HTTPStatus) +} + +func TestEventsPollRejectsUnservableFlagsBeforeAnyRequest(t *testing.T) { + cases := []struct { + name string + args []string + want string + }{ + {"since and position together", []string{"--since", "now", "--position", "p"}, "mutually exclusive"}, + {"since that is not an id", []string{"--since", "2026-07-14"}, "Invalid --since"}, + {"bucket that is not an id", []string{"--buckets", "one"}, "Invalid --buckets id"}, + {"creator that is not an id", []string{"--creators", "1,two"}, "Invalid --creators id"}, + {"performer that is neither an id nor self", []string{"--performers", "everyone"}, "Invalid --performers"}, + {"actor type outside the pair", []string{"--actor-types", "robot"}, "Invalid --actor-types"}, + {"max pages below one", []string{"--all", "--max-pages", "0"}, "--max-pages must be at least 1"}, + } + + for _, testCase := range cases { + t.Run(testCase.name, func(t *testing.T) { + app, transport, _ := setupFeedApp(t, eventsRoute(http.StatusOK, feedPageJSON("pos-1", ""))) + + err := executeRecordingCommand(NewEventsCmd(), app, append([]string{"poll"}, testCase.args...)...) + + cliErr := requireFeedError(t, err) + assert.Equal(t, output.CodeUsage, cliErr.Code) + assert.Contains(t, cliErr.Message, testCase.want) + assert.Empty(t, transport.recorded()) + }) + } +} + +func TestEventsPollTrailingCommaIsNotAnEmptyFilter(t *testing.T) { + app, transport, _ := setupFeedApp(t, eventsRoute(http.StatusOK, feedPageJSON("pos-1", ""))) + + require.NoError(t, executeRecordingCommand(NewEventsCmd(), app, "poll", "--types", "message.created,")) + + query, err := url.ParseQuery(transport.last(t).Query) + require.NoError(t, err) + assert.Equal(t, "message.created", query.Get("types")) +} + +func TestBareEventsShowsHelpRatherThanPolling(t *testing.T) { + app, transport, _ := setupFeedApp(t) + + require.NoError(t, executeRecordingCommand(NewEventsCmd(), app)) + + assert.Empty(t, transport.recorded()) +} + +func TestInboxServesItemsAndItsPosition(t *testing.T) { + app, transport, out := setupFeedApp(t, inboxRoute(http.StatusOK, inboxPageJSON("inbox-pos-1", "", 991, 992))) + + require.NoError(t, executeRecordingCommand(NewInboxCmd(), app, "--since", "0", "--reasons", "mentioned,assigned")) + + envelope := decodeFeedEnvelope(t, out) + var data inboxPollData + require.NoError(t, json.Unmarshal(envelope.Data, &data)) + assert.Equal(t, int64(991), data.Items[0].AddressingID) + assert.Equal(t, "inbox-pos-1", data.Position) + assert.Equal(t, "2 addressed item(s)", envelope.Summary) + + query, err := url.ParseQuery(transport.last(t).Query) + require.NoError(t, err) + assert.Equal(t, "0", query.Get("since")) + assert.Equal(t, "mentioned,assigned", query.Get("reasons")) +} + +func TestInboxAllFollowsNextUntilItIsAbsent(t *testing.T) { + next := feedBaseURL + feedInboxPath + "?position=inbox-pos-1&reasons=mentioned" + app, transport, out := setupFeedApp(t, inboxRoute(http.StatusOK, + inboxPageJSON("inbox-pos-1", next, 991), + inboxPageJSON("inbox-pos-2", "", 992), + )) + + require.NoError(t, executeRecordingCommand(NewInboxCmd(), app, "--since", "0", "--all")) + + envelope := decodeFeedEnvelope(t, out) + var data inboxPollData + require.NoError(t, json.Unmarshal(envelope.Data, &data)) + assert.Len(t, data.Items, 2) + assert.Equal(t, "inbox-pos-2", data.Position) + + queries := transport.queriesFor(feedInboxPath) + require.Len(t, queries, 2) + second, err := url.ParseQuery(queries[1]) + require.NoError(t, err) + assert.Equal(t, "inbox-pos-1", second.Get("position")) + assert.Equal(t, "mentioned", second.Get("reasons")) +} + +func TestInboxForbiddenSaysItIsAgentsOnly(t *testing.T) { + app, _, _ := setupFeedApp(t, inboxRoute(http.StatusForbidden, ``)) + + err := executeRecordingCommand(NewInboxCmd(), app, "--since", "now") + + cliErr := requireFeedError(t, err) + assert.Equal(t, output.CodeForbidden, cliErr.Code) + assert.Equal(t, output.ExitForbidden, cliErr.ExitCode()) + assert.Equal(t, "The inbox is served to agent principals only", cliErr.Message) + assert.Contains(t, cliErr.Hint, "basecamp events poll") +} + +func TestInboxPositionGoneReEntersAtTheEarliestRetainedItem(t *testing.T) { + resume := feedBaseURL + feedInboxPath + "?since=0" + app, _, _ := setupFeedApp(t, inboxRoute(http.StatusGone, + fmt.Sprintf(`{"error":"position fell behind the retention window","resume":%q}`, resume))) + + err := executeRecordingCommand(NewInboxCmd(), app, "--position", "ancient") + + cliErr := requireFeedError(t, err) + assert.Equal(t, output.CodeNotFound, cliErr.Code) + assert.Contains(t, cliErr.Message, resume) + assert.Equal(t, "Re-enter with: basecamp inbox --since 0", cliErr.Hint) +} + +func TestInboxRejectsUnservableFlagsBeforeAnyRequest(t *testing.T) { + app, transport, _ := setupFeedApp(t, inboxRoute(http.StatusOK, inboxPageJSON("inbox-pos-1", ""))) + + err := executeRecordingCommand(NewInboxCmd(), app, "--since", "now", "--position", "p") + + cliErr := requireFeedError(t, err) + assert.Equal(t, output.CodeUsage, cliErr.Code) + assert.Empty(t, transport.recorded()) +} + +func TestEventsTicketRedactsTheTicketAndItsURL(t *testing.T) { + app, _, out := setupFeedApp(t, stubRoute{ + method: http.MethodPost, + path: feedTicketPath, + status: http.StatusOK, + body: `{"ticket":"tkt-secret","expires_in":120,"url":"wss://chat.example.test/195539477?ticket=tkt-secret"}`, + }) + + require.NoError(t, executeRecordingCommand(NewEventsCmd(), app, "ticket")) + + rendered := out.String() + assert.NotContains(t, rendered, "tkt-secret") + assert.Contains(t, rendered, "195539477") + assert.Contains(t, decodeFeedEnvelope(t, out).Summary, "120s") +} + +func TestEventsTicketShowSecretPrintsItVerbatim(t *testing.T) { + app, _, out := setupFeedApp(t, stubRoute{ + method: http.MethodPost, + path: feedTicketPath, + status: http.StatusOK, + body: `{"ticket":"tkt-secret","expires_in":120,"url":"wss://chat.example.test/195539477?ticket=tkt-secret"}`, + }) + + require.NoError(t, executeRecordingCommand(NewEventsCmd(), app, "ticket", "--show-secret")) + + assert.Contains(t, out.String(), "wss://chat.example.test/195539477?ticket=tkt-secret") + assert.Empty(t, decodeFeedEnvelope(t, out).Notice) +} + +func TestRedactTicketURLWithholdsWhatItCannotReduce(t *testing.T) { + assert.Equal(t, redactedTicket, redactTicketURL("wss://chat.example.test/195539477")) + assert.Equal(t, redactedTicket, redactTicketURL("://")) + + redacted := redactTicketURL("wss://chat.example.test/195539477?ticket=tkt-secret") + assert.NotContains(t, redacted, "tkt-secret") + assert.Contains(t, redacted, "chat.example.test/195539477") +} + +func TestCheckContinuationFallsBackToTheDefaultHost(t *testing.T) { + require.NoError(t, checkContinuation("", feedBaseURL+feedEventsPath+"?position=p")) + require.Error(t, checkContinuation("", "http://3.basecampapi.com/99999/events.json")) + require.Error(t, checkContinuation("", "https://evil.example/99999/events.json")) +} diff --git a/internal/commands/inbox.go b/internal/commands/inbox.go new file mode 100644 index 000000000..1a39e0ac1 --- /dev/null +++ b/internal/commands/inbox.go @@ -0,0 +1,153 @@ +package commands + +import ( + "fmt" + + "github.com/spf13/cobra" + + "github.com/basecamp/basecamp-sdk/go/pkg/basecamp" + + "github.com/basecamp/basecamp-cli/internal/appctx" + "github.com/basecamp/basecamp-cli/internal/output" +) + +// NewInboxCmd creates the inbox command: the addressed-items lane of the +// account event feed. +func NewInboxCmd() *cobra.Command { + var ( + entry feedEntry + reasons []string + types []string + buckets []string + all bool + maxPages int + ) + + cmd := &cobra.Command{ + Use: "inbox", + Short: "Poll addressed items (agents only)", + Long: `Poll the authenticated principal's addressed items. + +The inbox is the low-noise "someone addressed you" lane of the account event +feed — mentions, assignments, subscriptions, watches, pings, and boosts of +your work. It is served to agent principals only for now; any other principal +receives a permission error. + + basecamp inbox --since now + basecamp inbox --since 0 --reasons mentioned,assigned + basecamp inbox --position "$POSITION" --all + +Each item is its own delivery: one event addresses you once per reason, so +deduplicate by addressing_id, never by the event's id.`, + Annotations: map[string]string{ + "agent_notes": "Account-wide — no --in <project> needed. Agents only; a person principal gets exit 4 (HTTP 403).\n" + + "Returns {items, position, next}. Persist 'position' only after processing the page's items, then resume with --position.\n" + + "--since takes an item id (int64), 'now' to enter at the present, or 0 to replay the earliest retained items (30-day retention).\n" + + "Deduplicate by addressing_id, not by event id: one event addresses you once per reason and each reason is its own item.\n" + + "Items are thin pointers — refetch the referenced recording with 'basecamp show <recording_id>' before acting on it.\n" + + "Items are never self-addressed, so no loop guard is needed here.\n" + + "Inbox positions are never interchangeable with 'basecamp events poll' positions.\n" + + "--all walks 'next' to the end of the current walk; it is not a live tail.", + }, + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, args []string) error { + app := appctx.FromContext(cmd.Context()) + + if err := entry.validate(); err != nil { + return err + } + if maxPages < 1 { + return output.ErrUsage("--max-pages must be at least 1") + } + + bucketIDs, err := feedIDs(trimFilter(buckets), "--buckets") + if err != nil { + return err + } + + if err := ensureAccount(cmd, app); err != nil { + return err + } + + opts := &basecamp.PollInboxOptions{ + Since: entry.since, + Position: entry.position, + Reasons: trimFilter(reasons), + Types: trimFilter(types), + Buckets: bucketIDs, + } + + items := make([]basecamp.InboxItem, 0) + var position, next string + pages, capped := 0, false + service := app.Account().EventFeed() + + for { + page, err := service.PollInbox(cmd.Context(), opts) + if err != nil { + return feedError(inboxLane, err) + } + pages++ + items = append(items, page.Items...) + position, next = page.Position, page.Next + + if !all || next == "" { + break + } + if pages >= maxPages { + capped = true + break + } + if err := checkContinuation(app.Config.BaseURL, next); err != nil { + return err + } + if opts, err = basecamp.PollInboxOptionsFromURL(next); err != nil { + return convertSDKError(err) + } + } + + summary := fmt.Sprintf("%d addressed item(s)", len(items)) + if pages > 1 { + summary = fmt.Sprintf("%s over %d pages", summary, pages) + } + if next != "" && !capped { + summary += "; more to serve" + } + + respOpts := []output.ResponseOption{ + output.WithSummary(summary), + output.WithBreadcrumbs( + output.Breadcrumb{ + Action: "resume", + Cmd: "basecamp inbox --position <position>", + Description: "Poll again from this page's position", + }, + output.Breadcrumb{ + Action: "show", + Cmd: "basecamp show <recording_id>", + Description: "Fetch a referenced recording", + }, + ), + } + if notice := feedWalkNotice(capped, maxPages); notice != "" { + respOpts = append(respOpts, output.WithNotice(notice)) + } + + return app.OK(map[string]any{ + "items": items, + "position": position, + "next": next, + }, respOpts...) + }, + } + + cmd.Flags().StringVar(&entry.since, "since", "", "Enter the inbox: an item id to start after, 'now', or 0 for the earliest retained items") + cmd.Flags().StringVar(&entry.position, "position", "", "Resume from a position token a previous page issued") + cmd.Flags().StringSliceVar(&reasons, "reasons", nil, "Filter by addressing reason: mentioned, assigned, subscribed, watched, pinged, boosted") + cmd.Flags().StringSliceVar(&types, "types", nil, "Narrow by event type (comma-separated, e.g. comment.created)") + cmd.Flags().StringSliceVar(&buckets, "buckets", nil, "Narrow by bucket (project) id (comma-separated)") + cmd.Flags().BoolVar(&all, "all", false, "Walk 'next' to the end of the current walk (not a live tail)") + cmd.Flags().IntVar(&maxPages, "max-pages", feedDefaultMaxPages, "Maximum pages to fetch with --all") + + return cmd +} diff --git a/internal/commands/mcp_test.go b/internal/commands/mcp_test.go index 8933a2e1a..2caaf42c2 100644 --- a/internal/commands/mcp_test.go +++ b/internal/commands/mcp_test.go @@ -141,7 +141,7 @@ func TestMCPCommandServesMCP(t *testing.T) { require.NoError(t, err) names = append(names, tool.Name) } - assert.Len(t, names, 16, "tools = %v", names) + assert.Len(t, names, 17, "tools = %v", names) result, err := session.CallTool(context.Background(), &mcp.CallToolParams{ Name: "basecamp_projects", diff --git a/internal/commands/recording_test.go b/internal/commands/recording_test.go index 0deced550..0d563a4e2 100644 --- a/internal/commands/recording_test.go +++ b/internal/commands/recording_test.go @@ -52,6 +52,13 @@ type stubRoute struct { // empty page, so a route that served the same body for every page number // would walk all the way to the cap and hide the behavior under test. pages []string + + // bodies, when set, serves bodies[i] to the i-th request this route + // matches and repeats the last entry past the end. Routes match on method + // and path alone, so a walk that re-requests one path with a different + // query — the event feed's position/next envelope, which carries no page + // number — needs the sequence to vary at all. + bodies []string } // pagedBody picks the response for a page-aware route. @@ -74,6 +81,9 @@ type recordingTransport struct { mu sync.Mutex requests []recordedCall routes []stubRoute + // matches counts, per route index, how many requests that route has + // served, so a route with a body sequence can advance through it. + matches map[int]int } func (t *recordingTransport) RoundTrip(req *http.Request) (*http.Response, error) { @@ -96,10 +106,13 @@ func (t *recordingTransport) RoundTrip(req *http.Request) (*http.Response, error }) t.mu.Unlock() - for _, route := range t.routes { + for index, route := range t.routes { if route.method == req.Method && route.path == req.URL.Path { responseBody := route.body - if route.pages != nil { + switch { + case route.bodies != nil: + responseBody = route.bodies[t.nextBody(index, len(route.bodies))] + case route.pages != nil: responseBody = pagedBody(route, req.URL.Query().Get("page")) } return &http.Response{ @@ -120,6 +133,23 @@ func (t *recordingTransport) RoundTrip(req *http.Request) (*http.Response, error }, nil } +// nextBody returns the index into a route's body sequence for this match, +// clamped to the last entry so a walk that outruns the fixture keeps getting +// the terminal page rather than panicking. +func (t *recordingTransport) nextBody(route, length int) int { + t.mu.Lock() + defer t.mu.Unlock() + if t.matches == nil { + t.matches = make(map[int]int) + } + index := t.matches[route] + t.matches[route]++ + if index >= length { + return length - 1 + } + return index +} + // recorded returns a snapshot of the requests seen so far. func (t *recordingTransport) recorded() []recordedCall { t.mu.Lock() diff --git a/scripts/check-bare-groups.sh b/scripts/check-bare-groups.sh index 06085e1df..d5467e4e6 100755 --- a/scripts/check-bare-groups.sh +++ b/scripts/check-bare-groups.sh @@ -18,6 +18,7 @@ ALLOWLIST=( NewURLCmd # shortcut: opens URL NewAssignmentsCmd # shortcut: shows assignments NewNotificationsCmd # shortcut: lists notifications + NewEventsCmd # shortcut: one recording's history, plus the account feed's subcommands ) is_allowed() { diff --git a/skills/basecamp/SKILL.md b/skills/basecamp/SKILL.md index a22915395..70182cf58 100644 --- a/skills/basecamp/SKILL.md +++ b/skills/basecamp/SKILL.md @@ -143,7 +143,7 @@ Full CLI coverage: 189 tracked in-scope endpoints across todos, cards, messages, renders as an empty bullet list. When the CLI version is unknown, check `basecamp --version` first, or pass the content portably as `"$(cat file.md)"` and verify the posted `content` when it matters. -6. **Project scope is mandatory for most commands** — via `--in <project>` or `.basecamp/config.json`. Cross-project exceptions: `basecamp reports assigned` for assigned work, `basecamp assignments` for structured assignment views, `basecamp reports overdue` for overdue todos, `basecamp reports schedule` for upcoming schedule across all projects, `basecamp recordings <type>` for browsing by type, `basecamp notifications` for notifications, `basecamp gauges list` for account-wide gauges, and the seven list commands covered in item 7. +6. **Project scope is mandatory for most commands** — via `--in <project>` or `.basecamp/config.json`. Cross-project exceptions: `basecamp reports assigned` for assigned work, `basecamp assignments` for structured assignment views, `basecamp reports overdue` for overdue todos, `basecamp reports schedule` for upcoming schedule across all projects, `basecamp recordings <type>` for browsing by type, `basecamp notifications` for notifications, `basecamp gauges list` for account-wide gauges, `basecamp events poll` and `basecamp inbox` for the account event feed, and the seven list commands covered in item 7. 7. **Account-wide listing.** `basecamp todos list --all-projects --json` lists across every project; the same flag does the same on `cards list`, `messages list`, `comments list`, `files list`, `forwards list`, and `checkins answers`. It overrides a configured project, and with no project in scope those commands already list account-wide rather than prompting. Flags that name something inside a single project are rejected there rather than silently ignored. Account-wide listings return **the first 100 items by default** — account-wide "all" is the whole account, not one project's worth. Use `--limit N` to raise the cap (it walks pages until N are collected) or `--all` for everything. `--page N` fetches exactly one page, but only on the paginated listings. The two overdue variants — `basecamp todos list --all-projects --overdue` and `basecamp cards list --all-projects --overdue` — come from unpaginated endpoints. They accept `--limit` and `--all` but **reject `--page`**, so do not generate `--page` against them. @@ -262,6 +262,8 @@ basecamp <cmd> --page 1 # First page only, no auto-pagination | Show + download | `basecamp todos show <id> --download-attachments --json` | | Stream attachment to stdout | `basecamp attachments download <id> --file <name> --out -` | | Change history for an item | `basecamp events <id\|url> --json` (when a card moved columns, when a todo was completed) | +| Account-wide activity feed (resumable) | `basecamp events poll --since now --json` (then resume with `--position`) | +| Items that addressed me (agents only) | `basecamp inbox --since now --json` | | Search | `basecamp search "query" --json` | | Parse URL | `basecamp url parse "<url>" --json` | | Upload file | `basecamp files uploads create <file> [--vault <folder_id>] --in <project> --json` | @@ -943,6 +945,49 @@ can tell you. `--page` accepts only `1`; use `--all` to walk every page. +### Event feed (account-wide, resumable) + +`basecamp events <id>` is one recording's history. The account-wide **event +feed** is a different resource, and the way an agent hears about activity it +did not cause: + +```bash +basecamp events poll --since now --json # Enter at the present +basecamp events poll --position "$POSITION" --json # Resume from a held position +basecamp events poll --since 0 --all --json # Replay served history +basecamp inbox --since now --json # Addressed items (agents only) +basecamp events ticket --json # Mint a live-stream ticket +``` + +Each page is an envelope: `events` (or `items`), a durable `position`, and a +`next` continuation URL while the current walk has more to serve. Persist +`position` only after processing the page, then pass it back with +`--position`. `--all` walks `next` to the end of the current walk — it is not +a live tail. + +The feed is a notification lane, not an audit log: + +- Deduplicate by event id — polls repeat what the live stream delivered, and + an event can appear up to ~30 seconds after it happens. +- Deduplicate inbox items by `addressing_id`, never by event id: one event + addresses you once per reason and each reason is its own item. +- Events are thin pointers. Refetch the recording (`basecamp show + <recording_id>`) before acting on it. +- An agent that acts on what it hears should pass `--exclude-performers self`, + which drops its own performances without hiding other agents' activity. + +Filters (`--types`, `--buckets`, `--creators`, `--performers`, +`--exclude-performers`, `--actor-types`; the inbox adds `--reasons`) are +comma-separated. Changing them invalidates a held position: the poll exits +`1` naming both filter digests, and the fix is to re-enter with `--since`. A +position that is no longer servable exits `2` and the hint names the +`--since` to re-enter with. + +`basecamp inbox` is served to agent principals only for now; other principals +get exit `4`. `basecamp events ticket` redacts the ticket and its URL unless +`--show-secret` is passed — both are bearers, so never log either, and mint a +fresh one per connection attempt. + ### Recordings (Cross-project) Use `basecamp recordings <type>` for cross-project type browsing. **For assigned todos, prefer `basecamp reports assigned`** — recordings do not include assignee data and cannot be filtered by person. From 0237fb633a1f20900807fabf2f647c1cfac0da4b Mon Sep 17 00:00:00 2001 From: Jeremy Daer <jeremy@37signals.com> Date: Wed, 16 Sep 2026 05:32:28 -0700 Subject: [PATCH 03/29] Name the lane's own identifier in an invalid --since, and re-hash the flake The two poll lanes number different things: --since takes an event id on the feed and an addressed-item id on the inbox. One shared remedy string sent an inbox caller looking for the wrong identifier, so the hint now comes from the lane, which already carried the rest of its per-lane text. The SDK pin also moved vendored dependencies, which invalidates the Nix flake's vendorHash. Recomputed and verified against a Docker nix build. --- internal/commands/feed.go | 22 ++++++++++++---------- internal/commands/feed_test.go | 12 ++++++++++++ internal/commands/inbox.go | 2 +- nix/package.nix | 2 +- 4 files changed, 26 insertions(+), 12 deletions(-) diff --git a/internal/commands/feed.go b/internal/commands/feed.go index 89ab33aca..a66922f79 100644 --- a/internal/commands/feed.go +++ b/internal/commands/feed.go @@ -45,6 +45,7 @@ const feedDefaultMaxPages = 50 type feedLane struct { name string pollCmd string + sinceHint string resumeSince string forbiddenMsg string forbiddenHint string @@ -52,12 +53,14 @@ type feedLane struct { var ( eventsLane = feedLane{ - name: "event", - pollCmd: "basecamp events poll", + name: "event", + pollCmd: "basecamp events poll", + sinceHint: "Pass an event id to start after, 'now' to enter at the present, or 0 to replay served history", } inboxLane = feedLane{ name: "inbox item", pollCmd: "basecamp inbox", + sinceHint: "Pass an inbox item id to start after, 'now' to enter at the present, or 0 for the earliest retained items", resumeSince: basecamp.SinceEpoch, forbiddenMsg: "The inbox is served to agent principals only", forbiddenHint: "Authenticate as an agent, or use 'basecamp events poll' for the account-wide feed", @@ -72,10 +75,12 @@ type feedEntry struct { } // validate rejects the two entry points the server would reject anyway, with -// a message that names the fix. The shape of --since is checked, never its -// value: an event id the feed has never served is the server's verdict to +// a message that names the fix. The remedy comes from the lane, because the +// two lanes number different things: --since takes an event id on the feed +// and an addressed-item id on the inbox. The shape of --since is checked, +// never its value: an id the lane has never served is the server's verdict to // give, not ours. -func (e feedEntry) validate() error { +func (e feedEntry) validate(lane feedLane) error { switch { case e.since != "" && e.position != "": return output.ErrUsage("--since and --position are mutually exclusive") @@ -83,10 +88,7 @@ func (e feedEntry) validate() error { return nil } if _, err := strconv.ParseInt(e.since, 10, 64); err != nil { - return output.ErrUsageHint( - fmt.Sprintf("Invalid --since: %s", e.since), - "Pass an event id to start after, 'now' to enter at the present, or 0 to replay served history", - ) + return output.ErrUsageHint(fmt.Sprintf("Invalid --since: %s", e.since), lane.sinceHint) } return nil } @@ -319,7 +321,7 @@ event id and refetch the referenced recording before acting on it.`, RunE: func(cmd *cobra.Command, args []string) error { app := appctx.FromContext(cmd.Context()) - if err := entry.validate(); err != nil { + if err := entry.validate(eventsLane); err != nil { return err } if maxPages < 1 { diff --git a/internal/commands/feed_test.go b/internal/commands/feed_test.go index 25683cd7d..0c075af9f 100644 --- a/internal/commands/feed_test.go +++ b/internal/commands/feed_test.go @@ -427,6 +427,18 @@ func TestInboxPositionGoneReEntersAtTheEarliestRetainedItem(t *testing.T) { assert.Equal(t, "Re-enter with: basecamp inbox --since 0", cliErr.Hint) } +// The two lanes number different things, so the remedy an invalid --since +// offers has to name the lane's own identifier rather than the feed's. +func TestInvalidSinceNamesTheLanesOwnIdentifier(t *testing.T) { + app, _, _ := setupFeedApp(t) + err := executeRecordingCommand(NewEventsCmd(), app, "poll", "--since", "yesterday") + assert.Contains(t, requireFeedError(t, err).Hint, "event id") + + app, _, _ = setupFeedApp(t) + err = executeRecordingCommand(NewInboxCmd(), app, "--since", "yesterday") + assert.Contains(t, requireFeedError(t, err).Hint, "inbox item id") +} + func TestInboxRejectsUnservableFlagsBeforeAnyRequest(t *testing.T) { app, transport, _ := setupFeedApp(t, inboxRoute(http.StatusOK, inboxPageJSON("inbox-pos-1", ""))) diff --git a/internal/commands/inbox.go b/internal/commands/inbox.go index 1a39e0ac1..67bbc7948 100644 --- a/internal/commands/inbox.go +++ b/internal/commands/inbox.go @@ -53,7 +53,7 @@ deduplicate by addressing_id, never by the event's id.`, RunE: func(cmd *cobra.Command, args []string) error { app := appctx.FromContext(cmd.Context()) - if err := entry.validate(); err != nil { + if err := entry.validate(inboxLane); err != nil { return err } if maxPages < 1 { diff --git a/nix/package.nix b/nix/package.nix index 854f20eca..3cfa0e643 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-qp2y5dPFcIjbJG133bTrqvy10lwguipA597VVIJcXkA="; + vendorHash = "sha256-cpyRw1eZo241lxDP5w2LRkNAQIiKVdDi6dGMBq5TctE="; subPackages = [ "cmd/basecamp" ]; From 571cb3bffa9f98eb78f49d52abd707825499924c Mon Sep 17 00:00:00 2001 From: Jeremy Daer <jeremy@37signals.com> Date: Wed, 16 Sep 2026 05:41:17 -0700 Subject: [PATCH 04/29] Preserve filters when re-entering a gone position, and count feed rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review findings, all real. A 410's resume URL preserves the request's canonical filters, and the hint threw them away: re-entering with a bare --since drops --types, --buckets and the --exclude-performers self loop guard, so an agent recovering from an expired position would start consuming its own activity. The hint now renders the filters back out of the resume URL through the lane's own parser. A resume URL that will not parse, or one carrying a value outside srv2's alphabet, falls back to the bare re-entry — a hint is meant to be pasted, and half-rendered server text is not. The continuation origin check compared raw scheme and host strings, so a base URL written with an explicit default port, or a host in different case, rejected its own continuations and broke --all after the first page. It now compares the scheme case-insensitively and reuses this package's sameHostPort, which normalizes the omitted port to the scheme's default. Both poll commands handed the whole envelope to the output writer, which is one object: --count answered 1 for every page and --ids-only found no id to print. The rows go through WithDisplayData, the seam the account-wide listings already use for exactly this. An inbox row's identity is its addressing id, because applying the feed's dedupe-by-event-id rule to the inbox would discard every reason but one. --- internal/commands/feed.go | 171 ++++++++++++++++++++++++++++----- internal/commands/feed_test.go | 67 +++++++++++++ internal/commands/inbox.go | 37 +++++++ 3 files changed, 250 insertions(+), 25 deletions(-) diff --git a/internal/commands/feed.go b/internal/commands/feed.go index a66922f79..6fc8ed4aa 100644 --- a/internal/commands/feed.go +++ b/internal/commands/feed.go @@ -43,30 +43,63 @@ const feedDefaultMaxPages = 50 // being servable. The feed re-enters at the epoch the 410 body names; the // inbox re-enters at since=0, the earliest item still retained. type feedLane struct { - name string - pollCmd string - sinceHint string - resumeSince string - forbiddenMsg string - forbiddenHint string + name string + pollCmd string + sinceHint string + resumeSince string + // continuationFilters reads the filter flags a continuation or resume URL + // carries, through the lane's own parser: the two lanes have different + // filter dimensions and their URLs are never interchangeable. + continuationFilters func(string) ([]flagValues, error) + forbiddenMsg string + forbiddenHint string } var ( eventsLane = feedLane{ - name: "event", - pollCmd: "basecamp events poll", - sinceHint: "Pass an event id to start after, 'now' to enter at the present, or 0 to replay served history", + name: "event", + pollCmd: "basecamp events poll", + sinceHint: "Pass an event id to start after, 'now' to enter at the present, or 0 to replay served history", + continuationFilters: eventsContinuationFilters, } inboxLane = feedLane{ - name: "inbox item", - pollCmd: "basecamp inbox", - sinceHint: "Pass an inbox item id to start after, 'now' to enter at the present, or 0 for the earliest retained items", - resumeSince: basecamp.SinceEpoch, - forbiddenMsg: "The inbox is served to agent principals only", - forbiddenHint: "Authenticate as an agent, or use 'basecamp events poll' for the account-wide feed", + name: "inbox item", + pollCmd: "basecamp inbox", + sinceHint: "Pass an inbox item id to start after, 'now' to enter at the present, or 0 for the earliest retained items", + resumeSince: basecamp.SinceEpoch, + continuationFilters: inboxContinuationFilters, + forbiddenMsg: "The inbox is served to agent principals only", + forbiddenHint: "Authenticate as an agent, or use 'basecamp events poll' for the account-wide feed", } ) +func eventsContinuationFilters(raw string) ([]flagValues, error) { + opts, err := basecamp.PollEventsOptionsFromURL(raw) + if err != nil { + return nil, err + } + return []flagValues{ + {"--types", opts.Types}, + {"--buckets", int64Strings(opts.Buckets)}, + {"--creators", int64Strings(opts.Creators)}, + {"--performers", opts.Performers}, + {"--exclude-performers", opts.ExcludePerformers}, + {"--actor-types", opts.ActorTypes}, + }, nil +} + +func inboxContinuationFilters(raw string) ([]flagValues, error) { + opts, err := basecamp.PollInboxOptionsFromURL(raw) + if err != nil { + return nil, err + } + return []flagValues{ + {"--reasons", opts.Reasons}, + {"--types", opts.Types}, + {"--buckets", int64Strings(opts.Buckets)}, + }, nil +} + // feedEntry is the entry point a poll presents: an explicit since, a held // position, or neither (which enters at the present). type feedEntry struct { @@ -168,6 +201,97 @@ func trimFilter(values []string) []string { return trimmed } +// resumeCommand renders the invocation that re-enters this lane after a 410. +// +// The server's resume URL preserves the request's canonical filters, and a +// bare --since would drop every one of them — including the +// --exclude-performers self loop guard, whose absence turns an agent's +// recovery into a run on its own activity. So the filters come back out of +// the resume URL rather than being left behind in it. +// +// A resume URL the SDK cannot parse, or one carrying a filter value outside +// the contract's alphabet, falls back to the lane's bare re-entry: a hint is +// meant to be pasted, and half-rendered server text is not. +func (l feedLane) resumeCommand(gone *basecamp.FeedPositionGoneError) string { + since := l.resumeSince + if gone.EpochAfterID != nil { + since = strconv.FormatInt(*gone.EpochAfterID, 10) + } + if since == "" { + since = basecamp.SinceNow + } + command := fmt.Sprintf("%s --since %s", l.pollCmd, since) + + filters, ok := l.resumeFilters(gone.Resume) + if !ok { + return command + } + return command + filters +} + +// resumeFilters renders the filter flags a resume URL carries, or reports +// that it carries none it can render. +func (l feedLane) resumeFilters(resume string) (string, bool) { + if resume == "" { + return "", false + } + flags, err := l.continuationFilters(resume) + if err != nil { + return "", false + } + + rendered := "" + for _, flag := range flags { + if len(flag.values) == 0 { + continue + } + for _, value := range flag.values { + if !renderableFilterValue(value) { + return "", false + } + } + rendered += fmt.Sprintf(" %s %s", flag.name, strings.Join(flag.values, ",")) + } + return rendered, rendered != "" +} + +// flagValues pairs a flag with the comma-joined list it carries. +type flagValues struct { + name string + values []string +} + +func int64Strings(values []int64) []string { + if len(values) == 0 { + return nil + } + rendered := make([]string, len(values)) + for i, value := range values { + rendered[i] = strconv.FormatInt(value, 10) + } + return rendered +} + +// renderableFilterValue reports whether a filter value from a server response +// is safe to paste into a shell command unquoted. srv2's domain is cataloged +// type strings, actor types, addressing reasons and integer ids, so anything +// outside that alphabet means the URL is not what the contract describes and +// the whole hint falls back rather than quoting around the surprise. +func renderableFilterValue(value string) bool { + if value == "" { + return false + } + for _, r := range value { + switch { + case r >= 'a' && r <= 'z', r >= 'A' && r <= 'Z', r >= '0' && r <= '9': + case r == '.', r == '_', r == '-': + default: + return false + } + } + return true +} + // feedError maps the poll lanes' documented failures onto the CLI's error // taxonomy. Every branch is an errors.As on a typed SDK error, so the split // the SDK is adding — a per-lane 410 and a reason-carrying 400 — lands as two @@ -200,14 +324,6 @@ func feedError(lane feedLane, err error) error { var gone *basecamp.FeedPositionGoneError if errors.As(err, &gone) { - since := lane.resumeSince - if gone.EpochAfterID != nil { - since = strconv.FormatInt(*gone.EpochAfterID, 10) - } - hint := fmt.Sprintf("Re-enter with: %s --since %s", lane.pollCmd, since) - if since == "" { - hint = fmt.Sprintf("Re-enter with: %s --since now", lane.pollCmd) - } message := fmt.Sprintf("Position is no longer servable for the %s feed", lane.name) if gone.Resume != "" { message = fmt.Sprintf("%s. Resume URL: %s", message, gone.Resume) @@ -215,7 +331,7 @@ func feedError(lane feedLane, err error) error { return &output.Error{ Code: output.CodeNotFound, Message: message, - Hint: hint, + Hint: "Re-enter with: " + lane.resumeCommand(gone), HTTPStatus: http.StatusGone, Cause: err, } @@ -256,7 +372,7 @@ func checkContinuation(baseURL, raw string) error { Message: "Continuation URL is not a URL", } } - if next.Scheme != base.Scheme || next.Host != base.Host { + if !strings.EqualFold(next.Scheme, base.Scheme) || !sameHostPort(next, base) { return &output.Error{ Code: output.CodeAPI, Message: "Continuation URL points outside the configured Basecamp API host", @@ -403,6 +519,11 @@ event id and refetch the referenced recording before acting on it.`, respOpts := []output.ResponseOption{ output.WithSummary(summary), + // The envelope is the machine contract, but it is one object, + // so --count would answer 1 for any page and --ids-only would + // find no id to print. The rows are what those modes are + // about; --json keeps the resumable envelope. + output.WithDisplayData(events), output.WithBreadcrumbs( output.Breadcrumb{ Action: "resume", diff --git a/internal/commands/feed_test.go b/internal/commands/feed_test.go index 0c075af9f..ea127b098 100644 --- a/internal/commands/feed_test.go +++ b/internal/commands/feed_test.go @@ -488,6 +488,73 @@ func TestRedactTicketURLWithholdsWhatItCannotReduce(t *testing.T) { assert.Contains(t, redacted, "chat.example.test/195539477") } +// A 410 on a filtered poll must hand back a re-entry that still carries the +// filters — the loop guard above all, since an agent that re-enters without +// it recovers straight into its own activity. +func TestPositionGoneRebuildsTheFiltersFromTheResumeURL(t *testing.T) { + resume := feedBaseURL + feedEventsPath + + "?since=900&types=message.created,comment.created&buckets=1,2&exclude_performers=51177542&actor_types=agent" + app, _, _ := setupFeedApp(t, eventsRoute(http.StatusGone, + fmt.Sprintf(`{"error":"position predates the feed epoch","epoch_after_id":900,"resume":%q}`, resume))) + + err := executeRecordingCommand(NewEventsCmd(), app, "poll", "--position", "ancient", + "--types", "message.created,comment.created") + + assert.Equal(t, + "Re-enter with: basecamp events poll --since 900 --types message.created,comment.created"+ + " --buckets 1,2 --exclude-performers 51177542 --actor-types agent", + requireFeedError(t, err).Hint) +} + +func TestInboxPositionGoneRebuildsItsOwnFilters(t *testing.T) { + resume := feedBaseURL + feedInboxPath + "?since=0&reasons=mentioned,assigned" + app, _, _ := setupFeedApp(t, inboxRoute(http.StatusGone, + fmt.Sprintf(`{"error":"position fell behind the retention window","resume":%q}`, resume))) + + err := executeRecordingCommand(NewInboxCmd(), app, "--position", "ancient") + + assert.Equal(t, "Re-enter with: basecamp inbox --since 0 --reasons mentioned,assigned", + requireFeedError(t, err).Hint) +} + +// A resume URL carrying anything outside srv2's alphabet is not what the +// contract describes, so the hint falls back to the bare re-entry rather than +// pasting server text into a command line. +func TestPositionGoneFallsBackWhenTheResumeURLIsNotRenderable(t *testing.T) { + resume := feedBaseURL + feedEventsPath + "?since=900&types=" + url.QueryEscape("message.created; rm -rf /") + app, _, _ := setupFeedApp(t, eventsRoute(http.StatusGone, + fmt.Sprintf(`{"error":"position predates the feed epoch","epoch_after_id":900,"resume":%q}`, resume))) + + err := executeRecordingCommand(NewEventsCmd(), app, "poll", "--position", "ancient") + + assert.Equal(t, "Re-enter with: basecamp events poll --since 900", requireFeedError(t, err).Hint) +} + +// --count counts the page's rows and --ids-only prints their identities; for +// the inbox that identity is the addressing id, never the event id. +func TestEnumeratingOutputModesSeeTheRows(t *testing.T) { + app, _, out := setupFeedApp(t, eventsRoute(http.StatusOK, feedPageJSON("pos-1", "", 11, 12))) + app.Output = output.New(output.Options{Format: output.FormatCount, Writer: out}) + require.NoError(t, executeRecordingCommand(NewEventsCmd(), app, "poll", "--since", "now")) + assert.Equal(t, "2\n", out.String()) + + app, _, out = setupFeedApp(t, eventsRoute(http.StatusOK, feedPageJSON("pos-1", "", 11, 12))) + app.Output = output.New(output.Options{Format: output.FormatIDs, Writer: out}) + require.NoError(t, executeRecordingCommand(NewEventsCmd(), app, "poll", "--since", "now")) + assert.Equal(t, "11\n12\n", out.String()) + + app, _, out = setupFeedApp(t, inboxRoute(http.StatusOK, inboxPageJSON("inbox-pos-1", "", 991, 992))) + app.Output = output.New(output.Options{Format: output.FormatIDs, Writer: out}) + require.NoError(t, executeRecordingCommand(NewInboxCmd(), app, "--since", "now")) + assert.Equal(t, "991\n992\n", out.String()) +} + +func TestContinuationOriginComparisonNormalizesDefaultPortsAndCase(t *testing.T) { + require.NoError(t, checkContinuation("https://3.basecampapi.com:443", feedBaseURL+feedEventsPath+"?position=p")) + require.NoError(t, checkContinuation(feedBaseURL, "HTTPS://3.BasecampAPI.com"+feedEventsPath+"?position=p")) + require.Error(t, checkContinuation(feedBaseURL, "https://3.basecampapi.com:8443"+feedEventsPath)) +} + func TestCheckContinuationFallsBackToTheDefaultHost(t *testing.T) { require.NoError(t, checkContinuation("", feedBaseURL+feedEventsPath+"?position=p")) require.Error(t, checkContinuation("", "http://3.basecampapi.com/99999/events.json")) diff --git a/internal/commands/inbox.go b/internal/commands/inbox.go index 67bbc7948..9246ee7d0 100644 --- a/internal/commands/inbox.go +++ b/internal/commands/inbox.go @@ -2,6 +2,7 @@ package commands import ( "fmt" + "time" "github.com/spf13/cobra" @@ -116,6 +117,13 @@ deduplicate by addressing_id, never by the event's id.`, respOpts := []output.ResponseOption{ output.WithSummary(summary), + // Rows for the enumerating output modes, as in events poll — + // and an item's identity here is its addressing id, so that is + // what the row's id is. The machine envelope keeps + // addressing_id under its own name, because applying the + // feed's dedupe-by-event-id rule to the inbox would discard + // every reason but one. + output.WithDisplayData(inboxRows(items)), output.WithBreadcrumbs( output.Breadcrumb{ Action: "resume", @@ -151,3 +159,32 @@ deduplicate by addressing_id, never by the event's id.`, return cmd } + +// inboxRow is one addressed item flattened for display and for the +// enumerating output modes: the item's own identity as id, the reason it +// addressed you, and the pointers you need to fetch what it points at. +type inboxRow struct { + ID int64 `json:"id"` + Reason string `json:"reason"` + AddressedAt time.Time `json:"addressed_at"` + EventID int64 `json:"event_id"` + EventType string `json:"event_type"` + BucketID int64 `json:"bucket_id"` + RecordingID int64 `json:"recording_id"` +} + +func inboxRows(items []basecamp.InboxItem) []inboxRow { + rows := make([]inboxRow, 0, len(items)) + for _, item := range items { + rows = append(rows, inboxRow{ + ID: item.AddressingID, + Reason: item.Reason, + AddressedAt: item.AddressedAt, + EventID: item.Event.ID, + EventType: item.Event.EventType, + BucketID: item.Event.BucketID, + RecordingID: item.Event.RecordingID, + }) + } + return rows +} From d61a138a6765212f103af91951cc39b9e5cb6f4b Mon Sep 17 00:00:00 2001 From: Jeremy Daer <jeremy@37signals.com> Date: Wed, 16 Sep 2026 05:47:17 -0700 Subject: [PATCH 05/29] Run the event-feed smoke suite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The file was written but named in no level array, so `make smoke` never ran it while the static coverage check — which scans files, not the arrays — reported it covered. It joins Level 0: the ticket mint is a POST, but it writes no record and leaves nothing behind for a parallel suite. The inbox case is allowlisted as an accepted gap. The lane is served to agent principals only, so a person's smoke token gets the 403 the test asserts before marking the gap, and an unallowlisted unverifiable blocks the run. --- e2e/smoke/.qa-allowlist | 4 ++++ e2e/smoke/run_smoke.sh | 1 + e2e/smoke/smoke_event_feed.bats | 5 +++++ 3 files changed, 10 insertions(+) diff --git a/e2e/smoke/.qa-allowlist b/e2e/smoke/.qa-allowlist index 693f7c9b3..952ab0c12 100644 --- a/e2e/smoke/.qa-allowlist +++ b/e2e/smoke/.qa-allowlist @@ -42,3 +42,7 @@ test_cards_move_to_wormhole_needs_a_linked_wormhole_and_is_destructive # needs l # Bubble Ups — BC5-added endpoint, may not exist on all QA environments test_notifications_bubbleups_lists_bubble_ups # endpoint not available on some servers — smoke + +# Inbox — served to agent principals only; a person's smoke token gets 403, +# which the test asserts before marking the gap. +test_inbox_polls_addressed_items # agents-only lane, smoke token is a person — smoke diff --git a/e2e/smoke/run_smoke.sh b/e2e/smoke/run_smoke.sh index b78108a96..834744f3d 100755 --- a/e2e/smoke/run_smoke.sh +++ b/e2e/smoke/run_smoke.sh @@ -76,6 +76,7 @@ level0=( "$SMOKE_DIR"/smoke_checkins.bats "$SMOKE_DIR"/smoke_schedule.bats "$SMOKE_DIR"/smoke_config_local.bats + "$SMOKE_DIR"/smoke_event_feed.bats ) level0_exist=() for f in "${level0[@]}"; do diff --git a/e2e/smoke/smoke_event_feed.bats b/e2e/smoke/smoke_event_feed.bats index be3d968f4..8e81c77ac 100644 --- a/e2e/smoke/smoke_event_feed.bats +++ b/e2e/smoke/smoke_event_feed.bats @@ -5,6 +5,11 @@ # signed resume token and the other two are bearers, and a smoke trace is a # file somebody keeps. Entering at the present (--since now) also keeps these # reads bounded — a since=0 replay would walk the account's served history. +# +# This sits in Level 0 alongside the other read-only suites even though the +# ticket mint is a POST: it writes no record, mints a stateless signed token +# that expires in about two minutes, and leaves nothing behind for a parallel +# suite to trip over. load smoke_helper From 07b0639a6e722ac16fe06a2ebb14904b0ebd481d Mon Sep 17 00:00:00 2001 From: Jeremy Daer <jeremy@37signals.com> Date: Wed, 16 Sep 2026 06:01:46 -0700 Subject: [PATCH 06/29] Carry the poll's own filters into every re-entry it advertises MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A position is bound to the filter set it was minted for, so every command this code hands back has to carry the filters or it is a command that fails. Three places did not. The resume breadcrumb offered `--position <position>` alone. On a filtered poll that is a different filter set, and the server answers 409 — the breadcrumb advertised its own failure. It now renders the filters the poll used, on both lanes. The 409 hint did the same thing in the other direction: the caller has just changed filters, and the remedy told them to re-enter without any. It now repeats the filters they presented. And a 410's resume URL is server text that this code displays as authoritative and reads filters out of, which is acting on it — so it now faces the same origin check the walk applies to a continuation. A resume URL pointing elsewhere is neither shown nor parsed, and the re-entry falls back to the filters the request was invoked with. The filter rendering is one helper now, fed either by the lane's continuation parser or by the request's own options. The skill's two project-scope exception lists had drifted apart, and the feed section described the inbox as the event filter set plus --reasons. It is not a superset: --creators, --performers, --exclude-performers and --actor-types are not flags there, and generated calls using them would fail. Both lanes now list their own filters. --- internal/commands/feed.go | 139 ++++++++++++++++++++++----------- internal/commands/feed_test.go | 70 ++++++++++++++++- internal/commands/inbox.go | 12 ++- skills/basecamp/SKILL.md | 23 ++++-- 4 files changed, 187 insertions(+), 57 deletions(-) diff --git a/internal/commands/feed.go b/internal/commands/feed.go index 6fc8ed4aa..7a05f8164 100644 --- a/internal/commands/feed.go +++ b/internal/commands/feed.go @@ -78,14 +78,7 @@ func eventsContinuationFilters(raw string) ([]flagValues, error) { if err != nil { return nil, err } - return []flagValues{ - {"--types", opts.Types}, - {"--buckets", int64Strings(opts.Buckets)}, - {"--creators", int64Strings(opts.Creators)}, - {"--performers", opts.Performers}, - {"--exclude-performers", opts.ExcludePerformers}, - {"--actor-types", opts.ActorTypes}, - }, nil + return eventsFilterFlags(opts), nil } func inboxContinuationFilters(raw string) ([]flagValues, error) { @@ -93,11 +86,7 @@ func inboxContinuationFilters(raw string) ([]flagValues, error) { if err != nil { return nil, err } - return []flagValues{ - {"--reasons", opts.Reasons}, - {"--types", opts.Types}, - {"--buckets", int64Strings(opts.Buckets)}, - }, nil + return inboxFilterFlags(opts), nil } // feedEntry is the entry point a poll presents: an explicit since, a held @@ -201,45 +190,67 @@ func trimFilter(values []string) []string { return trimmed } -// resumeCommand renders the invocation that re-enters this lane after a 410. +// feedRequest is what a poll knows about itself when it has to tell a caller +// how to come back: which lane it is, the configured API base a +// server-supplied URL has to match, and the filter flags it was invoked with. // -// The server's resume URL preserves the request's canonical filters, and a -// bare --since would drop every one of them — including the -// --exclude-performers self loop guard, whose absence turns an agent's -// recovery into a run on its own activity. So the filters come back out of -// the resume URL rather than being left behind in it. +// Every re-entry carries the filters, because a position is bound to its +// filter set: a resume command that dropped them would 409 on the spot, and +// dropping --exclude-performers self turns an agent's recovery into a run on +// its own activity. +type feedRequest struct { + lane feedLane + baseURL string + filters string +} + +// resumeCommand renders the invocation that re-enters this lane after a 410. // -// A resume URL the SDK cannot parse, or one carrying a filter value outside -// the contract's alphabet, falls back to the lane's bare re-entry: a hint is -// meant to be pasted, and half-rendered server text is not. -func (l feedLane) resumeCommand(gone *basecamp.FeedPositionGoneError) string { - since := l.resumeSince +// The server's resume URL preserves the request's canonical filters, so they +// come back out of it when it is one this client would follow. A resume URL +// that fails the origin check, that the SDK cannot parse, or that carries a +// filter value outside the contract's alphabet falls back to the filters this +// request was invoked with: a hint is meant to be pasted, and half-rendered +// server text is not. +func (r feedRequest) resumeCommand(gone *basecamp.FeedPositionGoneError) string { + since := r.lane.resumeSince if gone.EpochAfterID != nil { since = strconv.FormatInt(*gone.EpochAfterID, 10) } if since == "" { since = basecamp.SinceNow } - command := fmt.Sprintf("%s --since %s", l.pollCmd, since) + command := fmt.Sprintf("%s --since %s", r.lane.pollCmd, since) - filters, ok := l.resumeFilters(gone.Resume) - if !ok { - return command + if filters, ok := r.resumeFilters(gone.Resume); ok { + return command + filters } - return command + filters + return command + r.filters } // resumeFilters renders the filter flags a resume URL carries, or reports -// that it carries none it can render. -func (l feedLane) resumeFilters(resume string) (string, bool) { - if resume == "" { +// that it carries none this client would act on. +func (r feedRequest) resumeFilters(resume string) (string, bool) { + if resume == "" || !r.followable(resume) { return "", false } - flags, err := l.continuationFilters(resume) + flags, err := r.lane.continuationFilters(resume) if err != nil { return "", false } + return renderFilterFlags(flags) +} + +// followable reports whether a server-supplied URL passes the same origin +// check the walk applies to a continuation. A 410's resume URL is displayed +// as authoritative and read for filters, which is acting on it. +func (r feedRequest) followable(raw string) bool { + return checkContinuation(r.baseURL, raw) == nil +} +// renderFilterFlags renders flags as a pasteable suffix, reporting false when +// nothing was present or a value fell outside the alphabet below. +func renderFilterFlags(flags []flagValues) (string, bool) { rendered := "" for _, flag := range flags { if len(flag.values) == 0 { @@ -255,6 +266,34 @@ func (l feedLane) resumeFilters(resume string) (string, bool) { return rendered, rendered != "" } +// requestFilters renders the filter flags an invocation carried, for the +// resume breadcrumb and the re-entry hints. Unrenderable values yield an +// empty suffix; the surrounding description says to keep the same filters in +// words, which is true whether or not they can be pasted. +func requestFilters(flags []flagValues) string { + rendered, _ := renderFilterFlags(flags) + return rendered +} + +func eventsFilterFlags(opts *basecamp.PollEventsOptions) []flagValues { + return []flagValues{ + {"--types", opts.Types}, + {"--buckets", int64Strings(opts.Buckets)}, + {"--creators", int64Strings(opts.Creators)}, + {"--performers", opts.Performers}, + {"--exclude-performers", opts.ExcludePerformers}, + {"--actor-types", opts.ActorTypes}, + } +} + +func inboxFilterFlags(opts *basecamp.PollInboxOptions) []flagValues { + return []flagValues{ + {"--reasons", opts.Reasons}, + {"--types", opts.Types}, + {"--buckets", int64Strings(opts.Buckets)}, + } +} + // flagValues pairs a flag with the comma-joined list it carries. type flagValues struct { name string @@ -308,7 +347,7 @@ func renderableFilterValue(value string) bool { // 400 → whatever the server said. A malformed position and a malformed // filter have different fixes and the body says which; guessing // between them would send the caller to the wrong one. -func feedError(lane feedLane, err error) error { +func feedError(req feedRequest, err error) error { var mismatch *basecamp.FeedFilterMismatchError if errors.As(err, &mismatch) { return &output.Error{ @@ -316,7 +355,8 @@ func feedError(lane feedLane, err error) error { Message: fmt.Sprintf( "Position was minted for a different filter set (position digest %s, this request's filters digest %s)", mismatch.PositionDigest, mismatch.FiltersDigest), - Hint: fmt.Sprintf("Re-enter with --since to acknowledge the filter change: %s --since now", lane.pollCmd), + Hint: fmt.Sprintf("Re-enter with --since to acknowledge the filter change, keeping these filters: %s --since now%s", + req.lane.pollCmd, req.filters), HTTPStatus: http.StatusConflict, Cause: err, } @@ -324,25 +364,25 @@ func feedError(lane feedLane, err error) error { var gone *basecamp.FeedPositionGoneError if errors.As(err, &gone) { - message := fmt.Sprintf("Position is no longer servable for the %s feed", lane.name) - if gone.Resume != "" { + message := fmt.Sprintf("Position is no longer servable for the %s feed", req.lane.name) + if gone.Resume != "" && req.followable(gone.Resume) { message = fmt.Sprintf("%s. Resume URL: %s", message, gone.Resume) } return &output.Error{ Code: output.CodeNotFound, Message: message, - Hint: "Re-enter with: " + lane.resumeCommand(gone), + Hint: "Re-enter with: " + req.resumeCommand(gone), HTTPStatus: http.StatusGone, Cause: err, } } var sdkErr *basecamp.Error - if errors.As(err, &sdkErr) && sdkErr.HTTPStatus == http.StatusForbidden && lane.forbiddenMsg != "" { + if errors.As(err, &sdkErr) && sdkErr.HTTPStatus == http.StatusForbidden && req.lane.forbiddenMsg != "" { return &output.Error{ Code: output.CodeForbidden, - Message: lane.forbiddenMsg, - Hint: lane.forbiddenHint, + Message: req.lane.forbiddenMsg, + Hint: req.lane.forbiddenHint, HTTPStatus: http.StatusForbidden, Cause: err, } @@ -480,6 +520,12 @@ event id and refetch the referenced recording before acting on it.`, ActorTypes: kinds, } + request := feedRequest{ + lane: eventsLane, + baseURL: app.Config.BaseURL, + filters: requestFilters(eventsFilterFlags(opts)), + } + events := make([]basecamp.FeedEvent, 0) var position, next string pages, capped := 0, false @@ -488,7 +534,7 @@ event id and refetch the referenced recording before acting on it.`, for { page, err := service.PollEvents(cmd.Context(), opts) if err != nil { - return feedError(eventsLane, err) + return feedError(request, err) } pages++ events = append(events, page.Events...) @@ -526,9 +572,12 @@ event id and refetch the referenced recording before acting on it.`, output.WithDisplayData(events), output.WithBreadcrumbs( output.Breadcrumb{ - Action: "resume", - Cmd: "basecamp events poll --position <position>", - Description: "Poll again from this page's position", + Action: "resume", + // A position is bound to its filter set, so the + // resume command carries the filters this poll used; + // without them the server answers 409. + Cmd: "basecamp events poll --position <position>" + request.filters, + Description: "Poll again from this page's position, with the same filters", }, output.Breadcrumb{ Action: "show", diff --git a/internal/commands/feed_test.go b/internal/commands/feed_test.go index ea127b098..edc301e7e 100644 --- a/internal/commands/feed_test.go +++ b/internal/commands/feed_test.go @@ -277,7 +277,8 @@ func TestEventsPollFilterMismatchNamesBothDigests(t *testing.T) { app, _, _ := setupFeedApp(t, eventsRoute(http.StatusConflict, `{"error":"position was minted for a different filter set","position_digest":"44136fa355b3678a","filters_digest":"38b223c13c89dc89"}`)) - err := executeRecordingCommand(NewEventsCmd(), app, "poll", "--position", "stale") + err := executeRecordingCommand(NewEventsCmd(), app, "poll", "--position", "stale", + "--types", "message.created", "--exclude-performers", "self") cliErr := requireFeedError(t, err) assert.Equal(t, output.CodeUsage, cliErr.Code) @@ -285,7 +286,10 @@ func TestEventsPollFilterMismatchNamesBothDigests(t *testing.T) { assert.Equal(t, http.StatusConflict, cliErr.HTTPStatus) assert.Contains(t, cliErr.Message, "44136fa355b3678a") assert.Contains(t, cliErr.Message, "38b223c13c89dc89") - assert.Contains(t, cliErr.Hint, "--since") + // The filters the caller presented survive the recovery command: a + // re-entry without them is a different filter set again. + assert.Contains(t, cliErr.Hint, + "basecamp events poll --since now --types message.created --exclude-performers self") } func TestEventsPollPositionGoneCarriesTheResumeURLAndTheEpoch(t *testing.T) { @@ -555,6 +559,68 @@ func TestContinuationOriginComparisonNormalizesDefaultPortsAndCase(t *testing.T) require.Error(t, checkContinuation(feedBaseURL, "https://3.basecampapi.com:8443"+feedEventsPath)) } +// A position is bound to its filter set, so a resume breadcrumb that dropped +// the filters would hand back a command that 409s. +func TestResumeBreadcrumbCarriesTheFilters(t *testing.T) { + app, _, out := setupFeedApp(t, eventsRoute(http.StatusOK, feedPageJSON("pos-1", "", 11))) + app.Flags.Hints = true + + require.NoError(t, executeRecordingCommand(NewEventsCmd(), app, "poll", "--since", "now", + "--types", "message.created", "--buckets", "1,2", "--exclude-performers", "self")) + + assert.Equal(t, + "basecamp events poll --position <position> --types message.created"+ + " --buckets 1,2 --exclude-performers self", + resumeBreadcrumb(t, out)) +} + +func TestInboxResumeBreadcrumbCarriesItsOwnFilters(t *testing.T) { + app, _, out := setupFeedApp(t, inboxRoute(http.StatusOK, inboxPageJSON("inbox-pos-1", "", 991))) + app.Flags.Hints = true + + require.NoError(t, executeRecordingCommand(NewInboxCmd(), app, "--since", "now", "--reasons", "mentioned")) + + assert.Equal(t, "basecamp inbox --position <position> --reasons mentioned", resumeBreadcrumb(t, out)) +} + +// resumeBreadcrumb returns the command the response's resume breadcrumb +// offers, failing the test when there is none. +func resumeBreadcrumb(t *testing.T, out *bytes.Buffer) string { + t.Helper() + var envelope struct { + Breadcrumbs []struct { + Action string `json:"action"` + Cmd string `json:"cmd"` + } `json:"breadcrumbs"` + } + require.NoError(t, json.Unmarshal(out.Bytes(), &envelope)) + for _, crumb := range envelope.Breadcrumbs { + if crumb.Action == "resume" { + return crumb.Cmd + } + } + t.Fatal("no resume breadcrumb in the response") + return "" +} + +// A 410 body is server text. Its resume URL is displayed as authoritative and +// read for filters, which is acting on it, so it faces the same origin check +// the walk applies to a continuation — and the re-entry falls back to the +// filters this request was invoked with. +func TestPositionGoneRefusesAForeignResumeURL(t *testing.T) { + resume := "https://evil.example/99999/events.json?since=900&types=attacker.controlled" + app, _, _ := setupFeedApp(t, eventsRoute(http.StatusGone, + fmt.Sprintf(`{"error":"position predates the feed epoch","epoch_after_id":900,"resume":%q}`, resume))) + + err := executeRecordingCommand(NewEventsCmd(), app, "poll", "--position", "ancient", + "--types", "message.created") + + cliErr := requireFeedError(t, err) + assert.NotContains(t, cliErr.Message, "evil.example") + assert.NotContains(t, cliErr.Hint, "attacker.controlled") + assert.Equal(t, "Re-enter with: basecamp events poll --since 900 --types message.created", cliErr.Hint) +} + func TestCheckContinuationFallsBackToTheDefaultHost(t *testing.T) { require.NoError(t, checkContinuation("", feedBaseURL+feedEventsPath+"?position=p")) require.Error(t, checkContinuation("", "http://3.basecampapi.com/99999/events.json")) diff --git a/internal/commands/inbox.go b/internal/commands/inbox.go index 9246ee7d0..19aa7662d 100644 --- a/internal/commands/inbox.go +++ b/internal/commands/inbox.go @@ -78,6 +78,12 @@ deduplicate by addressing_id, never by the event's id.`, Buckets: bucketIDs, } + request := feedRequest{ + lane: inboxLane, + baseURL: app.Config.BaseURL, + filters: requestFilters(inboxFilterFlags(opts)), + } + items := make([]basecamp.InboxItem, 0) var position, next string pages, capped := 0, false @@ -86,7 +92,7 @@ deduplicate by addressing_id, never by the event's id.`, for { page, err := service.PollInbox(cmd.Context(), opts) if err != nil { - return feedError(inboxLane, err) + return feedError(request, err) } pages++ items = append(items, page.Items...) @@ -127,8 +133,8 @@ deduplicate by addressing_id, never by the event's id.`, output.WithBreadcrumbs( output.Breadcrumb{ Action: "resume", - Cmd: "basecamp inbox --position <position>", - Description: "Poll again from this page's position", + Cmd: "basecamp inbox --position <position>" + request.filters, + Description: "Poll again from this page's position, with the same filters", }, output.Breadcrumb{ Action: "show", diff --git a/skills/basecamp/SKILL.md b/skills/basecamp/SKILL.md index 70182cf58..aa30d41d7 100644 --- a/skills/basecamp/SKILL.md +++ b/skills/basecamp/SKILL.md @@ -205,7 +205,7 @@ basecamp <cmd> --page 1 # First page only, no auto-pagination ## Quick Reference -> **Note:** Most queries require project scope (via `--in <project>` or `.basecamp/config.json`). Cross-project exceptions: `basecamp reports assigned`, `basecamp assignments`, `basecamp reports overdue`, `basecamp reports schedule`, `basecamp recordings <type>`, `basecamp notifications`, `basecamp gauges list`. +> **Note:** Most queries require project scope (via `--in <project>` or `.basecamp/config.json`). Cross-project exceptions: `basecamp reports assigned`, `basecamp assignments`, `basecamp reports overdue`, `basecamp reports schedule`, `basecamp recordings <type>`, `basecamp notifications`, `basecamp gauges list`, `basecamp events poll`, `basecamp inbox`. > > Seven list commands also list account-wide: `basecamp todos list --all-projects --json`, and likewise `cards list`, `messages list`, `comments list`, `files list`, `forwards list`, and `checkins answers`. @@ -976,12 +976,21 @@ The feed is a notification lane, not an audit log: - An agent that acts on what it hears should pass `--exclude-performers self`, which drops its own performances without hiding other agents' activity. -Filters (`--types`, `--buckets`, `--creators`, `--performers`, -`--exclude-performers`, `--actor-types`; the inbox adds `--reasons`) are -comma-separated. Changing them invalidates a held position: the poll exits -`1` naming both filter digests, and the fix is to re-enter with `--since`. A -position that is no longer servable exits `2` and the hint names the -`--since` to re-enter with. +The two lanes have different filter sets, and they are not interchangeable: + +- `events poll`: `--types`, `--buckets`, `--creators`, `--performers`, + `--exclude-performers`, `--actor-types`. +- `inbox`: `--reasons`, `--types`, `--buckets`. The other four are not flags + here. + +Each takes a comma-separated list. These are the only narrowing available — +neither command takes `--in <project>`; narrow to a project with `--buckets`. + +A position is bound to the filter set it was minted for, so a resume has to +carry the same filters. Changing them exits `1` naming both filter digests, +and the fix is to re-enter with `--since`. A position that is no longer +servable exits `2`, and the hint carries the whole re-entry command, filters +included. `basecamp inbox` is served to agent principals only for now; other principals get exit `4`. `basecamp events ticket` redacts the ticket and its URL unless From cc2df292b93a8fe76dc3bb8d3d6febbfbc3c3c9d Mon Sep 17 00:00:00 2001 From: Jeremy Daer <jeremy@37signals.com> Date: Wed, 16 Sep 2026 06:11:41 -0700 Subject: [PATCH 07/29] Adopt the SDK's per-lane 410 and the 400's reason MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit basecamp/basecamp-sdk#912 splits what this code had to treat as one shape. Each lane's 410 is now its own type — the feed's names the epoch, the inbox's does not have one — so a lane can no longer read the other's recovery by accident. Each lane owns the arm that recognizes its own, and the epoch arrives as a plain int64 rather than a pointer that stood for "this is the inbox". The 400 now carries a reason, and the two cases it separates have opposite remedies: a malformed position wants a re-entry, a malformed filter wants the filters fixed and would be untouched by any position reset. Each gets the remedy its reason names. A 400 with no reason — an older server — still gets the server's message and no hint at all, because guessing between the two sends the caller to the wrong one. --- go.mod | 2 +- go.sum | 4 +- internal/commands/feed.go | 95 ++++++++++++++++++------ internal/commands/feed_test.go | 42 ++++++++++- internal/mcpserver/model/PROVENANCE.json | 4 +- internal/mcpserver/model/openapi.json | 56 +++++++++++--- internal/version/sdk-provenance.json | 6 +- 7 files changed, 167 insertions(+), 42 deletions(-) diff --git a/go.mod b/go.mod index b956437aa..51b6b0782 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( charm.land/bubbles/v2 v2.2.1 charm.land/bubbletea/v2 v2.0.9 charm.land/lipgloss/v2 v2.0.6 - github.com/basecamp/basecamp-sdk/go v0.18.1-0.20260916113525-e58b5823a8ca + github.com/basecamp/basecamp-sdk/go v0.18.1-0.20260916130353-a8046263835c 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 069fd05e9..208cf859d 100644 --- a/go.sum +++ b/go.sum @@ -87,8 +87,8 @@ github.com/aymanbagabas/go-udiff v0.4.1 h1:OEIrQ8maEeDBXQDoGCbbTTXYJMYRCRO1fnodZ github.com/aymanbagabas/go-udiff v0.4.1/go.mod h1:0L9PGwj20lrtmEMeyw4WKJ/TMyDtvAoK9bf2u/mNo3w= github.com/aymerick/douceur v0.2.0 h1:Mv+mAeH1Q+n9Fr+oyamOlAkUNPWPlA8PPGR0QAaYuPk= github.com/aymerick/douceur v0.2.0/go.mod h1:wlT5vV2O3h55X9m7iVYN0TBM0NH/MmbLnd30/FjWUq4= -github.com/basecamp/basecamp-sdk/go v0.18.1-0.20260916113525-e58b5823a8ca h1:blH50bacuga0dFf/YAddx+bRL4ovMgqWhz/9308Jozk= -github.com/basecamp/basecamp-sdk/go v0.18.1-0.20260916113525-e58b5823a8ca/go.mod h1:kIBDYwPMMD59PadNGxpH0YTQuI+blFPZ8MelGI0RK5Q= +github.com/basecamp/basecamp-sdk/go v0.18.1-0.20260916130353-a8046263835c h1:FJQx85IjtQkA2rLAHrO77op3VRA9b5U4S6qtdjSNdIU= +github.com/basecamp/basecamp-sdk/go v0.18.1-0.20260916130353-a8046263835c/go.mod h1:kIBDYwPMMD59PadNGxpH0YTQuI+blFPZ8MelGI0RK5Q= 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/feed.go b/internal/commands/feed.go index 7a05f8164..93cc0a667 100644 --- a/internal/commands/feed.go +++ b/internal/commands/feed.go @@ -43,16 +43,21 @@ const feedDefaultMaxPages = 50 // being servable. The feed re-enters at the epoch the 410 body names; the // inbox re-enters at since=0, the earliest item still retained. type feedLane struct { - name string - pollCmd string - sinceHint string - resumeSince string + name string + pollCmd string + sinceHint string // continuationFilters reads the filter flags a continuation or resume URL // carries, through the lane's own parser: the two lanes have different // filter dimensions and their URLs are never interchangeable. continuationFilters func(string) ([]flagValues, error) - forbiddenMsg string - forbiddenHint string + // positionGone recognizes this lane's 410 and answers with the --since + // that re-enters it and the server's resume URL. The SDK gives the two + // lanes distinct types precisely so one arm cannot silently handle the + // other's: the feed re-enters at the epoch, the inbox at the earliest + // retained item, and those recoveries are not interchangeable. + positionGone func(error) (since, resume string, ok bool) + forbiddenMsg string + forbiddenHint string } var ( @@ -61,18 +66,39 @@ var ( pollCmd: "basecamp events poll", sinceHint: "Pass an event id to start after, 'now' to enter at the present, or 0 to replay served history", continuationFilters: eventsContinuationFilters, + positionGone: eventsPositionGone, } inboxLane = feedLane{ name: "inbox item", pollCmd: "basecamp inbox", sinceHint: "Pass an inbox item id to start after, 'now' to enter at the present, or 0 for the earliest retained items", - resumeSince: basecamp.SinceEpoch, continuationFilters: inboxContinuationFilters, + positionGone: inboxPositionGone, forbiddenMsg: "The inbox is served to agent principals only", forbiddenHint: "Authenticate as an agent, or use 'basecamp events poll' for the account-wide feed", } ) +// eventsPositionGone reads the feed's 410, which names the epoch the feed can +// still serve from. +func eventsPositionGone(err error) (string, string, bool) { + var gone *basecamp.FeedPositionGoneError + if !errors.As(err, &gone) { + return "", "", false + } + return strconv.FormatInt(gone.EpochAfterID, 10), gone.Resume, true +} + +// inboxPositionGone reads the inbox's 410, which has no epoch: the position +// fell behind the retention window, and since=0 is the earliest retained item. +func inboxPositionGone(err error) (string, string, bool) { + var gone *basecamp.InboxPositionGoneError + if !errors.As(err, &gone) { + return "", "", false + } + return basecamp.SinceEpoch, gone.Resume, true +} + func eventsContinuationFilters(raw string) ([]flagValues, error) { opts, err := basecamp.PollEventsOptionsFromURL(raw) if err != nil { @@ -212,17 +238,13 @@ type feedRequest struct { // filter value outside the contract's alphabet falls back to the filters this // request was invoked with: a hint is meant to be pasted, and half-rendered // server text is not. -func (r feedRequest) resumeCommand(gone *basecamp.FeedPositionGoneError) string { - since := r.lane.resumeSince - if gone.EpochAfterID != nil { - since = strconv.FormatInt(*gone.EpochAfterID, 10) - } +func (r feedRequest) resumeCommand(since, resume string) string { if since == "" { since = basecamp.SinceNow } command := fmt.Sprintf("%s --since %s", r.lane.pollCmd, since) - if filters, ok := r.resumeFilters(gone.Resume); ok { + if filters, ok := r.resumeFilters(resume); ok { return command + filters } return command + r.filters @@ -341,12 +363,14 @@ func renderableFilterValue(value string) bool { // 409 → usage. The held position was minted for different filters; the fix // is the caller's, and both digests name which side is which. // 410 → not found. The position is no longer servable. Not the caller's -// mistake, so the message carries the lane's own re-entry point. +// mistake, so the message carries the lane's own re-entry point, read +// through the lane's own 410 type. // 403 → forbidden, with the lane's documented reason (the inbox is // agents-only for now). -// 400 → whatever the server said. A malformed position and a malformed -// filter have different fixes and the body says which; guessing -// between them would send the caller to the wrong one. +// 400 → the server's message, plus the remedy its reason names: re-enter +// for a malformed position, fix the filters for a malformed filter. +// A 400 carrying no reason gets no hint — the two remedies are +// opposite, and guessing sends the caller to the wrong one. func feedError(req feedRequest, err error) error { var mismatch *basecamp.FeedFilterMismatchError if errors.As(err, &mismatch) { @@ -362,21 +386,35 @@ func feedError(req feedRequest, err error) error { } } - var gone *basecamp.FeedPositionGoneError - if errors.As(err, &gone) { + if since, resume, ok := req.lane.positionGone(err); ok { message := fmt.Sprintf("Position is no longer servable for the %s feed", req.lane.name) - if gone.Resume != "" && req.followable(gone.Resume) { - message = fmt.Sprintf("%s. Resume URL: %s", message, gone.Resume) + if resume != "" && req.followable(resume) { + message = fmt.Sprintf("%s. Resume URL: %s", message, resume) } return &output.Error{ Code: output.CodeNotFound, Message: message, - Hint: "Re-enter with: " + req.resumeCommand(gone), + Hint: "Re-enter with: " + req.resumeCommand(since, resume), HTTPStatus: http.StatusGone, Cause: err, } } + var request *basecamp.FeedRequestError + if errors.As(err, &request) { + switch request.Reason { + case basecamp.FeedReasonInvalidPosition: + return feedRequestError(request, err, + fmt.Sprintf("Re-enter with: %s --since now%s", req.lane.pollCmd, req.filters)) + case basecamp.FeedReasonInvalidFilter: + return feedRequestError(request, err, + "Fix the filters and re-run; a position reset will not help") + } + // No reason: the server did not say which case this is, and the two + // have opposite remedies. Surface its message with no hint rather + // than send the caller to the wrong one. + } + var sdkErr *basecamp.Error if errors.As(err, &sdkErr) && sdkErr.HTTPStatus == http.StatusForbidden && req.lane.forbiddenMsg != "" { return &output.Error{ @@ -391,6 +429,19 @@ func feedError(req feedRequest, err error) error { return convertSDKError(err) } +// feedRequestError renders a 400 with the remedy its reason names, keeping +// the server's own message and the SDK's classification. +func feedRequestError(request *basecamp.FeedRequestError, cause error, hint string) error { + return &output.Error{ + Code: request.Err.Code, + Message: request.Err.Message, + Hint: hint, + HTTPStatus: request.Err.HTTPStatus, + Retryable: request.Err.Retryable, + Cause: cause, + } +} + // checkContinuation is the caller-side obligation the SDK's // Poll*OptionsFromURL documents: a continuation URL arrives in a server // response, so its origin is checked against the configured API base before diff --git a/internal/commands/feed_test.go b/internal/commands/feed_test.go index edc301e7e..83768180e 100644 --- a/internal/commands/feed_test.go +++ b/internal/commands/feed_test.go @@ -307,7 +307,10 @@ func TestEventsPollPositionGoneCarriesTheResumeURLAndTheEpoch(t *testing.T) { assert.Equal(t, "Re-enter with: basecamp events poll --since 900", cliErr.Hint) } -func TestEventsPollBadRequestKeepsTheServersMessage(t *testing.T) { +// A 400 carrying no reason is undifferentiated: a malformed position and a +// malformed filter have opposite remedies, so the server's message stands on +// its own rather than being decorated with a guess. +func TestEventsPollBadRequestWithoutAReasonKeepsTheServersMessageAndOffersNoRemedy(t *testing.T) { app, _, _ := setupFeedApp(t, eventsRoute(http.StatusBadRequest, `{"error":"unknown type: message.exploded"}`)) @@ -316,6 +319,43 @@ func TestEventsPollBadRequestKeepsTheServersMessage(t *testing.T) { cliErr := requireFeedError(t, err) assert.Contains(t, cliErr.Error(), "message.exploded") assert.Equal(t, http.StatusBadRequest, cliErr.HTTPStatus) + assert.Empty(t, cliErr.Hint) +} + +func TestEventsPollBadRequestNamesTheRemedyItsReasonImplies(t *testing.T) { + app, _, _ := setupFeedApp(t, eventsRoute(http.StatusBadRequest, + `{"error":"Unrecognized position.","reason":"invalid_position"}`)) + + err := executeRecordingCommand(NewEventsCmd(), app, "poll", "--position", "garbled", + "--types", "message.created") + + cliErr := requireFeedError(t, err) + assert.Equal(t, http.StatusBadRequest, cliErr.HTTPStatus) + assert.Equal(t, "Re-enter with: basecamp events poll --since now --types message.created", cliErr.Hint) +} + +func TestEventsPollBadFilterSaysAPositionResetWillNotHelp(t *testing.T) { + app, _, _ := setupFeedApp(t, eventsRoute(http.StatusBadRequest, + `{"error":"unknown type: message.exploded","reason":"invalid_filter"}`)) + + err := executeRecordingCommand(NewEventsCmd(), app, "poll", "--types", "message.exploded") + + cliErr := requireFeedError(t, err) + assert.Contains(t, cliErr.Hint, "a position reset will not help") +} + +// The two lanes' 410s are distinct SDK types on purpose. A feed 410 body +// reaching the inbox lane must not be read as the inbox's, and the other way +// round: the recoveries are not interchangeable. +func TestInboxDoesNotReadTheFeeds410(t *testing.T) { + app, _, _ := setupFeedApp(t, inboxRoute(http.StatusGone, + `{"error":"position predates the feed epoch","epoch_after_id":900,"resume":"`+ + feedBaseURL+feedEventsPath+`?since=900"}`)) + + err := executeRecordingCommand(NewInboxCmd(), app, "--position", "ancient") + + cliErr := requireFeedError(t, err) + assert.NotContains(t, cliErr.Hint, "--since 900") } func TestEventsPollRejectsUnservableFlagsBeforeAnyRequest(t *testing.T) { diff --git a/internal/mcpserver/model/PROVENANCE.json b/internal/mcpserver/model/PROVENANCE.json index 22751eba4..953da2197 100644 --- a/internal/mcpserver/model/PROVENANCE.json +++ b/internal/mcpserver/model/PROVENANCE.json @@ -1,7 +1,7 @@ { "source": "github.com/basecamp/basecamp-sdk", - "commit": "e58b5823a8ca79d42bb008b281679d8fcbebebfc", - "ref": "go/v0.18.0-50-ge58b5823", + "commit": "a8046263835c559be89e3b081a0428c131a4c663", + "ref": "go/v0.18.0-51-ga8046263", "files": ["behavior-model.json", "openapi.json"], "synced_by": "scripts/sync-mcp-model.sh", "patches": "binary-upload operations dropped (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/openapi.json b/internal/mcpserver/model/openapi.json index 0af60840a..8be3c444f 100644 --- a/internal/mcpserver/model/openapi.json +++ b/internal/mcpserver/model/openapi.json @@ -8377,7 +8377,7 @@ }, "/{accountId}/events.json": { "get": { - "description": "Poll the account event feed for events after a position (oldest first, strict event-id order, up to 100 per page).\n\n**Entry.** With neither `since` nor `position` the feed begins at the present\n(equivalent to `since=now`). `since=<event id>` starts after that id and\n`since=0` replays all served history back to the feed's epoch; `since` is a\nsigned 64-bit integer written in decimal, or the literal `now`. `position`\nresumes from a token a previous page issued \u2014 signed, opaque, bound to the\naccount and the filter set.\n\n**Pagination**: the body envelope, not the Link header. `position` is the\ndurable cursor (persist it only after processing the page's events); `next`\nis an absolute continuation URL present only while this walk has more to\nserve. Not wired into the generic Link paginator \u2014 see the section note.\n\n**Errors.** 400 for a malformed position (resume with `since=`) or a malformed\nfilter (the body names the filter; a position reset will not help) \u2014 both the\nflat `{error}` body. 409 (FeedFilterMismatchError) when the position was\nminted for a different filter set. 410 (FeedPositionGoneError) when the\nposition predates the feed's epoch; follow its `resume` URL.", + "description": "Poll the account event feed for events after a position (oldest first, strict event-id order, up to 100 per page).\n\n**Entry.** With neither `since` nor `position` the feed begins at the present\n(equivalent to `since=now`). `since=<event id>` starts after that id and\n`since=0` replays all served history back to the feed's epoch; `since` is a\nsigned 64-bit integer written in decimal, or the literal `now`. `position`\nresumes from a token a previous page issued \u2014 signed, opaque, bound to the\naccount and the filter set.\n\n**Pagination**: the body envelope, not the Link header. `position` is the\ndurable cursor (persist it only after processing the page's events); `next`\nis an absolute continuation URL present only while this walk has more to\nserve. Not wired into the generic Link paginator \u2014 see the section note.\n\n**Errors.** 400 (FeedRequestError) for a malformed position (resume with\n`since=`) or a malformed filter (fix the filters; a position reset will not\nhelp), told apart by its optional `reason` and undifferentiated when `reason`\nis absent. 409 (FeedFilterMismatchError) when the position was minted for a\ndifferent filter set. 410 (FeedPositionGoneError) when the position predates\nthe feed's epoch; its `resume` re-enters at the epoch.", "operationId": "PollEvents", "parameters": [ { @@ -8476,11 +8476,11 @@ } }, "400": { - "description": "BadRequestError 400 response", + "description": "FeedRequestError 400 response", "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/BadRequestErrorResponseContent" + "$ref": "#/components/schemas/FeedRequestErrorResponseContent" } } } @@ -9388,7 +9388,7 @@ }, "/{accountId}/inbox.json": { "get": { - "description": "Poll the authenticated agent's inbox for the items that addressed it (oldest first, strict item-id order); people receive 403.\n\nThe inbox is the low-noise \"someone addressed you\" lane as its own resource\nrather than a filter over the account feed. **Agents only for now**: any\nother principal receives 403.\n\nAn item is a first-class delivery with its own identity: one event can\naddress the same principal for several reasons, and each reason is its own\nitem. Deduplicate by `addressing_id`, never by event id. Items are never\nself-addressed, are kept for 30 days, and are dropped at read time when the\nevent is no longer readable.\n\n**Entry**: `since=0` replays the earliest retained items, `since=now` enters\nat the present, `position` resumes. Inbox positions are bound to the\naccount, the principal, and the filter set, and are never interchangeable\nwith feed positions.\n\n**Pagination**: the body envelope (`items`, `position`, `next`), exactly as\nPollEvents \u2014 not the Link header, and not the generic paginator.\n\n**Errors** follow PollEvents, except that 403 carries no body \u2014 the agent\nguard's bare `head :forbidden` (BareForbiddenError) \u2014 and that 410\n(FeedPositionGoneError) here means the position fell behind the retention\nwindow: `epoch_after_id` is absent and `resume` re-enters at `since=0`,\nthe earliest retained item.", + "description": "Poll the authenticated agent's inbox for the items that addressed it (oldest first, strict item-id order); people receive 403.\n\nThe inbox is the low-noise \"someone addressed you\" lane as its own resource\nrather than a filter over the account feed. **Agents only for now**: any\nother principal receives 403.\n\nAn item is a first-class delivery with its own identity: one event can\naddress the same principal for several reasons, and each reason is its own\nitem. Deduplicate by `addressing_id`, never by event id. Items are never\nself-addressed, are kept for 30 days, and are dropped at read time when the\nevent is no longer readable.\n\n**Entry**: `since=0` replays the earliest retained items, `since=now` enters\nat the present, `position` resumes. Inbox positions are bound to the\naccount, the principal, and the filter set, and are never interchangeable\nwith feed positions.\n\n**Pagination**: the body envelope (`items`, `position`, `next`), exactly as\nPollEvents \u2014 not the Link header, and not the generic paginator.\n\n**Errors** follow PollEvents (FeedRequestError 400, FeedFilterMismatchError\n409), except that 403 carries no body \u2014 the agent guard's bare\n`head :forbidden` (BareForbiddenError) \u2014 and that the 410 is the inbox's own\nInboxPositionGoneError: the position fell behind the retention window, there\nis no epoch, and `resume` re-enters at `since=0`, the earliest retained item\n\u2014 not the feed's recovery, and not interchangeable with it.", "operationId": "PollInbox", "parameters": [ { @@ -9460,11 +9460,11 @@ } }, "400": { - "description": "BadRequestError 400 response", + "description": "FeedRequestError 400 response", "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/BadRequestErrorResponseContent" + "$ref": "#/components/schemas/FeedRequestErrorResponseContent" } } } @@ -9493,11 +9493,11 @@ } }, "410": { - "description": "FeedPositionGoneError 410 response", + "description": "InboxPositionGoneError 410 response", "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/FeedPositionGoneErrorResponseContent" + "$ref": "#/components/schemas/InboxPositionGoneErrorResponseContent" } } } @@ -31393,26 +31393,43 @@ }, "FeedPositionGoneErrorResponseContent": { "type": "object", - "description": "410 from the event feed's poll lanes: the held `position` predates what the\nlane can still serve. On PollEvents that is the feed's epoch \u2014 an operational\nfence that can be raised \u2014 and `epoch_after_id` names it; on PollInbox it is\nthe inbox's 30-day retention window, and `epoch_after_id` is absent. Either\nway `resume` is an absolute URL that re-enters the same lane with the\nrequest's canonical filters preserved: at the epoch (`since=<epoch_after_id>`)\nfor the feed, at the earliest retained item (`since=0`) for the inbox.\nConsumers validate the URL (same origin as the API base, no scheme downgrade)\nbefore following it \u2014 SPEC.md \u00a723 \"Continuation and Resume URL Validation\".", + "description": "410 from PollEvents: the held `position` predates the feed's epoch \u2014 an\noperational fence that can be raised. `epoch_after_id` names the epoch and\n`resume` is an absolute URL that re-enters the feed AT THE EPOCH\n(`since=<epoch_after_id>`) with the request's canonical filters preserved,\nso the servable history above the fence is not skipped. Not interchangeable\nwith the inbox's InboxPositionGoneError, whose recovery re-enters at\n`since=0`. Consumers validate the URL (same origin as the API base, no scheme\ndowngrade) before following it \u2014 SPEC.md \u00a723 \"Continuation and Resume URL\nValidation\".", "properties": { "error": { "type": "string" }, "epoch_after_id": { "type": "integer", - "description": "The feed's epoch: the event id after which history is servable. Feed only.", + "description": "The feed's epoch: the event id after which history is servable.", "format": "int64" }, "resume": { "type": "string", - "description": "Absolute re-entry URL for the same lane, filters preserved." + "description": "Absolute re-entry URL for the feed, at the epoch, filters preserved." } }, "required": [ + "epoch_after_id", "error", "resume" ] }, + "FeedRequestErrorResponseContent": { + "type": "object", + "description": "400 from the event feed's poll lanes (PollEvents, PollInbox). Two distinct\ncases share the status: a malformed `position` (recover by re-entering with\n`since=`) and a malformed filter (fix the filters; a position reset will not\nhelp). `reason` tells them apart \u2014 `invalid_position` or `invalid_filter`\n(bc3 #13362) \u2014 and is OPTIONAL: a server that has not shipped it answers the\nflat `{error}` body, and a consumer that finds `reason` absent must treat the\n400 as undifferentiated and surface it rather than guess between recovering\nand stopping.", + "properties": { + "error": { + "type": "string" + }, + "reason": { + "type": "string", + "description": "`invalid_position` (re-enter with `since=`) or `invalid_filter` (fix the\nfilters). Absent from servers that predate bc3 #13362: then the 400 is\nundifferentiated." + } + }, + "required": [ + "error" + ] + }, "FieldErrorMap": { "type": "object", "additionalProperties": { @@ -32836,6 +32853,23 @@ "reason" ] }, + "InboxPositionGoneErrorResponseContent": { + "type": "object", + "description": "410 from PollInbox: the held `position` fell behind the inbox's 30-day\nretention window. There is no epoch; `resume` is an absolute URL that\nre-enters the inbox at `since=0`, the earliest retained item \u2014 exactly-once\ncontinuation, since a stale position has seen none of the retained backlog.\nA distinct shape from the feed's FeedPositionGoneError on purpose: the two\nrecoveries are not interchangeable, and a consumer must not handle one lane's\n410 with the other's arm. Validate the URL before following it, as for the\nfeed.", + "properties": { + "error": { + "type": "string" + }, + "resume": { + "type": "string", + "description": "Absolute re-entry URL for the inbox, at `since=0`, filters preserved." + } + }, + "required": [ + "error", + "resume" + ] + }, "InternalServerErrorResponseContent": { "type": "object", "properties": { diff --git a/internal/version/sdk-provenance.json b/internal/version/sdk-provenance.json index 3ed0615f5..fa44c345d 100644 --- a/internal/version/sdk-provenance.json +++ b/internal/version/sdk-provenance.json @@ -1,9 +1,9 @@ { "sdk": { "module": "github.com/basecamp/basecamp-sdk/go", - "version": "v0.18.1-0.20260916113525-e58b5823a8ca", - "revision": "e58b5823a8ca", - "updated_at": "2026-09-16T11:35:25Z" + "version": "v0.18.1-0.20260916130353-a8046263835c", + "revision": "a8046263835c", + "updated_at": "2026-09-16T13:03:53Z" }, "api": { "repo": "basecamp/bc3", From 055f87a4cf8af7b036dbae877eb06f58e43df68c Mon Sep 17 00:00:00 2001 From: Jeremy Daer <jeremy@37signals.com> Date: Wed, 16 Sep 2026 06:16:37 -0700 Subject: [PATCH 08/29] Keep the stream-ticket mint off the MCP surface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The mint returns a replayable bearer and a URL that embeds it, and the MCP dispatcher hands a result straight into a model transcript with no equivalent of the CLI's --show-secret gate. Nothing on that surface could use the ticket anyway: opening the stream needs a WebSocket, which a JSON tool call is not. The sync script's existing exclusion list is held to a "still not JSON" check — it exists for bodies the toolkit refuses to load, and that check is worth keeping — so the policy exclusion is its own list, named for the reason it exists. The skill said these commands do not take --in. They do take it, as a global flag, and ignore it; saying otherwise would have an agent expect Cobra to reject it. --- API-COVERAGE.md | 2 +- internal/mcpserver/catalog_test.go | 15 ++-- internal/mcpserver/domains.go | 2 +- internal/mcpserver/model/PROVENANCE.json | 2 +- internal/mcpserver/model/behavior-model.json | 12 --- internal/mcpserver/model/openapi.json | 86 ------------------- .../mcpserver/testdata/catalog_snapshot.txt | 3 +- scripts/sync-mcp-model.sh | 29 +++++-- skills/basecamp/SKILL.md | 5 +- 9 files changed, 41 insertions(+), 115 deletions(-) diff --git a/API-COVERAGE.md b/API-COVERAGE.md index 3d6397a0e..00d4e5598 100644 --- a/API-COVERAGE.md +++ b/API-COVERAGE.md @@ -232,7 +232,7 @@ cannot faithfully cover at least one endpoint for a reason outside the CLI. A | schedule_entries | 5 | `schedule` | ✅ | BC4 | - | list, show, create, update, occurrences. Create supports `--subscribe`/`--no-subscribe` | | events | 1 | `events` | ✅ | BC4 | - | Recording change audit trail | | **Event Feed** | -| event_feed | 3 | `events poll`, `events ticket`, `inbox` | ✅ | BC5 | - | The account-wide event feed: `GET /events.json` (`events poll`), `GET /inbox.json` (`inbox`, agent principals only), `POST /events/stream_ticket.json` (`events ticket`). Pagination is the body envelope — a durable `position` and a `next` continuation URL — not the Link-header page walk, so `--all` walks `next` rather than page numbers. The live WebSocket lane the ticket opens is out of scope for the CLI: the SDK's cable client carries a WebSocket dependency the CLI does not take | +| event_feed | 3 | `events poll`, `events ticket`, `inbox` | ✅ | BC5 | - | The account-wide event feed: `GET /events.json` (`events poll`), `GET /inbox.json` (`inbox`, agent principals only), `POST /events/stream_ticket.json` (`events ticket`). Pagination is the body envelope — a durable `position` and a `next` continuation URL — not the Link-header page walk, so `--all` walks `next` rather than page numbers. The live WebSocket lane the ticket opens is out of scope for the CLI: the SDK's cable client carries a WebSocket dependency the CLI does not take. The mint is a CLI command only — it is excluded from the MCP surface, whose dispatcher would return the bearer verbatim into a model transcript and which could not open the socket anyway | | **Webhooks** | | webhooks | 7 | `webhooks` | ✅ | BC4 | - | list, show, create, update, delete | | **Templates** | diff --git a/internal/mcpserver/catalog_test.go b/internal/mcpserver/catalog_test.go index 7dc07ce01..ee8b89420 100644 --- a/internal/mcpserver/catalog_test.go +++ b/internal/mcpserver/catalog_test.go @@ -45,15 +45,20 @@ func TestCatalogClaimsEveryTag(t *testing.T) { assert.Empty(t, cat.Unmapped, "every SDK tag must be claimed by a DomainSpec") } -// TestCatalogExcludesBinaryUploads pins the operations the sync script drops -// from the vendored model: raw-binary uploads can't ride the JSON tool-call -// convention. Uploads stay a CLI affair (basecamp attach / upload). -func TestCatalogExcludesBinaryUploads(t *testing.T) { +// TestCatalogExcludesUnservedOperations pins the operations the sync script +// drops from the vendored model. Raw-binary uploads can't ride the JSON +// tool-call convention, so uploads stay a CLI affair (basecamp attach / +// upload). The stream-ticket mint could ride it and is dropped anyway: its +// result is a replayable bearer and a URL embedding it, the dispatcher +// returns a result verbatim into a model transcript with no reveal opt-in, +// and nothing on this surface could open the WebSocket it is for. +func TestCatalogExcludesUnservedOperations(t *testing.T) { cat := loadForTest(t) excluded := map[string]bool{ "CreateAttachment": true, "CreateCampfireUpload": true, "UpdateAccountLogo": true, + "CreateStreamTicket": true, } model, composite := 0, 0 for _, d := range cat.Domains { @@ -66,7 +71,7 @@ func TestCatalogExcludesBinaryUploads(t *testing.T) { assert.False(t, excluded[op.ID], "operation %q should be excluded from the vendored model", op.ID) } } - assert.Equal(t, 262, model, "served model operation count") + assert.Equal(t, 261, model, "served model operation count") assert.Equal(t, 1, composite, "served composite operation count") } diff --git a/internal/mcpserver/domains.go b/internal/mcpserver/domains.go index cd69490e7..347849a77 100644 --- a/internal/mcpserver/domains.go +++ b/internal/mcpserver/domains.go @@ -82,7 +82,7 @@ var DomainSpecs = []catalog.DomainSpec{ { Key: "eventfeed", Tags: []string{"EventFeed"}, - Blurb: "The account event feed: poll events from a resumable position, poll the agent inbox of addressed items, and mint a ticket for the live stream. Pages carry a position and a continuation URL rather than page numbers; deduplicate events by id and inbox items by addressing_id, and refetch the referenced recording before acting on it.", + Blurb: "The account event feed: poll events from a resumable position, and poll the agent inbox of addressed items. Pages carry a position and a continuation URL rather than page numbers; deduplicate events by id and inbox items by addressing_id, and refetch the referenced recording before acting on it. Minting a live-stream ticket is not served here — the ticket is a bearer this surface cannot use, and 'basecamp events ticket' is where it lives.", }, { Key: "clientside", diff --git a/internal/mcpserver/model/PROVENANCE.json b/internal/mcpserver/model/PROVENANCE.json index 953da2197..5f7c5d4c6 100644 --- a/internal/mcpserver/model/PROVENANCE.json +++ b/internal/mcpserver/model/PROVENANCE.json @@ -4,5 +4,5 @@ "ref": "go/v0.18.0-51-ga8046263", "files": ["behavior-model.json", "openapi.json"], "synced_by": "scripts/sync-mcp-model.sh", - "patches": "binary-upload operations dropped (EXCLUDED_OPERATIONS); no tag patches applied (PATCHED_TAGS is empty — the export tags every operation) — see the sync script" + "patches": "binary-upload operations and the stream-ticket mint dropped (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 92ad368f4..f1b4727c5 100644 --- a/internal/mcpserver/model/behavior-model.json +++ b/internal/mcpserver/model/behavior-model.json @@ -294,18 +294,6 @@ ] } }, - "CreateStreamTicket": { - "idempotent": true, - "retry": { - "max": 3, - "base_delay_ms": 1000, - "backoff": "exponential", - "retry_on": [ - 429, - 503 - ] - } - }, "CreateTemplate": { "retry": { "max": 2, diff --git a/internal/mcpserver/model/openapi.json b/internal/mcpserver/model/openapi.json index 8be3c444f..0499916a3 100644 --- a/internal/mcpserver/model/openapi.json +++ b/internal/mcpserver/model/openapi.json @@ -8560,92 +8560,6 @@ } } }, - "/{accountId}/events/stream_ticket.json": { - "post": { - "description": "Mint a short-lived stream ticket and the exact WebSocket URL to open a live event stream with.\n\nThe ticket is signed and lives about two minutes; the response's `url` is\nthe one to connect to. Connect to `url` verbatim \u2014 never assemble the WebSocket URL (scheme, host,\npath, account prefix) client-side; the topology is the server's to change.\nMint a fresh ticket for every connection attempt: tickets expire and are not\nrefreshed by an open socket. The mint is a stateless signed capability with\nno server-side consumption, so a replayed POST is harmless and the operation\nis marked idempotent (safe to retry) \u2014 deliberately not a claim that two\nmints return the same ticket. The ticket is a replayable bearer credential\nwithin its window; `ticket` and `url` are marked sensitive (see StreamTicket).\n\nServes agent principals as well as people: an agent's client-credentials\ntoken can mint tickets for its own live stream. A ticket minted on a\ndelegated request carries its agent, so `self` on the socket it opens\nresolves to that agent.", - "operationId": "CreateStreamTicket", - "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": "CreateStreamTicket 200 response", - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/CreateStreamTicketResponseContent" - } - } - } - }, - "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": [ - "EventFeed" - ], - "x-basecamp-idempotent": { - "natural": true - }, - "x-basecamp-retry": { - "maxAttempts": 3, - "baseDelayMs": 1000, - "backoff": "exponential", - "retryOn": [ - 429, - 503 - ] - } - } - }, "/{accountId}/files.json": { "get": { "description": "Get every file recording across all accessible projects, newest-first (paginated).\nHeterogeneous: uploads and Basecamp documents carry their\nstandard recording shapes, while rich-text attachments are wrapped in a\nrecording envelope plus an `attachable_sgid` and blob metadata. Modeled as\nan optional-field superset (EverythingFile) so one element type decodes any\nvariant.", diff --git a/internal/mcpserver/testdata/catalog_snapshot.txt b/internal/mcpserver/testdata/catalog_snapshot.txt index 82eea3a47..f76b22ad3 100644 --- a/internal/mcpserver/testdata/catalog_snapshot.txt +++ b/internal/mcpserver/testdata/catalog_snapshot.txt @@ -314,13 +314,12 @@ ACTIONS (RO = read-only): - get_everything_unassigned_todos (RO, paginated): Open, unassigned to-dos across all accessible projects, grouped by project (paginated) == basecamp_eventfeed -The account event feed: poll events from a resumable position, poll the agent inbox of addressed items, and mint a ticket for the live stream. Pages carry a position and a continuation URL rather than page numbers; deduplicate events by id and inbox items by addressing_id, and refetch the referenced recording before acting on it. +The account event feed: poll events from a resumable position, and poll the agent inbox of addressed items. Pages carry a position and a continuation URL rather than page numbers; deduplicate events by id and inbox items by addressing_id, and refetch the referenced recording before acting on it. Minting a live-stream ticket is not served here — the ticket is a bearer this surface cannot use, and 'basecamp events ticket' is where it lives. Gateway tool: call with {"action": "...", "params": {...}}. Call {"action": "describe", "params": {"action": "NAME"}} for an action's full parameter schema. ACTIONS (RO = read-only): -- create_stream_ticket: Mint a short-lived stream ticket and the exact WebSocket URL to open a live event stream with - poll_events (RO): Poll the account event feed for events after a position (oldest first, strict event-id order, up to 100 per page) - poll_inbox (RO): Poll the authenticated agent's inbox for the items that addressed it (oldest first, strict item-id order); people receive 403 diff --git a/scripts/sync-mcp-model.sh b/scripts/sync-mcp-model.sh index 976747f0c..be71d3dd2 100755 --- a/scripts/sync-mcp-model.sh +++ b/scripts/sync-mcp-model.sh @@ -23,6 +23,14 @@ # bodies at load, so they are dropped from both model files here. Uploads # stay a CLI affair (basecamp attach / upload). # +# CreateStreamTicket is excluded for the opposite reason (POLICY_EXCLUDED_ +# OPERATIONS, which is not held to the "still not JSON" check): it rides the +# convention fine, but its result is a replayable bearer and a URL that embeds +# it, and the dispatcher returns a result verbatim into a model transcript +# with no reveal opt-in. Nothing on this surface could use the ticket anyway — +# opening the stream needs a WebSocket, which a JSON tool call is not — so the +# mint stays a CLI affair (basecamp events ticket --show-secret). +# # Usage: scripts/sync-mcp-model.sh [path-to-basecamp-sdk-checkout] set -eu @@ -54,6 +62,13 @@ EXCLUDED_OPERATIONS = { "UpdateAccountLogo", } +# Excluded by policy rather than by body shape, so these are not held to the +# "still not JSON" check above: they would ride the convention, we decline to +# serve them. +POLICY_EXCLUDED_OPERATIONS = { + "CreateStreamTicket", +} + PATCHED_TAGS: dict[str, str] = { # Empty: basecamp-sdk now tags every exported operation itself # (basecamp-sdk#878). Kept as a seam, because the export has left @@ -80,6 +95,10 @@ for path in list(doc["paths"]): del methods[method] excluded.append(oid) continue + if oid in POLICY_EXCLUDED_OPERATIONS: + del methods[method] + excluded.append(oid) + continue tags = op.get("tags") or [] if oid in PATCHED_TAGS: if tags: @@ -94,9 +113,9 @@ for path in list(doc["paths"]): missing = sorted(set(PATCHED_TAGS) - set(patched)) if missing: sys.exit(f"PATCHED_TAGS entries not found in the export: {missing}") -missing = sorted(set(EXCLUDED_OPERATIONS) - set(excluded)) +missing = sorted((EXCLUDED_OPERATIONS | POLICY_EXCLUDED_OPERATIONS) - set(excluded)) if missing: - sys.exit(f"EXCLUDED_OPERATIONS entries not found in the export: {missing}") + sys.exit(f"excluded operations not found in the export: {missing}") with open(f"{dest}/openapi.json", "w") as f: json.dump(doc, f, indent=2) @@ -106,13 +125,13 @@ with open(f"{dest}/openapi.json", "w") as f: # behavior model too. with open(f"{sdk}/behavior-model.json") as f: bm = json.load(f) -for oid in EXCLUDED_OPERATIONS: +for oid in EXCLUDED_OPERATIONS | POLICY_EXCLUDED_OPERATIONS: bm["operations"].pop(oid, None) with open(f"{dest}/behavior-model.json", "w") as f: json.dump(bm, f, indent=2) f.write("\n") -print(f"patched tags onto {len(patched)} untagged operations; excluded {len(excluded)} binary-upload operations") +print(f"patched tags onto {len(patched)} untagged operations; excluded {len(excluded)} operations") PY cat > "$dest/PROVENANCE.json" <<JSON @@ -122,7 +141,7 @@ cat > "$dest/PROVENANCE.json" <<JSON "ref": "$ref", "files": ["behavior-model.json", "openapi.json"], "synced_by": "scripts/sync-mcp-model.sh", - "patches": "binary-upload operations dropped (EXCLUDED_OPERATIONS); no tag patches applied (PATCHED_TAGS is empty — the export tags every operation) — see the sync script" + "patches": "binary-upload operations and the stream-ticket mint dropped (EXCLUDED_OPERATIONS); no tag patches applied (PATCHED_TAGS is empty — the export tags every operation) — see the sync script" } JSON diff --git a/skills/basecamp/SKILL.md b/skills/basecamp/SKILL.md index aa30d41d7..3c6b63d21 100644 --- a/skills/basecamp/SKILL.md +++ b/skills/basecamp/SKILL.md @@ -983,8 +983,9 @@ The two lanes have different filter sets, and they are not interchangeable: - `inbox`: `--reasons`, `--types`, `--buckets`. The other four are not flags here. -Each takes a comma-separated list. These are the only narrowing available — -neither command takes `--in <project>`; narrow to a project with `--buckets`. +Each takes a comma-separated list. These are the only narrowing available: +both commands accept the global `--in <project>` flag but ignore it, because +these are account endpoints. Narrow to a project with `--buckets`. A position is bound to the filter set it was minted for, so a resume has to carry the same filters. Changing them exits `1` naming both filter digests, From ffab993bbb74998c1cbdae93e54b2925546cf67c Mon Sep 17 00:00:00 2001 From: Jeremy Daer <jeremy@37signals.com> Date: Wed, 16 Sep 2026 06:25:11 -0700 Subject: [PATCH 09/29] Hand the position back to the person, not just the machine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rendered output is the page's rows, and the generic renderer drops a field named "position" from objects anyway — on a to-do or a card that name means an ordering integer. So someone polling without --json had no way to persist or resume their cursor. The notice carries the whole resume command now, filters included, in every format and whether or not hints are on. A walk that stopped at its page cap says both things in one sentence: more remains, and here is where to continue from. The resume breadcrumb renders the same command rather than a <position> placeholder it could not fill. --- internal/commands/feed.go | 53 +++++++++++++++++++++------------- internal/commands/feed_test.go | 46 +++++++++++++++++++++++++++-- internal/commands/inbox.go | 9 ++---- skills/basecamp/SKILL.md | 5 ++-- 4 files changed, 82 insertions(+), 31 deletions(-) diff --git a/internal/commands/feed.go b/internal/commands/feed.go index 93cc0a667..d7cb74f3f 100644 --- a/internal/commands/feed.go +++ b/internal/commands/feed.go @@ -473,13 +473,31 @@ func checkContinuation(baseURL, raw string) error { return nil } -// feedWalkNotice reports a walk that stopped at its page cap rather than at -// the end of the walk, so a caller reads "more remains" instead of "done". -func feedWalkNotice(capped bool, maxPages int) string { - if !capped { - return "" - } - return fmt.Sprintf("Stopped at --max-pages %d; more remains. Poll again from the position above.", maxPages) +// resumeNotice is where a person gets their position back. +// +// The envelope carries it for machines, but the rendered output is the rows — +// and the generic renderer drops a field named "position" from objects +// anyway, since on a to-do or a card that name means an ordering integer. So +// the notice, which every format prints verbatim, carries the whole resume +// command. A walk that stopped at its page cap says so in the same sentence, +// because "more remains" and "here is where to continue" are one thought. +func (r feedRequest) resumeNotice(position string, capped bool, maxPages int) string { + notice := "" + if capped { + notice = fmt.Sprintf("Stopped at --max-pages %d; more remains. ", maxPages) + } + if position == "" { + return strings.TrimSpace(notice) + } + return notice + "Resume from here with: " + r.positionCommand(position) +} + +// positionCommand is the invocation that resumes from a position. The filters +// ride along because a position is bound to the set it was minted for: the +// same command without them is a different filter set, and the server answers +// 409. +func (r feedRequest) positionCommand(position string) string { + return fmt.Sprintf("%s --position %s%s", r.lane.pollCmd, position, r.filters) } // newEventsPollCmd builds `basecamp events poll`. @@ -616,18 +634,17 @@ event id and refetch the referenced recording before acting on it.`, respOpts := []output.ResponseOption{ output.WithSummary(summary), - // The envelope is the machine contract, but it is one object, - // so --count would answer 1 for any page and --ids-only would - // find no id to print. The rows are what those modes are - // about; --json keeps the resumable envelope. + output.WithNotice(request.resumeNotice(position, capped, maxPages)), + // The envelope is one object: rendered for a person it is a + // Go dump, --count answers 1 for any page, and --ids-only + // finds no id to print. The rows are what all three want. + // The position a person needs to resume comes back in the + // notice below, since it lives in the envelope, not the rows. output.WithDisplayData(events), output.WithBreadcrumbs( output.Breadcrumb{ - Action: "resume", - // A position is bound to its filter set, so the - // resume command carries the filters this poll used; - // without them the server answers 409. - Cmd: "basecamp events poll --position <position>" + request.filters, + Action: "resume", + Cmd: request.positionCommand(position), Description: "Poll again from this page's position, with the same filters", }, output.Breadcrumb{ @@ -637,10 +654,6 @@ event id and refetch the referenced recording before acting on it.`, }, ), } - if notice := feedWalkNotice(capped, maxPages); notice != "" { - respOpts = append(respOpts, output.WithNotice(notice)) - } - return app.OK(map[string]any{ "events": events, "position": position, diff --git a/internal/commands/feed_test.go b/internal/commands/feed_test.go index 83768180e..7dd740852 100644 --- a/internal/commands/feed_test.go +++ b/internal/commands/feed_test.go @@ -231,7 +231,7 @@ func TestEventsPollAllFollowsNextUntilItIsAbsent(t *testing.T) { assert.Len(t, data.Events, 3) assert.Equal(t, "pos-2", data.Position) assert.Empty(t, data.Next) - assert.Empty(t, envelope.Notice) + assert.NotContains(t, envelope.Notice, "--max-pages") queries := transport.queriesFor(feedEventsPath) require.Len(t, queries, 2) @@ -574,6 +574,46 @@ func TestPositionGoneFallsBackWhenTheResumeURLIsNotRenderable(t *testing.T) { assert.Equal(t, "Re-enter with: basecamp events poll --since 900", requireFeedError(t, err).Hint) } +// The rendered output is the rows, and the generic renderer drops a field +// named "position" from objects anyway, so a person polling without --json +// would have no way to resume. The notice carries the whole command, in every +// format, hints on or off. +func TestHumanOutputCarriesTheResumeCommand(t *testing.T) { + app, _, out := setupFeedApp(t, eventsRoute(http.StatusOK, feedPageJSON("pos-1", "", 11))) + app.Output = output.New(output.Options{Format: output.FormatMarkdown, Writer: out}) + + require.NoError(t, executeRecordingCommand(NewEventsCmd(), app, "poll", "--since", "now", + "--types", "message.created")) + + assert.Contains(t, out.String(), + "Resume from here with: basecamp events poll --position pos-1 --types message.created") +} + +func TestInboxHumanOutputCarriesTheResumeCommand(t *testing.T) { + app, _, out := setupFeedApp(t, inboxRoute(http.StatusOK, inboxPageJSON("inbox-pos-1", "", 991))) + app.Output = output.New(output.Options{Format: output.FormatMarkdown, Writer: out}) + + require.NoError(t, executeRecordingCommand(NewInboxCmd(), app, "--since", "now")) + + assert.Contains(t, out.String(), "Resume from here with: basecamp inbox --position inbox-pos-1") +} + +// A capped walk says both things in one notice: more remains, and here is +// where to continue from. +func TestACappedWalkSaysSoAndStillHandsBackThePosition(t *testing.T) { + next := feedBaseURL + feedEventsPath + "?position=pos-1" + app, _, out := setupFeedApp(t, eventsRoute(http.StatusOK, + feedPageJSON("pos-1", next, 11), + feedPageJSON("pos-2", next, 12), + )) + + require.NoError(t, executeRecordingCommand(NewEventsCmd(), app, "poll", "--since", "0", "--all", "--max-pages", "1")) + + assert.Equal(t, + "Stopped at --max-pages 1; more remains. Resume from here with: basecamp events poll --position pos-1", + decodeFeedEnvelope(t, out).Notice) +} + // --count counts the page's rows and --ids-only prints their identities; for // the inbox that identity is the addressing id, never the event id. func TestEnumeratingOutputModesSeeTheRows(t *testing.T) { @@ -609,7 +649,7 @@ func TestResumeBreadcrumbCarriesTheFilters(t *testing.T) { "--types", "message.created", "--buckets", "1,2", "--exclude-performers", "self")) assert.Equal(t, - "basecamp events poll --position <position> --types message.created"+ + "basecamp events poll --position pos-1 --types message.created"+ " --buckets 1,2 --exclude-performers self", resumeBreadcrumb(t, out)) } @@ -620,7 +660,7 @@ func TestInboxResumeBreadcrumbCarriesItsOwnFilters(t *testing.T) { require.NoError(t, executeRecordingCommand(NewInboxCmd(), app, "--since", "now", "--reasons", "mentioned")) - assert.Equal(t, "basecamp inbox --position <position> --reasons mentioned", resumeBreadcrumb(t, out)) + assert.Equal(t, "basecamp inbox --position inbox-pos-1 --reasons mentioned", resumeBreadcrumb(t, out)) } // resumeBreadcrumb returns the command the response's resume breadcrumb diff --git a/internal/commands/inbox.go b/internal/commands/inbox.go index 19aa7662d..1f2a823ab 100644 --- a/internal/commands/inbox.go +++ b/internal/commands/inbox.go @@ -123,9 +123,10 @@ deduplicate by addressing_id, never by the event's id.`, respOpts := []output.ResponseOption{ output.WithSummary(summary), + output.WithNotice(request.resumeNotice(position, capped, maxPages)), // Rows for the enumerating output modes, as in events poll — // and an item's identity here is its addressing id, so that is - // what the row's id is. The machine envelope keeps + // what the row's id is. The rendered envelope keeps // addressing_id under its own name, because applying the // feed's dedupe-by-event-id rule to the inbox would discard // every reason but one. @@ -133,7 +134,7 @@ deduplicate by addressing_id, never by the event's id.`, output.WithBreadcrumbs( output.Breadcrumb{ Action: "resume", - Cmd: "basecamp inbox --position <position>" + request.filters, + Cmd: request.positionCommand(position), Description: "Poll again from this page's position, with the same filters", }, output.Breadcrumb{ @@ -143,10 +144,6 @@ deduplicate by addressing_id, never by the event's id.`, }, ), } - if notice := feedWalkNotice(capped, maxPages); notice != "" { - respOpts = append(respOpts, output.WithNotice(notice)) - } - return app.OK(map[string]any{ "items": items, "position": position, diff --git a/skills/basecamp/SKILL.md b/skills/basecamp/SKILL.md index 3c6b63d21..6a2765916 100644 --- a/skills/basecamp/SKILL.md +++ b/skills/basecamp/SKILL.md @@ -962,8 +962,9 @@ basecamp events ticket --json # Mint a live-stream ticket Each page is an envelope: `events` (or `items`), a durable `position`, and a `next` continuation URL while the current walk has more to serve. Persist `position` only after processing the page, then pass it back with -`--position`. `--all` walks `next` to the end of the current walk — it is not -a live tail. +`--position`. The response's `notice` spells the whole resume command out, +filters included. `--all` walks `next` to the end of the current walk — it is +not a live tail. The feed is a notification lane, not an audit log: From 2becc615c2de46367a1e2ddffcd439bb0c867cc5 Mon Sep 17 00:00:00 2001 From: Jeremy Daer <jeremy@37signals.com> Date: Wed, 16 Sep 2026 06:34:17 -0700 Subject: [PATCH 10/29] Quote the position in the command it hands back MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A position is opaque server text going into a command somebody pastes, and nothing about it promises to be a bare word. It goes through this package's shellQuote, as every other emitted command's interpolated value does. The filter values beside it are not quoted because they are checked against srv2's alphabet first, and a value outside it drops the whole suffix rather than being rendered — a whitelist, where the position can only have a blacklist. --- internal/commands/feed.go | 6 ++++-- internal/commands/feed_test.go | 11 +++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/internal/commands/feed.go b/internal/commands/feed.go index d7cb74f3f..3aa9cd730 100644 --- a/internal/commands/feed.go +++ b/internal/commands/feed.go @@ -495,9 +495,11 @@ func (r feedRequest) resumeNotice(position string, capped bool, maxPages int) st // positionCommand is the invocation that resumes from a position. The filters // ride along because a position is bound to the set it was minted for: the // same command without them is a different filter set, and the server answers -// 409. +// 409. The position itself is shell-quoted — it is opaque server text going +// into a command somebody pastes, and nothing about it promises to be a bare +// word. func (r feedRequest) positionCommand(position string) string { - return fmt.Sprintf("%s --position %s%s", r.lane.pollCmd, position, r.filters) + return fmt.Sprintf("%s --position %s%s", r.lane.pollCmd, shellQuote(position), r.filters) } // newEventsPollCmd builds `basecamp events poll`. diff --git a/internal/commands/feed_test.go b/internal/commands/feed_test.go index 7dd740852..3df92b2c5 100644 --- a/internal/commands/feed_test.go +++ b/internal/commands/feed_test.go @@ -598,6 +598,17 @@ func TestInboxHumanOutputCarriesTheResumeCommand(t *testing.T) { assert.Contains(t, out.String(), "Resume from here with: basecamp inbox --position inbox-pos-1") } +// An opaque position goes into a command somebody pastes, so it is quoted +// rather than trusted to be a bare word. +func TestAPositionIsShellQuotedInTheResumeCommand(t *testing.T) { + app, _, out := setupFeedApp(t, eventsRoute(http.StatusOK, feedPageJSON("pos 1; rm -rf /", "", 11))) + app.Flags.Hints = true + + require.NoError(t, executeRecordingCommand(NewEventsCmd(), app, "poll", "--since", "now")) + + assert.Equal(t, `basecamp events poll --position 'pos 1; rm -rf /'`, resumeBreadcrumb(t, out)) +} + // A capped walk says both things in one notice: more remains, and here is // where to continue from. func TestACappedWalkSaysSoAndStillHandsBackThePosition(t *testing.T) { From 62c66dc0a548256698ea9e4bcd90a473e465bc76 Mon Sep 17 00:00:00 2001 From: Jeremy Daer <jeremy@37signals.com> Date: Wed, 16 Sep 2026 06:42:07 -0700 Subject: [PATCH 11/29] Re-hash the flake for the SDK re-pin --- nix/package.nix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nix/package.nix b/nix/package.nix index 3cfa0e643..0ec678cc7 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-cpyRw1eZo241lxDP5w2LRkNAQIiKVdDi6dGMBq5TctE="; + vendorHash = "sha256-eTROZTeU4FbooBGNSKiuqH8ohtXhvnDrHTB69pACHL4="; subPackages = [ "cmd/basecamp" ]; From b283eccf36238e549c55b38fceca378a60374dd2 Mon Sep 17 00:00:00 2001 From: Jeremy Daer <jeremy@37signals.com> Date: Wed, 16 Sep 2026 12:00:37 -0700 Subject: [PATCH 12/29] Say the walk was capped in --ids-only and --count too Those two formats print rows alone: Writer.write routes them straight to writeIDs/writeCount, so no envelope field survives and the notice carrying the cap warning and the resume position was destroyed rather than relocated. A capped --all walk handed back an id list or a count that read as the complete answer. Rows-only formats now print the notice on stderr, where it cannot corrupt the stream a script is reading. Quiet mode keeps its diagnostic-only gate: its stdout is the whole Data object, position and next included. --- internal/commands/feed.go | 11 ++++-- internal/commands/feed_test.go | 54 ++++++++++++++++++++++++++++ internal/output/envelope.go | 32 ++++++++++++----- internal/output/output_test.go | 64 ++++++++++++++++++++++++++++++++++ 4 files changed, 149 insertions(+), 12 deletions(-) diff --git a/internal/commands/feed.go b/internal/commands/feed.go index 3aa9cd730..de5867666 100644 --- a/internal/commands/feed.go +++ b/internal/commands/feed.go @@ -478,9 +478,14 @@ func checkContinuation(baseURL, raw string) error { // The envelope carries it for machines, but the rendered output is the rows — // and the generic renderer drops a field named "position" from objects // anyway, since on a to-do or a card that name means an ordering integer. So -// the notice, which every format prints verbatim, carries the whole resume -// command. A walk that stopped at its page cap says so in the same sentence, -// because "more remains" and "here is where to continue" are one thought. +// the notice carries the whole resume command. A walk that stopped at its +// page cap says so in the same sentence, because "more remains" and "here is +// where to continue" are one thought. +// +// Every format reaches it: the envelope formats render it, and --ids-only +// and --count — whose stdout is rows alone — get it on stderr. Without that +// second channel a capped walk prints an id list or a count that reads as +// the complete answer. func (r feedRequest) resumeNotice(position string, capped bool, maxPages int) string { notice := "" if capped { diff --git a/internal/commands/feed_test.go b/internal/commands/feed_test.go index 3df92b2c5..51382bd5f 100644 --- a/internal/commands/feed_test.go +++ b/internal/commands/feed_test.go @@ -644,6 +644,60 @@ func TestEnumeratingOutputModesSeeTheRows(t *testing.T) { assert.Equal(t, "991\n992\n", out.String()) } +// The enumerating modes print rows alone, so a capped walk would otherwise +// hand back an id list or a count that reads as the complete answer. The +// notice reaches them on stderr, where it cannot corrupt the stream a script +// is reading. +func TestACappedWalkWarnsTheEnumeratingOutputModesToo(t *testing.T) { + next := feedBaseURL + feedEventsPath + "?position=pos-1" + wanted := "Stopped at --max-pages 1; more remains. " + + "Resume from here with: basecamp events poll --position pos-1" + + for _, mode := range []struct { + name string + format output.Format + stdout string + }{ + {"ids", output.FormatIDs, "11\n"}, + {"count", output.FormatCount, "1\n"}, + } { + t.Run(mode.name, func(t *testing.T) { + app, _, out := setupFeedApp(t, eventsRoute(http.StatusOK, + feedPageJSON("pos-1", next, 11), + feedPageJSON("pos-2", next, 12), + )) + var stderr bytes.Buffer + app.Output = output.New(output.Options{Format: mode.format, Writer: out, ErrWriter: &stderr}) + + require.NoError(t, executeRecordingCommand(NewEventsCmd(), app, + "poll", "--since", "0", "--all", "--max-pages", "1")) + + assert.Equal(t, mode.stdout, out.String()) + assert.Equal(t, "notice: "+wanted+"\n", stderr.String()) + }) + } +} + +// The inbox lane caps the same way and owes the same warning. +func TestACappedInboxWalkWarnsTheEnumeratingOutputModesToo(t *testing.T) { + next := feedBaseURL + feedInboxPath + "?position=inbox-pos-1" + app, _, out := setupFeedApp(t, inboxRoute(http.StatusOK, + inboxPageJSON("inbox-pos-1", next, 991), + inboxPageJSON("inbox-pos-2", next, 992), + )) + var stderr bytes.Buffer + app.Output = output.New(output.Options{Format: output.FormatIDs, Writer: out, ErrWriter: &stderr}) + + require.NoError(t, executeRecordingCommand(NewInboxCmd(), app, + "--since", "0", "--all", "--max-pages", "1")) + + assert.Equal(t, "991\n", out.String()) + assert.Equal(t, + "notice: Stopped at --max-pages 1; more remains. "+ + "Resume from here with: basecamp inbox --position inbox-pos-1\n", + stderr.String()) +} + func TestContinuationOriginComparisonNormalizesDefaultPortsAndCase(t *testing.T) { require.NoError(t, checkContinuation("https://3.basecampapi.com:443", feedBaseURL+feedEventsPath+"?position=p")) require.NoError(t, checkContinuation(feedBaseURL, "HTTPS://3.BasecampAPI.com"+feedEventsPath+"?position=p")) diff --git a/internal/output/envelope.go b/internal/output/envelope.go index 6903cf8df..18357d2bb 100644 --- a/internal/output/envelope.go +++ b/internal/output/envelope.go @@ -90,7 +90,7 @@ const ( type Options struct { Format Format Writer io.Writer - ErrWriter io.Writer // Diagnostic output (notices in quiet mode); defaults to os.Stderr. + ErrWriter io.Writer // Notices stdout cannot carry (see write); defaults to os.Stderr. Verbose bool JQFilter string // jq expression to apply to JSON output (built-in via gojq) } @@ -210,13 +210,21 @@ func WithErrorStats(metrics *observability.SessionMetrics) ErrorResponseOption { } func (w *Writer) write(v any) error { - // In quiet mode (--agent/--quiet), surface diagnostic notices on stderr so - // automation consumers can detect degraded operations (e.g. unresolved - // mentions). Only notices marked as diagnostic emit here — informational - // notices like truncation warnings stay silent. This runs before the --jq - // early-return so that --agent --jq still emits the diagnostic. - if w.opts.Format == FormatQuiet { - if resp, ok := v.(*Response); ok && resp.noticeDiagnostic && resp.Notice != "" { + // Notices that stdout cannot carry go to stderr, so a machine-readable + // stream stays uncorrupted and the caller still hears them. + // + // Quiet mode (--agent/--quiet) prints the whole Data object, so only a + // degraded-operation diagnostic (an unresolved mention, say) needs the + // second channel; informational notices like truncation warnings stay + // silent. --ids-only and --count print rows alone — no envelope field + // survives them at all — so for those every notice is otherwise + // destroyed rather than relocated, and all of them emit here. + // + // This runs before the --jq early-return so that --agent --jq still + // emits the diagnostic. + if w.opts.Format == FormatQuiet || w.opts.Format == FormatIDs || w.opts.Format == FormatCount { + if resp, ok := v.(*Response); ok && resp.Notice != "" && + (resp.noticeDiagnostic || w.opts.Format != FormatQuiet) { // The notice may interpolate API-controlled strings and stderr is // a terminal sink; sanitize and keep the diagnostic to one line. // Sanitize first, then gate: an all-escape notice collapses to "" @@ -604,6 +612,10 @@ func WithSummary(s string) ResponseOption { // WithNotice adds an informational notice to the response. // Use this for non-error messages like truncation warnings. // Like WithSummary, the value is stored verbatim; terminal sinks sanitize. +// +// Every format renders it: the envelope formats carry it in the response, +// and the rows-only ones (--ids-only, --count) print it on stderr, since +// their stdout has no field to carry it in. func WithNotice(s string) ResponseOption { return func(r *Response) { r.Notice = s; r.noticeDiagnostic = false } } @@ -611,7 +623,9 @@ func WithNotice(s string) ResponseOption { // WithDiagnostic sets a notice that is also emitted to stderr in quiet mode. // Use this for degraded-operation warnings (e.g. unresolved mentions) that // automation consumers need to detect. Truncation and other informational -// notices should use WithNotice instead. +// notices should use WithNotice instead — which still reaches stderr under +// --ids-only and --count; the distinction is quiet mode, whose stdout +// already carries the whole payload. func WithDiagnostic(s string) ResponseOption { return func(r *Response) { r.Notice = s; r.noticeDiagnostic = true } } diff --git a/internal/output/output_test.go b/internal/output/output_test.go index 959b20aec..97a3046d6 100644 --- a/internal/output/output_test.go +++ b/internal/output/output_test.go @@ -792,6 +792,70 @@ func TestWriterCountFormatSingleItem(t *testing.T) { assert.Equal(t, "1\n", output) } +// TestWriterIDsFormatNoticeOnStderr and its --count twin verify the rows-only +// formats still deliver a notice. Their stdout has no envelope field to carry +// one, so without the stderr channel a capped walk prints an id list or a +// count that reads as the complete answer. +func TestWriterIDsFormatNoticeOnStderr(t *testing.T) { + var stdout, stderr bytes.Buffer + w := New(Options{ + Format: FormatIDs, + Writer: &stdout, + ErrWriter: &stderr, + }) + + data := []map[string]any{{"id": 1}, {"id": 2}} + err := w.OK(data, WithNotice("Stopped at --max-pages 2; more remains.")) + require.NoError(t, err) + + assert.Equal(t, "1\n2\n", stdout.String(), "notice must not corrupt the id stream") + assert.Contains(t, stderr.String(), "Stopped at --max-pages 2; more remains.") +} + +func TestWriterCountFormatNoticeOnStderr(t *testing.T) { + var stdout, stderr bytes.Buffer + w := New(Options{ + Format: FormatCount, + Writer: &stdout, + ErrWriter: &stderr, + }) + + data := []map[string]any{{"id": 1}, {"id": 2}} + err := w.OK(data, WithNotice("Stopped at --max-pages 2; more remains.")) + require.NoError(t, err) + + assert.Equal(t, "2\n", stdout.String(), "notice must not corrupt the count stream") + assert.Contains(t, stderr.String(), "Stopped at --max-pages 2; more remains.") +} + +func TestWriterIDsFormatNoNoticeLeavesStderrEmpty(t *testing.T) { + var stdout, stderr bytes.Buffer + w := New(Options{ + Format: FormatIDs, + Writer: &stdout, + ErrWriter: &stderr, + }) + + err := w.OK([]map[string]any{{"id": 1}}, WithSummary("all good")) + require.NoError(t, err) + + assert.Empty(t, stderr.String()) +} + +func TestWriterIDsFormatNoticeIsSanitized(t *testing.T) { + var stdout, stderr bytes.Buffer + w := New(Options{ + Format: FormatIDs, + Writer: &stdout, + ErrWriter: &stderr, + }) + + err := w.OK([]map[string]any{{"id": 1}}, WithNotice("\x1b]0;evil\x07more\nremains")) + require.NoError(t, err) + + assert.Equal(t, "notice: more remains\n", stderr.String()) +} + func TestDefaultOptions(t *testing.T) { opts := DefaultOptions() From 66d8c26f7398df6bf8920e368261f6ce68054829 Mon Sep 17 00:00:00 2001 From: Jeremy Daer <jeremy@37signals.com> Date: Wed, 16 Sep 2026 12:00:37 -0700 Subject: [PATCH 13/29] Name the set the stream-ticket mint is actually excluded by MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The generated provenance credited EXCLUDED_OPERATIONS, but the mint is in POLICY_EXCLUDED_OPERATIONS. The two sets answer different questions — the toolkit cannot load a non-JSON body, versus we decline to serve an operation that would load fine — and naming the wrong one erases the reason the mint is absent for whoever reads the file to find out. Fixed in the generator and regenerated; the model files came back byte-identical. --- internal/mcpserver/model/PROVENANCE.json | 2 +- scripts/sync-mcp-model.sh | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/internal/mcpserver/model/PROVENANCE.json b/internal/mcpserver/model/PROVENANCE.json index 5f7c5d4c6..7fb80ac03 100644 --- a/internal/mcpserver/model/PROVENANCE.json +++ b/internal/mcpserver/model/PROVENANCE.json @@ -4,5 +4,5 @@ "ref": "go/v0.18.0-51-ga8046263", "files": ["behavior-model.json", "openapi.json"], "synced_by": "scripts/sync-mcp-model.sh", - "patches": "binary-upload operations and the stream-ticket mint dropped (EXCLUDED_OPERATIONS); no tag patches applied (PATCHED_TAGS is empty — the export tags every operation) — see the sync script" + "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/scripts/sync-mcp-model.sh b/scripts/sync-mcp-model.sh index be71d3dd2..5912137bc 100755 --- a/scripts/sync-mcp-model.sh +++ b/scripts/sync-mcp-model.sh @@ -13,7 +13,8 @@ # Tag patch: the toolkit catalog joins operations to domains by tag (exactly # one per operation, refused otherwise), and the SDK export used to leave a # handful of operations untagged. It no longer does, so PATCHED_TAGS is empty -# and the only divergence from the upstream export is EXCLUDED_OPERATIONS. +# and the only divergences from the upstream export are EXCLUDED_OPERATIONS +# and POLICY_EXCLUDED_OPERATIONS. # The table stays as a seam: an operation that grew an upstream tag is a # sync-time conflict rather than a silent double-tag, and an operation that # arrives untagged stops the sync rather than failing later at Load. @@ -141,7 +142,7 @@ cat > "$dest/PROVENANCE.json" <<JSON "ref": "$ref", "files": ["behavior-model.json", "openapi.json"], "synced_by": "scripts/sync-mcp-model.sh", - "patches": "binary-upload operations and the stream-ticket mint dropped (EXCLUDED_OPERATIONS); no tag patches applied (PATCHED_TAGS is empty — the export tags every operation) — see the sync script" + "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" } JSON From 6e241bfc88fcda96edec47f23b3c5cf83ccac417 Mon Sep 17 00:00:00 2001 From: Jeremy Daer <jeremy@37signals.com> Date: Wed, 16 Sep 2026 12:07:45 -0700 Subject: [PATCH 14/29] Withhold a cable URL that still carries the ticket anywhere MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Blanking the ticket parameter is not the same as redacting the ticket. net/url preserved the rest of the response verbatim, so a URL whose fragment, userinfo, path or second parameter repeated the credential printed it without --show-secret — against the function's own stated posture that any component can be carrying it. The display URL is now built from the components a cable URL needs, which drops userinfo and fragment outright, and the result is withheld whole if the ticket survives in it. --- internal/commands/feed.go | 29 +++++++++++++++++------ internal/commands/feed_test.go | 43 +++++++++++++++++++++++++++++++--- 2 files changed, 62 insertions(+), 10 deletions(-) diff --git a/internal/commands/feed.go b/internal/commands/feed.go index de5867666..8ed0e362a 100644 --- a/internal/commands/feed.go +++ b/internal/commands/feed.go @@ -726,7 +726,7 @@ attempt; an open socket does not refresh one. } if !showSecret { data["ticket"] = redactedTicket - data["url"] = redactTicketURL(ticket.URL) + data["url"] = redactTicketURL(ticket.URL, ticket.Ticket) } respOpts := []output.ResponseOption{ @@ -750,10 +750,15 @@ attempt; an open socket does not refresh one. // credential invites a consumer to present it. const redactedTicket = "[REDACTED]" -// redactTicketURL renders a cable URL without its credential. A URL that -// cannot be parsed is withheld whole rather than echoed: the ticket is -// opaque, so any component of an unparseable URL can be carrying it. -func redactTicketURL(raw string) string { +// redactTicketURL renders a cable URL without its credential. The ticket is +// opaque and the response is not ours to trust, so the display URL is built +// from the components a cable URL needs rather than by editing the one that +// arrived: userinfo and fragment are dropped outright, and a URL that cannot +// be parsed, carries no ticket parameter, or still contains the ticket after +// all that — echoed in the path, or in a second parameter — is withheld +// whole. Any component can be carrying it, so a rendering that still holds +// it is worth nothing. +func redactTicketURL(raw, ticket string) string { parsed, err := url.Parse(raw) if err != nil { return redactedTicket @@ -763,6 +768,16 @@ func redactTicketURL(raw string) string { return redactedTicket } query.Set("ticket", redactedTicket) - parsed.RawQuery = query.Encode() - return parsed.String() + + display := url.URL{ + Scheme: parsed.Scheme, + Host: parsed.Host, + Path: parsed.Path, + RawQuery: query.Encode(), + } + rendered := display.String() + if ticket != "" && strings.Contains(rendered, ticket) { + return redactedTicket + } + return rendered } diff --git a/internal/commands/feed_test.go b/internal/commands/feed_test.go index 51382bd5f..b3ce2b19f 100644 --- a/internal/commands/feed_test.go +++ b/internal/commands/feed_test.go @@ -524,14 +524,51 @@ func TestEventsTicketShowSecretPrintsItVerbatim(t *testing.T) { } func TestRedactTicketURLWithholdsWhatItCannotReduce(t *testing.T) { - assert.Equal(t, redactedTicket, redactTicketURL("wss://chat.example.test/195539477")) - assert.Equal(t, redactedTicket, redactTicketURL("://")) + assert.Equal(t, redactedTicket, redactTicketURL("wss://chat.example.test/195539477", "tkt-secret")) + assert.Equal(t, redactedTicket, redactTicketURL("://", "tkt-secret")) - redacted := redactTicketURL("wss://chat.example.test/195539477?ticket=tkt-secret") + redacted := redactTicketURL("wss://chat.example.test/195539477?ticket=tkt-secret", "tkt-secret") assert.NotContains(t, redacted, "tkt-secret") assert.Contains(t, redacted, "chat.example.test/195539477") } +// Redacting the ticket parameter is not the same as redacting the ticket: the +// response is not ours to trust, and a URL that carries the credential +// anywhere else is withheld whole rather than echoed with one parameter +// blanked. +func TestRedactTicketURLWithholdsATicketEchoedOutsideItsParameter(t *testing.T) { + for _, raw := range []string{ + "wss://chat.example.test/195539477?ticket=tkt-secret#tkt-secret", + "wss://chat.example.test/tkt-secret?ticket=tkt-secret", + "wss://chat.example.test/195539477?ticket=tkt-secret&retry=tkt-secret", + "wss://tkt-secret@chat.example.test/195539477?ticket=tkt-secret", + } { + assert.NotContains(t, redactTicketURL(raw, "tkt-secret"), "tkt-secret", raw) + } +} + +// An empty ticket must not make every URL look like it carries one. +func TestRedactTicketURLStillRendersWhenTheTicketIsEmpty(t *testing.T) { + assert.Equal(t, + "wss://chat.example.test/195539477?ticket="+url.QueryEscape(redactedTicket), + redactTicketURL("wss://chat.example.test/195539477?ticket=", "")) +} + +// The whole point of the redacted URL is that a person can still see which +// cable endpoint they would be opening. +func TestEventsTicketRedactionKeepsTheEndpointVisible(t *testing.T) { + app, _, out := setupFeedApp(t, stubRoute{ + method: http.MethodPost, + path: feedTicketPath, + status: http.StatusOK, + body: `{"ticket":"tkt-secret","expires_in":120,"url":"wss://chat.example.test/195539477?ticket=tkt-secret#tkt-secret"}`, + }) + + require.NoError(t, executeRecordingCommand(NewEventsCmd(), app, "ticket")) + + assert.NotContains(t, out.String(), "tkt-secret") +} + // A 410 on a filtered poll must hand back a re-entry that still carries the // filters — the loop guard above all, since an agent that re-enters without // it recovers straight into its own activity. From 348a753c6a1dae247924ebee6d6047bb2c8711f9 Mon Sep 17 00:00:00 2001 From: Jeremy Daer <jeremy@37signals.com> Date: Wed, 16 Sep 2026 12:14:22 -0700 Subject: [PATCH 15/29] Resume from the filters the last page was minted under The walk refreshed its request options from each continuation but not the filter string the resume command and breadcrumb are rendered from, so a continuation carrying its own filters produced a re-entry describing the first call's flags. A position is bound to the set it was minted for, so that re-entry is a different set and the server answers 409. --- internal/commands/feed.go | 4 ++++ internal/commands/feed_test.go | 32 ++++++++++++++++++++++++++++++++ internal/commands/inbox.go | 1 + 3 files changed, 37 insertions(+) diff --git a/internal/commands/feed.go b/internal/commands/feed.go index 8ed0e362a..c5c309ee4 100644 --- a/internal/commands/feed.go +++ b/internal/commands/feed.go @@ -629,6 +629,10 @@ event id and refetch the referenced recording before acting on it.`, if opts, err = basecamp.PollEventsOptionsFromURL(next); err != nil { return convertSDKError(err) } + // A position is bound to the filter set it was minted for, and + // the continuation carries its own. Re-entering with the first + // call's flags would be a different set, and a 409. + request.filters = requestFilters(eventsFilterFlags(opts)) } summary := fmt.Sprintf("%d event(s)", len(events)) diff --git a/internal/commands/feed_test.go b/internal/commands/feed_test.go index b3ce2b19f..9d6f214d3 100644 --- a/internal/commands/feed_test.go +++ b/internal/commands/feed_test.go @@ -735,6 +735,38 @@ func TestACappedInboxWalkWarnsTheEnumeratingOutputModesToo(t *testing.T) { stderr.String()) } +// A position is bound to the filter set it was minted for, and the walk's +// last page is minted under the continuation's filters, not the first call's. +// A resume command built from the original flags is a different set, and the +// server answers 409. +func TestTheResumeCommandCarriesTheContinuationsOwnFilters(t *testing.T) { + next := feedBaseURL + feedEventsPath + "?position=pos-1&types=message.created" + app, _, out := setupFeedApp(t, eventsRoute(http.StatusOK, + feedPageJSON("pos-1", next, 11), + feedPageJSON("pos-2", "", 12), + )) + + require.NoError(t, executeRecordingCommand(NewEventsCmd(), app, "poll", "--since", "0", "--all")) + + assert.Equal(t, + "Resume from here with: basecamp events poll --position pos-2 --types message.created", + decodeFeedEnvelope(t, out).Notice) +} + +func TestTheInboxResumeCommandCarriesTheContinuationsOwnFilters(t *testing.T) { + next := feedBaseURL + feedInboxPath + "?position=inbox-pos-1&reasons=mentioned" + app, _, out := setupFeedApp(t, inboxRoute(http.StatusOK, + inboxPageJSON("inbox-pos-1", next, 991), + inboxPageJSON("inbox-pos-2", "", 992), + )) + + require.NoError(t, executeRecordingCommand(NewInboxCmd(), app, "--since", "0", "--all")) + + assert.Equal(t, + "Resume from here with: basecamp inbox --position inbox-pos-2 --reasons mentioned", + decodeFeedEnvelope(t, out).Notice) +} + func TestContinuationOriginComparisonNormalizesDefaultPortsAndCase(t *testing.T) { require.NoError(t, checkContinuation("https://3.basecampapi.com:443", feedBaseURL+feedEventsPath+"?position=p")) require.NoError(t, checkContinuation(feedBaseURL, "HTTPS://3.BasecampAPI.com"+feedEventsPath+"?position=p")) diff --git a/internal/commands/inbox.go b/internal/commands/inbox.go index 1f2a823ab..c8a3e41fd 100644 --- a/internal/commands/inbox.go +++ b/internal/commands/inbox.go @@ -111,6 +111,7 @@ deduplicate by addressing_id, never by the event's id.`, if opts, err = basecamp.PollInboxOptionsFromURL(next); err != nil { return convertSDKError(err) } + request.filters = requestFilters(inboxFilterFlags(opts)) } summary := fmt.Sprintf("%d addressed item(s)", len(items)) From 250e2958206cfc3549478ae371f93b8a2cb3e3ed Mon Sep 17 00:00:00 2001 From: Jeremy Daer <jeremy@37signals.com> Date: Wed, 16 Sep 2026 12:14:30 -0700 Subject: [PATCH 16/29] Suppress --project when the id-or-url positional is optional too The classifier matched the required spelling only. events grew subcommands this release and so spells its positional [id|url], which put the irrelevant inherited --project back into its agent help. --- internal/cli/help.go | 5 +++-- internal/cli/help_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/internal/cli/help.go b/internal/cli/help.go index e63dc9e2f..5ea87c669 100644 --- a/internal/cli/help.go +++ b/internal/cli/help.go @@ -415,11 +415,12 @@ func parentScopedFlags(cmd *cobra.Command) *pflag.FlagSet { // Rules: // - Only root-level persistent flags (pointer identity vs root.PersistentFlags) // - Only flags in salientRootFlags -// - Suppress --project for commands whose Use contains <id|url> +// - Suppress --project for commands that take an id-or-url positional, +// whether the spelling is required (<id|url>) or optional ([id|url]) func curatedInheritedFlags(cmd *cobra.Command) *pflag.FlagSet { root := cmd.Root() filtered := pflag.NewFlagSet("inherited", pflag.ContinueOnError) - acceptsID := strings.Contains(cmd.Use, "<id|url>") + acceptsID := strings.Contains(cmd.Use, "id|url") cmd.InheritedFlags().VisitAll(func(f *pflag.Flag) { rootFlag := root.PersistentFlags().Lookup(f.Name) diff --git a/internal/cli/help_test.go b/internal/cli/help_test.go index 52a8eccb3..3eacb64c2 100644 --- a/internal/cli/help_test.go +++ b/internal/cli/help_test.go @@ -871,6 +871,30 @@ func TestAgentHelpSuppressesProjectForIDURLCommands(t *testing.T) { assert.Contains(t, names, "json") } +// A command whose id-or-url positional is optional is still an id-or-url +// command: events grew subcommands and so spells it [id|url], and --project +// is no more relevant to it than before. +func TestAgentHelpSuppressesProjectWhenTheIDURLPositionalIsOptional(t *testing.T) { + isolateHelpTest(t) + + var buf bytes.Buffer + cmd := NewRootCmd() + cmd.AddCommand(commands.NewEventsCmd()) + cmd.SetOut(&buf) + cmd.SetArgs([]string{"events", "--agent", "--help"}) + require.NoError(t, cmd.Execute()) + + var info agentHelpInfo + require.NoError(t, json.Unmarshal(buf.Bytes(), &info)) + + names := make([]string, len(info.InheritedFlags)) + for i, f := range info.InheritedFlags { + names[i] = f.Name + } + assert.NotContains(t, names, "project", "agent help should suppress --project for [id|url] commands too") + assert.Contains(t, names, "json") +} + func TestAgentHelpPromotesParentScopedFlags(t *testing.T) { isolateHelpTest(t) From 70a68c4a4918c2fc5a8306231c76d677dae9418a Mon Sep 17 00:00:00 2001 From: Jeremy Daer <jeremy@37signals.com> Date: Wed, 16 Sep 2026 12:14:30 -0700 Subject: [PATCH 17/29] Say 195 endpoints and name the event feed The skill's coverage header and the API-COVERAGE version label both described an earlier surface than the one this release ships, which is the sort of contradiction an agent reads as authoritative. The counts now come from the coverage matrix and the version label defers to the pin. --- API-COVERAGE.md | 10 ++++++---- skills/basecamp/SKILL.md | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/API-COVERAGE.md b/API-COVERAGE.md index 00d4e5598..6faacb4cf 100644 --- a/API-COVERAGE.md +++ b/API-COVERAGE.md @@ -48,10 +48,12 @@ Out-of-scope sections are excluded from parity totals and scripts: chatbots (dif > Note: the per-row `Endpoints` column in the Coverage by Section table sums higher than the Summary totals above. The discrepancy predates the BC5 baseline; the row count (48 sections) is authoritative for the `Since` column. Reconciling endpoint counts is pre-existing maintenance, tracked separately. -**SDK version:** v0.16.0 (adds the to-do list template library and asynchronous -copy operations; `internal/version/sdk-provenance.json` is authoritative). The -event-feed operations are unreleased, so the pin is a basecamp-sdk `main` -pseudo-version until the next SDK release tag. The +**SDK version:** the pin in `go.mod`, with +`internal/version/sdk-provenance.json` authoritative for the exact commit. The +event-feed operations are unreleased, so that pin is a basecamp-sdk `main` +pseudo-version rather than a release tag; the last tagged release it builds on +is v0.16.0, which added the to-do list template library and asynchronous copy +operations. The command surface below largely dates to the v0.12.0 bump, which added 20 exported Go methods over 13 new backend operations; the extra seven wrapped endpoints that already existed but were reachable only through the raw generated client, diff --git a/skills/basecamp/SKILL.md b/skills/basecamp/SKILL.md index 6a2765916..6af8d5a65 100644 --- a/skills/basecamp/SKILL.md +++ b/skills/basecamp/SKILL.md @@ -81,7 +81,7 @@ argument-hint: "[action] [args...]" # /basecamp - Basecamp Workflow Command -Full CLI coverage: 189 tracked in-scope endpoints across todos, cards, messages, files, schedule, check-ins, timeline, recordings, templates, webhooks, subscriptions, lineup, chat, pings, gauges, assignments, notifications, and accounts. +Full CLI coverage: 195 tracked in-scope endpoints across todos, cards, messages, files, schedule, check-ins, timeline, recordings, templates, webhooks, subscriptions, lineup, chat, pings, gauges, assignments, notifications, the account event feed, and accounts. ## Agent Invariants From ae1a9344018653f1d8163ea691b8f74d7b335fd3 Mon Sep 17 00:00:00 2001 From: Jeremy Daer <jeremy@37signals.com> Date: Wed, 16 Sep 2026 12:20:01 -0700 Subject: [PATCH 18/29] Build the redacted cable URL's query instead of editing it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Carrying the server's other parameters through meant asking whether a copy of the ticket was hiding in one, and that question has no stable answer: the rendering escapes as it sees fit, so a ticket "abc/def" echoed as "abc%2Fdef" matched neither a search for the raw ticket nor one for the escaped form. The query is now rebuilt from a constant, which leaves only scheme, host and path to vet — and those are compared decoded. --- internal/commands/feed.go | 38 ++++++++++++++++++---------------- internal/commands/feed_test.go | 16 ++++++++++++++ 2 files changed, 36 insertions(+), 18 deletions(-) diff --git a/internal/commands/feed.go b/internal/commands/feed.go index c5c309ee4..2df918cc4 100644 --- a/internal/commands/feed.go +++ b/internal/commands/feed.go @@ -754,34 +754,36 @@ attempt; an open socket does not refresh one. // credential invites a consumer to present it. const redactedTicket = "[REDACTED]" -// redactTicketURL renders a cable URL without its credential. The ticket is -// opaque and the response is not ours to trust, so the display URL is built -// from the components a cable URL needs rather than by editing the one that -// arrived: userinfo and fragment are dropped outright, and a URL that cannot -// be parsed, carries no ticket parameter, or still contains the ticket after -// all that — echoed in the path, or in a second parameter — is withheld -// whole. Any component can be carrying it, so a rendering that still holds -// it is worth nothing. +// redactTicketURL renders a cable URL without its credential: enough of the +// endpoint to see where a stream would open, and none of the response's own +// text. +// +// The ticket is opaque and the body is not ours to trust, so the display URL +// is built rather than edited. Only scheme, host and path survive; the query +// is rebuilt from a constant, which is what makes this answerable at all — +// carrying the server's other parameters through means asking whether a copy +// of the ticket is hiding in one, and the answer depends on the encoding the +// rendering happens to choose (a ticket "abc/def" echoed as "abc%2Fdef" +// reads as neither). Userinfo and the fragment are dropped the same way. +// +// What is left is compared decoded: url.Parse unescapes the path, so a copy +// escaped into it is still caught, and a URL carrying the ticket in its host +// or path is withheld whole. An unparseable URL, or one with no ticket +// parameter, is withheld too — it is not the shape this was promised. func redactTicketURL(raw, ticket string) string { parsed, err := url.Parse(raw) - if err != nil { + if err != nil || !parsed.Query().Has("ticket") { return redactedTicket } - query := parsed.Query() - if !query.Has("ticket") { + if ticket != "" && (strings.Contains(parsed.Host, ticket) || strings.Contains(parsed.Path, ticket)) { return redactedTicket } - query.Set("ticket", redactedTicket) display := url.URL{ Scheme: parsed.Scheme, Host: parsed.Host, Path: parsed.Path, - RawQuery: query.Encode(), + RawQuery: url.Values{"ticket": {redactedTicket}}.Encode(), } - rendered := display.String() - if ticket != "" && strings.Contains(rendered, ticket) { - return redactedTicket - } - return rendered + return display.String() } diff --git a/internal/commands/feed_test.go b/internal/commands/feed_test.go index 9d6f214d3..0c2f9af0b 100644 --- a/internal/commands/feed_test.go +++ b/internal/commands/feed_test.go @@ -542,11 +542,27 @@ func TestRedactTicketURLWithholdsATicketEchoedOutsideItsParameter(t *testing.T) "wss://chat.example.test/tkt-secret?ticket=tkt-secret", "wss://chat.example.test/195539477?ticket=tkt-secret&retry=tkt-secret", "wss://tkt-secret@chat.example.test/195539477?ticket=tkt-secret", + "wss://chat.example.test/195539477%2Ftkt-secret?ticket=tkt-secret", } { assert.NotContains(t, redactTicketURL(raw, "tkt-secret"), "tkt-secret", raw) } } +// A copy of the ticket in another parameter used to survive whenever the +// rendering escaped it into a spelling a search for the raw ticket does not +// match — "abc/def" carried as "abc%2Fdef". The query is rebuilt from a +// constant rather than carried through, so there is no second parameter to +// hide in and no encoding to reason about. +func TestRedactTicketURLKeepsNoneOfTheServersOtherParameters(t *testing.T) { + assert.Equal(t, + "wss://chat.example.test/cable?ticket="+url.QueryEscape(redactedTicket), + redactTicketURL("wss://chat.example.test/cable?ticket=abc/def&retry=abc/def", "abc/def")) + + assert.NotContains(t, + redactTicketURL("wss://chat.example.test/cable?ticket=abc%2Fdef&retry=abc%2Fdef", "abc/def"), + "abc") +} + // An empty ticket must not make every URL look like it carries one. func TestRedactTicketURLStillRendersWhenTheTicketIsEmpty(t *testing.T) { assert.Equal(t, From 13f535ab0f9a10885a196ec4f7006d031a6a256d Mon Sep 17 00:00:00 2001 From: Jeremy Daer <jeremy@37signals.com> Date: Wed, 16 Sep 2026 12:20:01 -0700 Subject: [PATCH 19/29] Project the feed smoke reads down to what they assert MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit bats keeps $output and a failing assertion prints it in full, so capturing a whole poll envelope put a signed resume position in logs somebody keeps — which is what this suite's own header said it did not do. Each read now asks for the fields it checks and nothing else. --- e2e/smoke/smoke_event_feed.bats | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/e2e/smoke/smoke_event_feed.bats b/e2e/smoke/smoke_event_feed.bats index 8e81c77ac..6c576f069 100644 --- a/e2e/smoke/smoke_event_feed.bats +++ b/e2e/smoke/smoke_event_feed.bats @@ -1,10 +1,13 @@ #!/usr/bin/env bats # smoke_event_feed.bats - Level 0: Account event feed operations # -# Nothing here prints a position, a ticket or a cable URL: the first is a -# signed resume token and the other two are bearers, and a smoke trace is a -# file somebody keeps. Entering at the present (--since now) also keeps these -# reads bounded — a since=0 replay would walk the account's served history. +# Nothing here captures a position, a ticket or a cable URL: the first is a +# signed resume token and the other two are bearers, and bats keeps $output — +# a failing assertion prints it in full (test_helper.bash:118) into logs +# somebody keeps. So every read projects the envelope down to the fields it +# asserts with --jq, rather than capturing the whole thing and trusting +# redaction. Entering at the present (--since now) also keeps these reads +# bounded — a since=0 replay would walk the account's served history. # # This sits in Level 0 alongside the other read-only suites even though the # ticket mint is a POST: it writes no record, mints a stateless signed token @@ -18,7 +21,7 @@ setup_file() { } @test "events poll enters the feed at the present" { - run_smoke basecamp events poll --since now --json + run_smoke basecamp events poll --since now --jq '{ok: .ok}' assert_success assert_json_value '.ok' 'true' } @@ -30,14 +33,14 @@ setup_file() { } @test "events ticket mints a redacted ticket" { - run_smoke basecamp events ticket --json + run_smoke basecamp events ticket --jq '{ok: .ok, data: {ticket: .data.ticket}}' assert_success assert_json_value '.ok' 'true' assert_json_value '.data.ticket' '[REDACTED]' } @test "inbox polls addressed items" { - run_smoke basecamp inbox --since now --json + run_smoke basecamp inbox --since now --jq '{ok: .ok, error: .error}' # The inbox is served to agent principals only; a person's token gets 403. if [[ "$status" -ne 0 ]]; then assert_output_contains "agent principals only" From ca6141379998cd3045e411f2ecfbc719a8a565a8 Mon Sep 17 00:00:00 2001 From: Jeremy Daer <jeremy@37signals.com> Date: Wed, 16 Sep 2026 12:24:49 -0700 Subject: [PATCH 20/29] Say that the redacted cable URL is a diagnostic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It carries ticket=[REDACTED] and so opens nothing whatever else it keeps. The connectable URL is the mint's own, which --show-secret returns verbatim and which does not pass through the redactor at all — so dropping the server's other parameters from the diagnostic costs no artifact anyone connects with, including a parameter the mint might start sending. Pinned with a test over --show-secret. --- internal/commands/feed.go | 8 ++++++++ internal/commands/feed_test.go | 22 ++++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/internal/commands/feed.go b/internal/commands/feed.go index 2df918cc4..d725eac02 100644 --- a/internal/commands/feed.go +++ b/internal/commands/feed.go @@ -758,6 +758,14 @@ const redactedTicket = "[REDACTED]" // endpoint to see where a stream would open, and none of the response's own // text. // +// The result is a diagnostic, never a paste target — it carries +// ticket=[REDACTED], so it cannot open a stream whatever else it keeps. The +// connectable URL is the one --show-secret returns, which is the server's +// verbatim and passes through this function not at all. That is what makes +// dropping the server's other parameters here free: no artifact anyone +// connects with loses them, including a protocol or region parameter the +// mint might start sending. +// // The ticket is opaque and the body is not ours to trust, so the display URL // is built rather than edited. Only scheme, host and path survive; the query // is rebuilt from a constant, which is what makes this answerable at all — diff --git a/internal/commands/feed_test.go b/internal/commands/feed_test.go index 0c2f9af0b..e7ee70abd 100644 --- a/internal/commands/feed_test.go +++ b/internal/commands/feed_test.go @@ -523,6 +523,28 @@ func TestEventsTicketShowSecretPrintsItVerbatim(t *testing.T) { assert.Empty(t, decodeFeedEnvelope(t, out).Notice) } +// Dropping the server's other parameters from the redacted rendering is only +// free because nothing connectable goes through it: --show-secret hands back +// the mint's URL verbatim. A protocol, version or region parameter the server +// starts sending must survive that path even though the diagnostic drops it. +func TestShowSecretKeepsEveryParameterTheMintSent(t *testing.T) { + const minted = "wss://chat.example.test/195539477?protocol=v2®ion=iad&ticket=tkt-secret" + app, _, out := setupFeedApp(t, stubRoute{ + method: http.MethodPost, + path: feedTicketPath, + status: http.StatusOK, + body: `{"ticket":"tkt-secret","expires_in":120,"url":"` + minted + `"}`, + }) + + require.NoError(t, executeRecordingCommand(NewEventsCmd(), app, "ticket", "--show-secret")) + + var data struct { + URL string `json:"url"` + } + require.NoError(t, json.Unmarshal(decodeFeedEnvelope(t, out).Data, &data)) + assert.Equal(t, minted, data.URL) +} + func TestRedactTicketURLWithholdsWhatItCannotReduce(t *testing.T) { assert.Equal(t, redactedTicket, redactTicketURL("wss://chat.example.test/195539477", "tkt-secret")) assert.Equal(t, redactedTicket, redactTicketURL("://", "tkt-secret")) From 45c8cd0d23cc709ec68245f7c69eb9959dca2e5d Mon Sep 17 00:00:00 2001 From: Jeremy Daer <jeremy@37signals.com> Date: Wed, 16 Sep 2026 12:24:49 -0700 Subject: [PATCH 21/29] Name v0.18.0 as the release the pin builds on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pseudo-version builds on v0.18.0, the latest release, not on v0.16.0 — which keeps its own sentence as the release that added the template operations. --- API-COVERAGE.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/API-COVERAGE.md b/API-COVERAGE.md index 6faacb4cf..976a74500 100644 --- a/API-COVERAGE.md +++ b/API-COVERAGE.md @@ -51,9 +51,8 @@ Out-of-scope sections are excluded from parity totals and scripts: chatbots (dif **SDK version:** the pin in `go.mod`, with `internal/version/sdk-provenance.json` authoritative for the exact commit. The event-feed operations are unreleased, so that pin is a basecamp-sdk `main` -pseudo-version rather than a release tag; the last tagged release it builds on -is v0.16.0, which added the to-do list template library and asynchronous copy -operations. The +pseudo-version rather than a release tag; it builds on v0.18.0, the latest +release. The command surface below largely dates to the v0.12.0 bump, which added 20 exported Go methods over 13 new backend operations; the extra seven wrapped endpoints that already existed but were reachable only through the raw generated client, From 03edb00b332060b258db768e102e616b078c4148 Mon Sep 17 00:00:00 2001 From: Jeremy Daer <jeremy@37signals.com> Date: Wed, 16 Sep 2026 12:36:56 -0700 Subject: [PATCH 22/29] Capture the redaction verdict, not the ticket it checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Projecting the ticket and comparing it in bats meant the one regression this test exists to catch — redaction failing — was also the one that put a live bearer into the failure output. The comparison now happens inside --jq, so only its boolean leaves the process, and the filter stays null-safe so a failed mint reports its error rather than a jq error. --- e2e/smoke/smoke_event_feed.bats | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/e2e/smoke/smoke_event_feed.bats b/e2e/smoke/smoke_event_feed.bats index 6c576f069..93127362b 100644 --- a/e2e/smoke/smoke_event_feed.bats +++ b/e2e/smoke/smoke_event_feed.bats @@ -32,11 +32,17 @@ setup_file() { assert_output_contains "Invalid --since" } +# The comparison happens inside --jq so that only its verdict is captured. +# Projecting the ticket itself and comparing here would mean that the one +# regression this test exists to catch — redaction failing — is also the one +# that puts a live bearer in the failure output. @test "events ticket mints a redacted ticket" { - run_smoke basecamp events ticket --jq '{ok: .ok, data: {ticket: .data.ticket}}' + run_smoke basecamp events ticket \ + --jq '{ok: .ok, error: .error, ticket_redacted: (.data.ticket == "[REDACTED]"), url_redacted: ((.data.url // "") | contains("REDACTED"))}' assert_success assert_json_value '.ok' 'true' - assert_json_value '.data.ticket' '[REDACTED]' + assert_json_value '.ticket_redacted' 'true' + assert_json_value '.url_redacted' 'true' } @test "inbox polls addressed items" { From ce9c288e3c77bc62a76d8f826f69804f31de5fc6 Mon Sep 17 00:00:00 2001 From: Jeremy Daer <jeremy@37signals.com> Date: Wed, 16 Sep 2026 12:49:34 -0700 Subject: [PATCH 23/29] Vet the redacted URL as rendered, not component by component The scheme was not on the list of components checked, so a response that echoed the ticket there printed it. A list of components to check is a list to be caught out by; the finished string is checked instead, alongside the decoded path, which is the one place an escaped copy could hide from it. --- internal/commands/feed.go | 20 ++++++++++++-------- internal/commands/feed_test.go | 1 + 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/internal/commands/feed.go b/internal/commands/feed.go index d725eac02..f993b82a5 100644 --- a/internal/commands/feed.go +++ b/internal/commands/feed.go @@ -774,18 +774,18 @@ const redactedTicket = "[REDACTED]" // rendering happens to choose (a ticket "abc/def" echoed as "abc%2Fdef" // reads as neither). Userinfo and the fragment are dropped the same way. // -// What is left is compared decoded: url.Parse unescapes the path, so a copy -// escaped into it is still caught, and a URL carrying the ticket in its host -// or path is withheld whole. An unparseable URL, or one with no ticket -// parameter, is withheld too — it is not the shape this was promised. +// What survives is then vetted as the finished string rather than component +// by component, because a list of components to check is a list to be caught +// out by — the scheme was missed exactly that way. Both spellings are tested: +// the rendering, which escapes, and the decoded path, which is where an +// escaped copy would hide from it. Anything still holding the ticket is +// withheld whole, as is a URL that will not parse or carries no ticket +// parameter — that is not the shape this was promised. func redactTicketURL(raw, ticket string) string { parsed, err := url.Parse(raw) if err != nil || !parsed.Query().Has("ticket") { return redactedTicket } - if ticket != "" && (strings.Contains(parsed.Host, ticket) || strings.Contains(parsed.Path, ticket)) { - return redactedTicket - } display := url.URL{ Scheme: parsed.Scheme, @@ -793,5 +793,9 @@ func redactTicketURL(raw, ticket string) string { Path: parsed.Path, RawQuery: url.Values{"ticket": {redactedTicket}}.Encode(), } - return display.String() + rendered := display.String() + if ticket != "" && (strings.Contains(rendered, ticket) || strings.Contains(parsed.Path, ticket)) { + return redactedTicket + } + return rendered } diff --git a/internal/commands/feed_test.go b/internal/commands/feed_test.go index e7ee70abd..8d57973e1 100644 --- a/internal/commands/feed_test.go +++ b/internal/commands/feed_test.go @@ -565,6 +565,7 @@ func TestRedactTicketURLWithholdsATicketEchoedOutsideItsParameter(t *testing.T) "wss://chat.example.test/195539477?ticket=tkt-secret&retry=tkt-secret", "wss://tkt-secret@chat.example.test/195539477?ticket=tkt-secret", "wss://chat.example.test/195539477%2Ftkt-secret?ticket=tkt-secret", + "tkt-secret://chat.example.test/195539477?ticket=tkt-secret", } { assert.NotContains(t, redactTicketURL(raw, "tkt-secret"), "tkt-secret", raw) } From 668781196c17f8cec92db29ceda57c8b4be37abc Mon Sep 17 00:00:00 2001 From: Jeremy Daer <jeremy@37signals.com> Date: Wed, 16 Sep 2026 12:50:07 -0700 Subject: [PATCH 24/29] Refuse an entry flag given an empty value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The documented way to resume is --position "$POSITION". An unset variable makes that an empty string, which read the same as omitting the flag and entered at the present — silently skipping the backlog the caller was resuming. Both lanes now say so instead, following the blank-flag pattern checkins already uses. --- internal/commands/feed.go | 21 +++++++++++++++++++-- internal/commands/feed_test.go | 31 +++++++++++++++++++++++++++++++ internal/commands/inbox.go | 2 +- 3 files changed, 51 insertions(+), 3 deletions(-) diff --git a/internal/commands/feed.go b/internal/commands/feed.go index f993b82a5..51effd9e1 100644 --- a/internal/commands/feed.go +++ b/internal/commands/feed.go @@ -128,7 +128,24 @@ type feedEntry struct { // and an addressed-item id on the inbox. The shape of --since is checked, // never its value: an id the lane has never served is the server's verdict to // give, not ours. -func (e feedEntry) validate(lane feedLane) error { +// +// A flag given an empty value is refused before any of that. The documented +// way to resume is --position "$POSITION", so an unset variable arrives as an +// empty string — indistinguishable here from not passing the flag, which +// enters at the present. Silently skipping the backlog the caller meant to +// resume is the worst available reading of that, so it is not offered. +func (e feedEntry) validate(cmd *cobra.Command, lane feedLane) error { + for _, given := range []struct{ name, value string }{ + {"since", e.since}, + {"position", e.position}, + } { + if cmd.Flags().Changed(given.name) && strings.TrimSpace(given.value) == "" { + return output.ErrUsageHint( + fmt.Sprintf("--%s was given an empty value", given.name), + "An empty value reads the same as omitting the flag, which enters at the present and skips what came before; pass a value or drop the flag") + } + } + switch { case e.since != "" && e.position != "": return output.ErrUsage("--since and --position are mutually exclusive") @@ -553,7 +570,7 @@ event id and refetch the referenced recording before acting on it.`, RunE: func(cmd *cobra.Command, args []string) error { app := appctx.FromContext(cmd.Context()) - if err := entry.validate(eventsLane); err != nil { + if err := entry.validate(cmd, eventsLane); err != nil { return err } if maxPages < 1 { diff --git a/internal/commands/feed_test.go b/internal/commands/feed_test.go index 8d57973e1..5b43659ea 100644 --- a/internal/commands/feed_test.go +++ b/internal/commands/feed_test.go @@ -8,6 +8,8 @@ import ( "net/url" "testing" + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -545,6 +547,35 @@ func TestShowSecretKeepsEveryParameterTheMintSent(t *testing.T) { assert.Equal(t, minted, data.URL) } +// The documented resume form is --position "$POSITION". An unset variable +// makes that an empty string, which used to read as "no entry point given" +// and enter at the present — skipping exactly the backlog the caller was +// trying to resume. Neither lane offers that reading any more. +func TestAnEmptyEntryFlagIsRefusedRatherThanReadAsOmitted(t *testing.T) { + for _, tc := range []struct { + name string + cmd func() *cobra.Command + args []string + }{ + {"events position", NewEventsCmd, []string{"poll", "--position", ""}}, + {"events since", NewEventsCmd, []string{"poll", "--since", ""}}, + {"events position blank", NewEventsCmd, []string{"poll", "--position", " "}}, + {"inbox position", NewInboxCmd, []string{"--position", ""}}, + {"inbox since", NewInboxCmd, []string{"--since", ""}}, + } { + t.Run(tc.name, func(t *testing.T) { + app, transport, _ := setupFeedApp(t) + + err := executeRecordingCommand(tc.cmd(), app, tc.args...) + + cliErr := requireFeedError(t, err) + assert.Equal(t, output.CodeUsage, cliErr.Code) + assert.Contains(t, cliErr.Message, "empty value") + assert.Empty(t, transport.recorded(), "nothing should reach the server") + }) + } +} + func TestRedactTicketURLWithholdsWhatItCannotReduce(t *testing.T) { assert.Equal(t, redactedTicket, redactTicketURL("wss://chat.example.test/195539477", "tkt-secret")) assert.Equal(t, redactedTicket, redactTicketURL("://", "tkt-secret")) diff --git a/internal/commands/inbox.go b/internal/commands/inbox.go index c8a3e41fd..10b72301d 100644 --- a/internal/commands/inbox.go +++ b/internal/commands/inbox.go @@ -54,7 +54,7 @@ deduplicate by addressing_id, never by the event's id.`, RunE: func(cmd *cobra.Command, args []string) error { app := appctx.FromContext(cmd.Context()) - if err := entry.validate(inboxLane); err != nil { + if err := entry.validate(cmd, inboxLane); err != nil { return err } if maxPages < 1 { From 50bc7cba54db6c07cd4478a87e6204f3aa26b935 Mon Sep 17 00:00:00 2001 From: Jeremy Daer <jeremy@37signals.com> Date: Wed, 16 Sep 2026 12:50:07 -0700 Subject: [PATCH 25/29] Write down why events keeps RunE alongside its subcommands The allowlist gained events without the documented exception list gaining it, so the enforced convention and the written one disagreed. events is also the one plural noun in that set, so appending it to a list of commands "without a sibling plural group" would have moved the contradiction rather than fixed it. --- STYLE.md | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/STYLE.md b/STYLE.md index 57933f538..451376104 100644 --- a/STYLE.md +++ b/STYLE.md @@ -42,8 +42,16 @@ existing group noun is not allowed. Shortcut commands without a sibling plural group — `search`, `url`, `recordings`, `timesheet`, `assignments`, `notifications`, `setup`, `completion` -— may have both `RunE` and subcommands. `scripts/check-bare-groups.sh` enforces -this with an allowlist. +— may have both `RunE` and subcommands. + +`events` is the one plural noun in that set. `events <id|url>` reads a single +recording's change history and predates the account-wide event feed that +`events poll` and `events ticket` now serve, so the bare form stays an action +rather than becoming a group: turning it into one would break every existing +`basecamp events <id>` invocation to gain nothing. + +`scripts/check-bare-groups.sh` enforces this with an allowlist; a command added +there belongs in this section too, with the reason it is an exception. ## File Organization From a56a169382235d71e40f1ae4c8e64e956fca4cb8 Mon Sep 17 00:00:00 2001 From: Jeremy Daer <jeremy@37signals.com> Date: Wed, 16 Sep 2026 12:52:35 -0700 Subject: [PATCH 26/29] State the redaction assertion's coverage component by component MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Asserting on the finished string is what makes the class dead — it covers components nobody enumerated, which is how the scheme was missed. But what is known to be covered still has to be readable, so there is now a case per component: scheme, userinfo, host, port, path, escaped path, a second parameter, a repeated one, and the fragment. Each also checks the URL was withheld whole rather than partially scrubbed. --- internal/commands/feed_test.go | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/internal/commands/feed_test.go b/internal/commands/feed_test.go index 5b43659ea..4213d5df8 100644 --- a/internal/commands/feed_test.go +++ b/internal/commands/feed_test.go @@ -589,16 +589,31 @@ func TestRedactTicketURLWithholdsWhatItCannotReduce(t *testing.T) { // response is not ours to trust, and a URL that carries the credential // anywhere else is withheld whole rather than echoed with one parameter // blanked. +// One case per component the URL has, so the exit-point assertion's coverage +// is stated rather than assumed. The point of asserting on the finished +// string is that it also covers components nobody enumerated — the scheme was +// missed by a hand-written list — but a list of what is known to be covered +// is still what a reader needs. func TestRedactTicketURLWithholdsATicketEchoedOutsideItsParameter(t *testing.T) { - for _, raw := range []string{ - "wss://chat.example.test/195539477?ticket=tkt-secret#tkt-secret", - "wss://chat.example.test/tkt-secret?ticket=tkt-secret", - "wss://chat.example.test/195539477?ticket=tkt-secret&retry=tkt-secret", - "wss://tkt-secret@chat.example.test/195539477?ticket=tkt-secret", - "wss://chat.example.test/195539477%2Ftkt-secret?ticket=tkt-secret", - "tkt-secret://chat.example.test/195539477?ticket=tkt-secret", + for _, tc := range []struct{ component, raw string }{ + {"scheme", "tkt-secret://chat.example.test/195539477?ticket=tkt-secret"}, + {"userinfo", "wss://tkt-secret@chat.example.test/195539477?ticket=tkt-secret"}, + {"host", "wss://tkt-secret.example.test/195539477?ticket=tkt-secret"}, + {"port", "wss://chat.example.test:8443/tkt-secret?ticket=tkt-secret"}, + {"path", "wss://chat.example.test/tkt-secret?ticket=tkt-secret"}, + {"escaped path", "wss://chat.example.test/195539477%2Ftkt-secret?ticket=tkt-secret"}, + {"second query parameter", "wss://chat.example.test/195539477?ticket=tkt-secret&retry=tkt-secret"}, + {"repeated ticket parameter", "wss://chat.example.test/195539477?ticket=tkt-secret&ticket=tkt-secret"}, + {"fragment", "wss://chat.example.test/195539477?ticket=tkt-secret#tkt-secret"}, } { - assert.NotContains(t, redactTicketURL(raw, "tkt-secret"), "tkt-secret", raw) + t.Run(tc.component, func(t *testing.T) { + rendered := redactTicketURL(tc.raw, "tkt-secret") + assert.NotContains(t, rendered, "tkt-secret") + // Withheld whole, never a partially scrubbed URL. + if rendered != redactedTicket { + assert.NotContains(t, rendered, "tkt-") + } + }) } } From 3b6f8f358cebe2d1201626616b14f1b3eed6848f Mon Sep 17 00:00:00 2001 From: Jeremy Daer <jeremy@37signals.com> Date: Wed, 16 Sep 2026 13:01:57 -0700 Subject: [PATCH 27/29] Treat every ticket the response presents as the secret MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The assertion checked the body's ticket field only. A response whose URL carries a different ticket than its body makes the URL's the operative bearer — the one that would actually open the stream — and it survived in the path because nothing was looking for it. The check now runs over the body's ticket and the URL's own ticket values alike. --- internal/commands/feed.go | 24 ++++++++++++++++++------ internal/commands/feed_test.go | 18 ++++++++++++++++++ 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/internal/commands/feed.go b/internal/commands/feed.go index 51effd9e1..a6c9a0602 100644 --- a/internal/commands/feed.go +++ b/internal/commands/feed.go @@ -795,12 +795,21 @@ const redactedTicket = "[REDACTED]" // by component, because a list of components to check is a list to be caught // out by — the scheme was missed exactly that way. Both spellings are tested: // the rendering, which escapes, and the decoded path, which is where an -// escaped copy would hide from it. Anything still holding the ticket is -// withheld whole, as is a URL that will not parse or carries no ticket -// parameter — that is not the shape this was promised. +// escaped copy would hide from it. +// +// Every ticket the response presents counts as the secret, not just the +// body's field. If the URL's own ticket parameter disagrees with the body, +// the URL's is the one that would open the stream, so it is the one that +// must not survive. Anything still holding any of them is withheld whole, as +// is a URL that will not parse or carries no ticket parameter — that is not +// the shape this was promised. func redactTicketURL(raw, ticket string) string { parsed, err := url.Parse(raw) - if err != nil || !parsed.Query().Has("ticket") { + if err != nil { + return redactedTicket + } + query := parsed.Query() + if !query.Has("ticket") { return redactedTicket } @@ -811,8 +820,11 @@ func redactTicketURL(raw, ticket string) string { RawQuery: url.Values{"ticket": {redactedTicket}}.Encode(), } rendered := display.String() - if ticket != "" && (strings.Contains(rendered, ticket) || strings.Contains(parsed.Path, ticket)) { - return redactedTicket + + for _, secret := range append([]string{ticket}, query["ticket"]...) { + if secret != "" && (strings.Contains(rendered, secret) || strings.Contains(parsed.Path, secret)) { + return redactedTicket + } } return rendered } diff --git a/internal/commands/feed_test.go b/internal/commands/feed_test.go index 4213d5df8..339e237b2 100644 --- a/internal/commands/feed_test.go +++ b/internal/commands/feed_test.go @@ -589,6 +589,24 @@ func TestRedactTicketURLWithholdsWhatItCannotReduce(t *testing.T) { // response is not ours to trust, and a URL that carries the credential // anywhere else is withheld whole rather than echoed with one parameter // blanked. +// The secret is every ticket the response presents. When the URL's own +// ticket parameter disagrees with the body's field, the URL's is the one that +// would open the stream — so checking only the body's would clear a display +// URL that still carries the operative bearer. +func TestRedactTicketURLTreatsTheURLsOwnTicketAsSecretToo(t *testing.T) { + assert.Equal(t, redactedTicket, + redactTicketURL("wss://chat.example.test/url-secret?ticket=url-secret", "body-secret")) + + assert.Equal(t, redactedTicket, + redactTicketURL("wss://url-secret.example.test/cable?ticket=url-secret", "body-secret")) + + // Agreeing values still render: this widens what counts as the secret, + // it does not withhold every URL. + assert.Equal(t, + "wss://chat.example.test/195539477?ticket="+url.QueryEscape(redactedTicket), + redactTicketURL("wss://chat.example.test/195539477?ticket=tkt-secret", "tkt-secret")) +} + // One case per component the URL has, so the exit-point assertion's coverage // is stated rather than assumed. The point of asserting on the finished // string is that it also covers components nobody enumerated — the scheme was From aa3e458c1c4388ee8d467d0fe2ded9e61bab9448 Mon Sep 17 00:00:00 2001 From: Jeremy Daer <jeremy@37signals.com> Date: Wed, 16 Sep 2026 13:15:21 -0700 Subject: [PATCH 28/29] Check the redacted URL's decoding, not its components url.URL.String escapes per component and picks the encoding itself, so a search for the raw secret misses whichever component it re-spelled: the scheme got through that way, then a non-ASCII host. Decoding the finished string normalizes every component at once, including the ones nobody has named, and replaces the hand-placed path check rather than joining it. --- internal/commands/feed.go | 30 ++++++++++++++++++++---------- internal/commands/feed_test.go | 13 +++++++++++++ 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/internal/commands/feed.go b/internal/commands/feed.go index a6c9a0602..d6b0cfe43 100644 --- a/internal/commands/feed.go +++ b/internal/commands/feed.go @@ -791,17 +791,23 @@ const redactedTicket = "[REDACTED]" // rendering happens to choose (a ticket "abc/def" echoed as "abc%2Fdef" // reads as neither). Userinfo and the fragment are dropped the same way. // -// What survives is then vetted as the finished string rather than component -// by component, because a list of components to check is a list to be caught -// out by — the scheme was missed exactly that way. Both spellings are tested: -// the rendering, which escapes, and the decoded path, which is where an -// escaped copy would hide from it. +// What survives is then vetted at the exit point, against two axes that each +// cost a round of this to find. // -// Every ticket the response presents counts as the secret, not just the +// Which string is secret: every ticket the response presents, not just the // body's field. If the URL's own ticket parameter disagrees with the body, -// the URL's is the one that would open the stream, so it is the one that -// must not survive. Anything still holding any of them is withheld whole, as -// is a URL that will not parse or carries no ticket parameter — that is not +// the URL's is the one that would open the stream. +// +// Which spelling to look for: the rendering and its percent-decoding, not the +// components one at a time. url.URL.String escapes per component and picks +// the encoding itself — an escaped path, a non-ASCII host — so a search for +// the raw secret against a list of components misses whichever one it +// happened to re-spell, which is how the scheme and then the host got +// through. Decoding the finished string normalizes that away for every +// component at once, including the ones nobody has named. +// +// Anything still holding any ticket is withheld whole, as is a URL that will +// not parse, will not decode, or carries no ticket parameter — that is not // the shape this was promised. func redactTicketURL(raw, ticket string) string { parsed, err := url.Parse(raw) @@ -820,9 +826,13 @@ func redactTicketURL(raw, ticket string) string { RawQuery: url.Values{"ticket": {redactedTicket}}.Encode(), } rendered := display.String() + decoded, err := url.PathUnescape(rendered) + if err != nil { + return redactedTicket + } for _, secret := range append([]string{ticket}, query["ticket"]...) { - if secret != "" && (strings.Contains(rendered, secret) || strings.Contains(parsed.Path, secret)) { + if secret != "" && (strings.Contains(rendered, secret) || strings.Contains(decoded, secret)) { return redactedTicket } } diff --git a/internal/commands/feed_test.go b/internal/commands/feed_test.go index 339e237b2..171ccadd7 100644 --- a/internal/commands/feed_test.go +++ b/internal/commands/feed_test.go @@ -589,6 +589,18 @@ func TestRedactTicketURLWithholdsWhatItCannotReduce(t *testing.T) { // response is not ours to trust, and a URL that carries the credential // anywhere else is withheld whole rather than echoed with one parameter // blanked. +// url.URL.String picks its own encoding per component, so a search for the +// raw secret misses whichever component it re-spelled — the scheme, then the +// host. The rendering is decoded before it is checked, which normalizes that +// for every component at once rather than one more at a time. +func TestRedactTicketURLWithholdsATicketTheRenderingRespells(t *testing.T) { + assert.Equal(t, redactedTicket, + redactTicketURL("wss://\u79d8\u5bc6.example/cable?ticket=\u79d8\u5bc6", "\u79d8\u5bc6")) + + assert.Equal(t, redactedTicket, + redactTicketURL("wss://chat.example.test/a\u79d8\u5bc6b?ticket=\u79d8\u5bc6", "\u79d8\u5bc6")) +} + // The secret is every ticket the response presents. When the URL's own // ticket parameter disagrees with the body's field, the URL's is the one that // would open the stream — so checking only the body's would clear a display @@ -620,6 +632,7 @@ func TestRedactTicketURLWithholdsATicketEchoedOutsideItsParameter(t *testing.T) {"port", "wss://chat.example.test:8443/tkt-secret?ticket=tkt-secret"}, {"path", "wss://chat.example.test/tkt-secret?ticket=tkt-secret"}, {"escaped path", "wss://chat.example.test/195539477%2Ftkt-secret?ticket=tkt-secret"}, + {"escaped host", "wss://tkt-secret%2Eexample.test/cable?ticket=tkt-secret"}, {"second query parameter", "wss://chat.example.test/195539477?ticket=tkt-secret&retry=tkt-secret"}, {"repeated ticket parameter", "wss://chat.example.test/195539477?ticket=tkt-secret&ticket=tkt-secret"}, {"fragment", "wss://chat.example.test/195539477?ticket=tkt-secret#tkt-secret"}, From dbbff8e04444e9273bd9a34e41b6ddf0bbda199e Mon Sep 17 00:00:00 2001 From: Jeremy Daer <jeremy@37signals.com> Date: Wed, 16 Sep 2026 13:15:28 -0700 Subject: [PATCH 29/29] Refuse a list filter given an empty value An unset variable marks the flag provided but leaves it carrying nothing, and dropping it silently widened the request: --buckets to account-wide, and --exclude-performers to no loop guard at all, so an agent starts reacting to its own activity. The guard reads the flags the caller set rather than a list of names to keep in step, so a filter added later is covered without being enrolled. A trailing comma after a real value is still fine. --- internal/commands/feed.go | 31 +++++++++++++++++++++++++++++++ internal/commands/feed_test.go | 31 +++++++++++++++++++++++++++++++ internal/commands/inbox.go | 3 +++ 3 files changed, 65 insertions(+) diff --git a/internal/commands/feed.go b/internal/commands/feed.go index d6b0cfe43..d03240926 100644 --- a/internal/commands/feed.go +++ b/internal/commands/feed.go @@ -9,6 +9,7 @@ import ( "strings" "github.com/spf13/cobra" + "github.com/spf13/pflag" "github.com/basecamp/basecamp-sdk/go/pkg/basecamp" @@ -217,6 +218,33 @@ func feedActorTypes(values []string) ([]string, error) { // trimFilter drops empty entries a trailing comma leaves behind, so // --types "message.created," is the one-type filter it reads as rather than a // filter carrying an empty type the server rejects. +// refuseEmptyFilterFlags rejects a list filter that was given but carries +// nothing. A trailing comma leaving an empty entry is fine — trimFilter drops +// it — but a flag whose every entry normalizes away came from an unset +// variable, and silently dropping it widens the request instead of failing. +// On --exclude-performers that means losing the loop guard that keeps an +// agent from reacting to its own activity. +// +// It reads the flags the caller actually set rather than a list of names to +// keep in step, so a filter added later is covered without being enrolled. +func refuseEmptyFilterFlags(cmd *cobra.Command) error { + empty := "" + cmd.Flags().Visit(func(f *pflag.Flag) { + if empty != "" || f.Value.Type() != "stringSlice" { + return + } + if values, err := cmd.Flags().GetStringSlice(f.Name); err == nil && len(trimFilter(values)) == 0 { + empty = f.Name + } + }) + if empty != "" { + return output.ErrUsageHint( + fmt.Sprintf("--%s was given an empty value", empty), + "An empty value reads the same as omitting the filter, which widens the request; pass a value or drop the flag") + } + return nil +} + func trimFilter(values []string) []string { if len(values) == 0 { return nil @@ -576,6 +604,9 @@ event id and refetch the referenced recording before acting on it.`, if maxPages < 1 { return output.ErrUsage("--max-pages must be at least 1") } + if err := refuseEmptyFilterFlags(cmd); err != nil { + return err + } bucketIDs, err := feedIDs(trimFilter(buckets), "--buckets") if err != nil { diff --git a/internal/commands/feed_test.go b/internal/commands/feed_test.go index 171ccadd7..298e7bee5 100644 --- a/internal/commands/feed_test.go +++ b/internal/commands/feed_test.go @@ -547,6 +547,37 @@ func TestShowSecretKeepsEveryParameterTheMintSent(t *testing.T) { assert.Equal(t, minted, data.URL) } +// An unset variable in a list filter marks the flag provided but leaves it +// carrying nothing. Dropping it silently widens the request — and on +// --exclude-performers it drops the guard that keeps an agent from reacting +// to its own activity, which is the one filter whose absence is a loop. +func TestAnEmptyFilterFlagIsRefusedRatherThanWidenTheRequest(t *testing.T) { + for _, tc := range []struct { + name string + cmd func() *cobra.Command + args []string + }{ + {"exclude-performers", NewEventsCmd, []string{"poll", "--since", "now", "--exclude-performers", ""}}, + {"buckets", NewEventsCmd, []string{"poll", "--since", "now", "--buckets", ""}}, + {"types", NewEventsCmd, []string{"poll", "--since", "now", "--types", ""}}, + {"actor-types blank", NewEventsCmd, []string{"poll", "--since", "now", "--actor-types", " "}}, + {"bare comma", NewEventsCmd, []string{"poll", "--since", "now", "--creators", ","}}, + {"inbox reasons", NewInboxCmd, []string{"--since", "now", "--reasons", ""}}, + {"inbox buckets", NewInboxCmd, []string{"--since", "now", "--buckets", ""}}, + } { + t.Run(tc.name, func(t *testing.T) { + app, transport, _ := setupFeedApp(t) + + err := executeRecordingCommand(tc.cmd(), app, tc.args...) + + cliErr := requireFeedError(t, err) + assert.Equal(t, output.CodeUsage, cliErr.Code) + assert.Contains(t, cliErr.Message, "empty value") + assert.Empty(t, transport.recorded(), "nothing should reach the server") + }) + } +} + // The documented resume form is --position "$POSITION". An unset variable // makes that an empty string, which used to read as "no entry point given" // and enter at the present — skipping exactly the backlog the caller was diff --git a/internal/commands/inbox.go b/internal/commands/inbox.go index 10b72301d..6615ac17a 100644 --- a/internal/commands/inbox.go +++ b/internal/commands/inbox.go @@ -60,6 +60,9 @@ deduplicate by addressing_id, never by the event's id.`, if maxPages < 1 { return output.ErrUsage("--max-pages must be at least 1") } + if err := refuseEmptyFilterFlags(cmd); err != nil { + return err + } bucketIDs, err := feedIDs(trimFilter(buckets), "--buckets") if err != nil {