From c39164e3bed6dddbe75e9a02e13ba4a3781f1953 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Tue, 4 Aug 2026 11:28:10 -0700 Subject: [PATCH 1/3] Correct the coverage claim, and file two upload-versions communiques MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit API-COVERAGE.md asserted "100% coverage of tracked in-scope API (184/184 endpoints)". That was false, and the matrix had no way to say so: with only ✅ and ⏭️ available, a partly-covered section had to be recorded as fully covered. Add a narrow third status — ⚠️ blocked, meaning the CLI cannot faithfully cover an endpoint for a reason outside the CLI, and the row must name the blocker. The one gap is GET /uploads/:id/versions.json. The command is written; the SDK is what blocks it. UploadsService.ListVersions types the response as []Upload, but the API returns version events (action, details, recording_id), so a caller gets Upload structs with blank title/filename/status and an id that is the event id and does not resolve as an upload. Held rather than shipped, because shipping it means shipping wrong data. COMMUNIQUE-sdk-upload-versions-response.md reports that to the SDK team, with the live response, the bc3 controller that produces it, and a suggested UploadVersion type. COMMUNIQUE-upload-versions-api.md addresses #404 to BC3 Rails, and corrects the premise it was drafted on. PUT /uploads/:id.json does not accept attachable_sgid: UploadsController#update permits only base_name and description, and uploadable_params is wired to create alone — so the parameter is silently dropped, not rejected. Replacement does exist (stage an Upload, then POST .../uploads/:id/versions swaps the recording's recordable), but it is undocumented, redirects instead of rendering JSON, takes form-shaped array params, and stages via multipart rather than attachable_sgid. The communique asks for an API-reachable shape and offers two. Neither the SDK nor the bc3 repository was modified. Refs #404 --- API-COVERAGE.md | 29 +++- COMMUNIQUE-sdk-upload-versions-response.md | 126 ++++++++++++++ COMMUNIQUE-upload-versions-api.md | 188 +++++++++++++++++++++ 3 files changed, 339 insertions(+), 4 deletions(-) create mode 100644 COMMUNIQUE-sdk-upload-versions-response.md create mode 100644 COMMUNIQUE-upload-versions-api.md diff --git a/API-COVERAGE.md b/API-COVERAGE.md index 20307708..6bb09c87 100644 --- a/API-COVERAGE.md +++ b/API-COVERAGE.md @@ -6,12 +6,29 @@ Coverage of Basecamp 3 API endpoints. Source: [bc3-api/sections](https://github. | Status | Sections | Endpoints | |--------|----------|-----------| -| ✅ Implemented | 50 | 184 | +| ✅ Implemented | 49 | 183 | +| ⚠️ Blocked | 1 | 1 | | ⏭️ Out of scope | 4 | 12 | | **Total tracked** | **54** | **196** | -**100% coverage of tracked in-scope API** (184/184 endpoints). This is not a -complete bc-api parity figure. The five BC5 sections introduced by bc-api#410 +**183 of 184 tracked in-scope endpoints.** The one gap is `GET +/uploads/:id/versions.json` in the `uploads` section, and it is not a missing +command — the command is written. The SDK's `UploadsService.ListVersions` +decodes the response with the wrong type: the API returns version *events* +(`action`, `details`, `recording_id`), the OpenAPI spec models them as +`[]Upload`, so a caller gets Upload structs with blank `title`/`filename`/ +`status` and an `id` that is the event id and does not resolve as an upload. +Shipping a command on that would mean shipping wrong data, so it is held. See +[COMMUNIQUE-sdk-upload-versions-response.md](COMMUNIQUE-sdk-upload-versions-response.md). + +This file previously read "100% coverage of tracked in-scope API (184/184)". +That was wrong, and the matrix had no way to say so — with only ✅ and ⏭️ +available, a partly-covered section had to be recorded as fully covered. Hence +the third status above. It is deliberately narrow: ⚠️ means the CLI cannot +faithfully cover an endpoint for a reason outside the CLI, and it names the +blocker. + +This is not a complete bc-api parity figure. The five BC5 sections introduced by bc-api#410 that were previously untracked — `my_bookmarks`, `drafts`, `my_notes`, `calendars`, and `question_reminders` — are now tracked and implemented. The pinned SDK's `EverythingService` is fully reached — see [Account-wide @@ -152,6 +169,10 @@ Design discussion: basecamp/basecamp-cli#585. Contract and invariants: The **Since** column tags each row with the Basecamp version that introduced its section: `BC4` for sections that shipped before Basecamp 5, `BC5` for sections introduced in Basecamp 5. If a BC5 release adds endpoints to an existing BC4 section, split them into a new row tagged `BC5` rather than bumping the BC4 row's `Endpoints` count — that keeps the column unambiguous per row. Column dropped post-BC4 decommission. +**Status** is one of ✅ implemented, ⏭️ out of scope, or ⚠️ blocked — the CLI +cannot faithfully cover at least one endpoint for a reason outside the CLI. A +⚠️ row must name its blocker in Notes. + | Section | Endpoints | CLI Command | Status | Since | Priority | Notes | |---------|-----------|-------------|--------|-------|----------|-------| | **Core** | @@ -190,7 +211,7 @@ The **Since** column tags each row with the Basecamp version that introduced its | search | 2 | `search` | ✅ | BC4 | - | Full-text search + metadata. Filters: `--project`/`--in`, `--type`, `--creator`, `--since` (BC5-only), `--file-type`, `--exclude-chat`. Metadata lists recording/file search types | | recordings | 4 | `recordings` | ✅ | BC4 | - | Browse by type/status, trash/archive/restore | | **Files & Documents** | -| uploads | 8 | `files`, `uploads` | ✅ | BC4 | - | list, show, create. Create supports `--visible-to-clients` (root vault only) | +| uploads | 8 | `files`, `uploads` | ⚠️ | BC4 | - | list, show, create, update, download; trash/archive/restore go through `recordings`. Create supports `--visible-to-clients` (root vault only). **Blocked:** `GET /uploads/:id/versions.json` — the SDK's `ListVersions` types the response as `[]Upload`, but the API returns version *events*, so the data comes back blank and misleading. Command is written and held on `feat/files-versions`; see [COMMUNIQUE-sdk-upload-versions-response.md](COMMUNIQUE-sdk-upload-versions-response.md). Replacing an upload's *file* is a separate BC3 gap — see [COMMUNIQUE-upload-versions-api.md](COMMUNIQUE-upload-versions-api.md) | | vaults | 8 | `files`, `vaults` | ✅ | BC4 | - | list, show, create | | documents | 8 | `files`, `docs` | ✅ | BC4 | - | list, show, create, update. Create supports `--subscribe`/`--no-subscribe`, `--visible-to-clients` (root vault only) | | attachments | 1 | `uploads`, `attachments` | ✅ | BC4 | - | Upload via `attach`; list embedded attachments via `attachments list` (parses `` from content) | diff --git a/COMMUNIQUE-sdk-upload-versions-response.md b/COMMUNIQUE-sdk-upload-versions-response.md new file mode 100644 index 00000000..af676acd --- /dev/null +++ b/COMMUNIQUE-sdk-upload-versions-response.md @@ -0,0 +1,126 @@ +# Communique: `UploadsService.ListVersions` Decodes Version Events as Uploads + +**From:** Basecamp CLI team +**To:** basecamp-sdk (Go) +**Re:** `ListUploadVersionsResponseContent` is modeled as `[]Upload`; the API +returns version events +**Affects:** SDK v0.12.0 (current pin) — `pkg/basecamp/vaults.go:956`, +`pkg/generated/client.gen.go:1625` +**Context:** CLI branch `feat/files-versions`, held rather than merged because +of this + +--- + +## The defect + +`GET /uploads/{id}/versions.json` returns a list of **version events**. The +OpenAPI spec models it as a list of **uploads**: + +```go +// pkg/generated/client.gen.go:1625 +type ListUploadVersionsResponseContent = []Upload +``` + +`UploadsService.ListVersions` then decodes each element with +`uploadFromGenerated`, so events are silently coerced into `Upload` structs. +Nothing errors — the fields simply do not line up. + +## Evidence + +`bc3-api/sections/uploads.md` documents the response as events, and a live +`GET /uploads/10165243019/versions.json` against production agrees: + +```json +[ + { + "id": 16850997498, + "recording_id": 10165243019, + "action": "active", + "details": { "delivered_recipient_ids": [], "status_was": "drafted" }, + "created_at": "2026-08-04T17:15:01.261Z", + "creator": { "id": 39024674, "name": "…" } + } +] +``` + +The bc3 implementation confirms it — +`Uploads::VersionsController#index` renders +`@recording.version_events_since_publication.reverse_chronologically`. + +## What a caller gets today + +Running the same request through `ListVersions` yields an `Upload` that is +mostly empty, and worse, one that is confidently wrong: + +```json +{ + "id": 16850997498, + "created_at": "2026-08-04T17:15:01.261Z", + "creator": { "id": 39024674, "name": "…" }, + "title": "", + "filename": "", + "status": "", + "content_type": "", + "byte_size": 0, + "download_url": "", + "url": "", + "app_url": "", + "updated_at": "0001-01-01T00:00:00Z" +} +``` + +Three distinct problems, in increasing order of harm: + +1. **Dropped fields.** `action`, `details` and `recording_id` — the only fields + that say *what happened* — have nowhere to land and are discarded. +2. **Empty fields that read as data.** `title`, `filename`, `status`, + `download_url`, `byte_size` are zero values, not absent ones. A caller + rendering a versions table gets blank cells and no signal that the blanks + are an artifact. +3. **A misleading `id`.** `16850997498` is the *event* id. Typed as an `Upload`, + it invites `Uploads().Get(ctx, 16850997498)`, which 404s. The real upload id + is in the discarded `recording_id`. + +## Suggested fix + +Model the response as its own type rather than reusing `Upload`: + +```go +// UploadVersion is one entry in an upload's change history. +type UploadVersion struct { + ID int64 // the event id, not an upload id + RecordingID int64 // the upload this version belongs to + Action string // e.g. "active", "blob_changed" + Details map[string]any + CreatedAt time.Time + Creator Person +} + +type UploadVersionListResult struct { + Versions []UploadVersion + Meta ListMeta +} +``` + +This is a breaking change to `UploadVersionListResult.Versions`, but the +current element type carries no usable data, so we do not expect real +callers to be relying on it. + +The spec is the root cause — `ListUploadVersionsResponseContent = []Upload` +comes from the OpenAPI description, so the fix likely belongs upstream of the +generator, with the hand-written service following. + +## What it unblocks + +CLI PR for `basecamp files versions` is written, tested and complete, and is +held unmerged solely on this. As soon as `ListVersions` returns the event +shape, the command ships: `--limit`/`--page`/`--all` semantics, catalog entry, +surface snapshot and smoke coverage are already in place. + +It also matters for `API-COVERAGE.md`. `ListVersions` is the one uploads +endpoint the CLI does not cover, and the reason is this type — not a missing +command. + +--- + +**Note:** no changes were made to the SDK repository. Reporting only. diff --git a/COMMUNIQUE-upload-versions-api.md b/COMMUNIQUE-upload-versions-api.md new file mode 100644 index 00000000..9a2d89cf --- /dev/null +++ b/COMMUNIQUE-upload-versions-api.md @@ -0,0 +1,188 @@ +# Communique: Replacing an Upload's File Over the API + +**From:** Basecamp CLI team +**To:** BC3 Rails development +**Re:** Exposing upload file replacement (new versions) as a documented API +**Context:** CLI issue [#404](https://github.com/basecamp/basecamp-cli/issues/404) — replacing a +signed release binary in place + +--- + +## The ask in one line + +Basecamp can already replace an upload's file in place, keeping the recording +id and accumulating versions. The web app does it. The API cannot, and it is +the one thing standing between a release script and a stable download URL. + +## The use case + +A release pipeline uploads a signed Windows `.exe` to a Docs & Files folder and +publishes the link. Next release, it needs the *same* link to serve the new +binary. Today the only API-reachable move is `POST /vaults/:id/uploads.json`, +which creates a **new** recording with a **new** id and a **new** URL. So the +folder accumulates `basecamp-v0.8.1.exe`, `basecamp-v0.8.2.exe`, … and every +previously published link points at a stale build. + +What we want instead is one durable recording whose file is replaced on each +release — exactly what `GET /uploads/:id/versions.json` already describes: + +> Each version event represents a file replacement. + +The read side of this feature is public. The write side is not. + +## What already exists (and why it isn't reachable) + +Replacement is implemented and shipping. It's a two-step flow: + +**1. Stage the file** — `POST /uploads/stage` (`Uploads::StageController#create`): + +```ruby +def create + @upload = Upload.create!(file: params[:file]) + render :show +end +``` + +Creates an `Upload` with no recording, and renders its signed GID. + +**2. Swap it in** — `POST /buckets/:bucket/uploads/:upload_id/versions` +(`Uploads::VersionsController#create`): + +```ruby +def create + Upload.transaction do + subscribers = find_subscribers + @upload.update! base_name: upload_params[:base_name], description: upload_params[:description] + @recording.update! recordable: @upload + @recording.notify subscribers, event: @recording.version_events_since_publication.last + @recording.change_subscribers added: subscribers + end + + redirect_to @recording +end +``` + +The recording keeps its id; `Upload#recorded_as` tracks a `blob_changed` event, +which is what the versions index later lists. This is precisely the behavior +#404 needs. + +Four things keep it out of reach for an API client: + +1. **Undocumented.** Neither route appears in + [bc3-api](https://github.com/basecamp/bc3-api). `sections/uploads.md` + documents `GET /uploads/2/versions.json` and stops there. +2. **No JSON response.** `create` ends in `redirect_to @recording` with no + `respond_to`, so a `.json` request gets a `302`, not the updated upload. +3. **Form-shaped params.** It reads `params.require(:uploads).first`, i.e. an + array of `{sgid, base_name, description}` — shaped for the composer's + multi-file form, not for a single-resource API call. +4. **Different upload convention.** Staging takes a raw multipart + `params[:file]`. Every documented API upload path instead goes through + `POST /attachments.json` and carries the result as `attachable_sgid`. + `Upload::Creation` already accepts `attachable_sgid` — but + `UploadsController` only wires it up on `create`: + + ```ruby + before_action :set_new_upload, only: :create + + def update + @recording.update! recordable: @upload.changing(upload_params), status: status_param + end + + def upload_params + params[:upload]&.permit(:base_name, :description) || {} + end + + def uploadable_params + params.permit(:attachable_sgid, :file) + end + ``` + + So an API client that PUTs `attachable_sgid` to `/uploads/2.json` gets a + `200 OK` and **no replacement** — the parameter is silently dropped, not + rejected. That silence is its own small problem: the caller has no way to + tell a no-op from a success. + +## Two possible shapes + +**Option A — accept `attachable_sgid` on update.** Smallest surface, matches the +convention every other API upload path already uses: + +``` +PUT /uploads/2.json +{ "attachable_sgid": "BAh7CEk...", "base_name": "basecamp-windows-amd64" } +``` + +Permit `:attachable_sgid` in `upload_params` for API requests, resolve it the +way `Upload::Creation` does, and swap the recordable when present. Omitting it +keeps today's metadata-only behavior, so this is additive. + +One wrinkle worth deciding deliberately: `Uploads::VersionsController#create` +does subscriber notification and `change_subscribers` work that +`UploadsController#update` does not. If replacement moves to `update`, that +notification path needs to come along, or the API's replacements will be +quieter than the web app's. + +**Option B — document a JSON `POST /uploads/2/versions.json`.** Keeps +replacement where it already lives, alongside the versions index a client would +poll afterward: + +``` +POST /buckets/1/uploads/2/versions.json +{ "attachable_sgid": "BAh7CEk...", "base_name": "basecamp-windows-amd64" } +``` + +Needs a `respond_to` returning the upload JSON instead of a redirect, and a +single-object param shape next to the existing array form. + +We have no stake in which. Option A is less for a client to learn; Option B +keeps the notification behavior where it is and reads more honestly as "add a +version." + +## A related read-side note + +`GET /uploads/:id/versions.json` returns **version events**, not uploads: + +```json +[ + { + "id": 16850997498, + "recording_id": 10165243019, + "action": "active", + "details": { "status_was": "drafted" }, + "created_at": "2026-08-04T17:15:01.261Z", + "creator": { "id": 39024674, "name": "…" } + } +] +``` + +The documented example in `sections/uploads.md` agrees. The OpenAPI spec behind +the Go SDK does not — it models the response as `[]Upload`, which is a separate +report to the SDK team. Flagging it here only because whichever write shape you +pick, a client will want the response and the versions index to describe the +same thing. If a version event ever carried the replaced file's `filename`, +`byte_size` and `download_url`, "show me this file's history" would become +answerable in one call. + +## What this unblocks + +- **#404** — a release script that replaces a binary in place, so a published + link keeps working across releases +- **CLI** — `basecamp files versions` becomes a listing of changes the CLI can + itself produce, rather than a read-only window onto web-app activity +- **Any integration** that mirrors an external artifact into Basecamp and wants + one stable recording instead of a growing pile of near-duplicate uploads + +## Impact on existing behavior + +Additive under either option. The web composer's flow is untouched; today's +`PUT /uploads/:id.json` keeps working unchanged when the new parameter is +absent. + +--- + +**Question for BC3:** is the staged-upload requirement +(`require_staged_upload` — the replacement Upload must have no recordings yet) +essential to how versions are tracked, or an artifact of the composer flow? It +shapes whether an API client stages first or hands over an `attachable_sgid` +directly. From e45840806a19cee7965f5ae7bd27b730ef23720d Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Tue, 4 Aug 2026 12:21:35 -0700 Subject: [PATCH 2/3] Don't equate every version event with a file replacement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review caught the communique quoting bc3-api's "each version event represents a file replacement" approvingly, then pasting a response example whose action is "active" with status_was: drafted — a publication transition, not a new file. The endpoint renders version_events_since_publication, i.e. the whole event stream. Replacement is the blob_changed subset, emitted by Upload#recorded_as → track_blob_change when the new recordable's blob differs. Say so, in both places the document leans on the endpoint, and ask BC3 not to model the listing as binary revisions. Also tighten the "what this unblocks" bullet: files versions can only observe web-app replacements today, and would list ones the CLI itself caused only if this ships. --- COMMUNIQUE-upload-versions-api.md | 42 ++++++++++++++++++++----------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/COMMUNIQUE-upload-versions-api.md b/COMMUNIQUE-upload-versions-api.md index 9a2d89cf..37e38bdf 100644 --- a/COMMUNIQUE-upload-versions-api.md +++ b/COMMUNIQUE-upload-versions-api.md @@ -24,11 +24,20 @@ folder accumulates `basecamp-v0.8.1.exe`, `basecamp-v0.8.2.exe`, … and every previously published link points at a stale build. What we want instead is one durable recording whose file is replaced on each -release — exactly what `GET /uploads/:id/versions.json` already describes: - -> Each version event represents a file replacement. - -The read side of this feature is public. The write side is not. +release. `GET /uploads/:id/versions.json` already exposes the *record* of that +happening — the read side of this feature is public. The write side is not. + +One clarification on that endpoint, since the rest of this document leans on it: +it returns `version_events_since_publication`, which is every recording event, +not only replacements. bc3-api currently describes it as "each version event +represents a file replacement," which overstates it — the response example +below carries an `"action": "active"` event with `"status_was": "drafted"`, +a publication transition rather than a new file. File replacement is the +`blob_changed` subset, emitted by `Upload#recorded_as` → `track_blob_change` +when the new recordable's blob differs from the previous one. Whatever write +shape you pick, please don't treat the endpoint as a list of binary revisions — +it isn't one today, and a client filtering for replacements has to select +`blob_changed` itself. ## What already exists (and why it isn't reachable) @@ -156,20 +165,25 @@ version." ] ``` -The documented example in `sections/uploads.md` agrees. The OpenAPI spec behind -the Go SDK does not — it models the response as `[]Upload`, which is a separate -report to the SDK team. Flagging it here only because whichever write shape you -pick, a client will want the response and the versions index to describe the -same thing. If a version event ever carried the replaced file's `filename`, -`byte_size` and `download_url`, "show me this file's history" would become -answerable in one call. +Note the `action` here is `active`, not `blob_changed` — as above, this listing +is the full event stream since publication, and replacements are one kind of +entry in it. + +The documented example in `sections/uploads.md` agrees that these are events. +The OpenAPI spec behind the Go SDK does not — it models the response as +`[]Upload`, which is a separate report to the SDK team. Flagging it here only +because whichever write shape you pick, a client will want the response and the +versions index to describe the same thing. If a `blob_changed` event carried the +replaced file's `filename`, `byte_size` and `download_url`, "show me this file's +history" would become answerable in one call. ## What this unblocks - **#404** — a release script that replaces a binary in place, so a published link keeps working across releases -- **CLI** — `basecamp files versions` becomes a listing of changes the CLI can - itself produce, rather than a read-only window onto web-app activity +- **CLI** — today `basecamp files versions` can only *observe* replacements made + in the web app. If this ships, it would list `blob_changed` events the CLI + itself caused - **Any integration** that mirrors an external artifact into Basecamp and wants one stable recording instead of a growing pile of near-duplicate uploads From b9a719599c964e0d3040171d48bc55721994dd72 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Tue, 4 Aug 2026 13:36:32 -0700 Subject: [PATCH 3/3] Keep the communiques out of the source tree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit basecamp-cli is public; bc3 is private. COMMUNIQUE-upload-versions-api.md quoted three verbatim Ruby blocks from bc3 — Uploads::StageController#create, Uploads::VersionsController#create, and UploadsController's before_action and param filters — so merging it would have published private source in a public repo. Caught before merge. Beyond the leak, a public CLI's source tree is the wrong venue for a cross-team ask. These are messages to BC3 and to the SDK, not documentation of this tool; they belong in issues on those repos, where the recipients are. Drop both files. Keep the API-COVERAGE.md correction, which is a claim about this CLI and does belong here, with its links to the removed files replaced by plain description. --- API-COVERAGE.md | 7 +- COMMUNIQUE-sdk-upload-versions-response.md | 126 ------------- COMMUNIQUE-upload-versions-api.md | 202 --------------------- 3 files changed, 4 insertions(+), 331 deletions(-) delete mode 100644 COMMUNIQUE-sdk-upload-versions-response.md delete mode 100644 COMMUNIQUE-upload-versions-api.md diff --git a/API-COVERAGE.md b/API-COVERAGE.md index 6bb09c87..0dfb277d 100644 --- a/API-COVERAGE.md +++ b/API-COVERAGE.md @@ -18,8 +18,9 @@ decodes the response with the wrong type: the API returns version *events* (`action`, `details`, `recording_id`), the OpenAPI spec models them as `[]Upload`, so a caller gets Upload structs with blank `title`/`filename`/ `status` and an `id` that is the event id and does not resolve as an upload. -Shipping a command on that would mean shipping wrong data, so it is held. See -[COMMUNIQUE-sdk-upload-versions-response.md](COMMUNIQUE-sdk-upload-versions-response.md). +Shipping a command on that would mean shipping wrong data, so it is held. +Reported to the SDK; the command lands when `ListVersions` returns an event +type. This file previously read "100% coverage of tracked in-scope API (184/184)". That was wrong, and the matrix had no way to say so — with only ✅ and ⏭️ @@ -211,7 +212,7 @@ cannot faithfully cover at least one endpoint for a reason outside the CLI. A | search | 2 | `search` | ✅ | BC4 | - | Full-text search + metadata. Filters: `--project`/`--in`, `--type`, `--creator`, `--since` (BC5-only), `--file-type`, `--exclude-chat`. Metadata lists recording/file search types | | recordings | 4 | `recordings` | ✅ | BC4 | - | Browse by type/status, trash/archive/restore | | **Files & Documents** | -| uploads | 8 | `files`, `uploads` | ⚠️ | BC4 | - | list, show, create, update, download; trash/archive/restore go through `recordings`. Create supports `--visible-to-clients` (root vault only). **Blocked:** `GET /uploads/:id/versions.json` — the SDK's `ListVersions` types the response as `[]Upload`, but the API returns version *events*, so the data comes back blank and misleading. Command is written and held on `feat/files-versions`; see [COMMUNIQUE-sdk-upload-versions-response.md](COMMUNIQUE-sdk-upload-versions-response.md). Replacing an upload's *file* is a separate BC3 gap — see [COMMUNIQUE-upload-versions-api.md](COMMUNIQUE-upload-versions-api.md) | +| uploads | 8 | `files`, `uploads` | ⚠️ | BC4 | - | list, show, create, update, download; trash/archive/restore go through `recordings`. Create supports `--visible-to-clients` (root vault only). **Blocked:** `GET /uploads/:id/versions.json` — the SDK's `ListVersions` types the response as `[]Upload`, but the API returns version *events*, so the data comes back blank and misleading. Command is written and held on `feat/files-versions`, pending an SDK fix. Replacing an upload's *file* has no API at all — a separate BC3 gap, tracked outside this repo | | vaults | 8 | `files`, `vaults` | ✅ | BC4 | - | list, show, create | | documents | 8 | `files`, `docs` | ✅ | BC4 | - | list, show, create, update. Create supports `--subscribe`/`--no-subscribe`, `--visible-to-clients` (root vault only) | | attachments | 1 | `uploads`, `attachments` | ✅ | BC4 | - | Upload via `attach`; list embedded attachments via `attachments list` (parses `` from content) | diff --git a/COMMUNIQUE-sdk-upload-versions-response.md b/COMMUNIQUE-sdk-upload-versions-response.md deleted file mode 100644 index af676acd..00000000 --- a/COMMUNIQUE-sdk-upload-versions-response.md +++ /dev/null @@ -1,126 +0,0 @@ -# Communique: `UploadsService.ListVersions` Decodes Version Events as Uploads - -**From:** Basecamp CLI team -**To:** basecamp-sdk (Go) -**Re:** `ListUploadVersionsResponseContent` is modeled as `[]Upload`; the API -returns version events -**Affects:** SDK v0.12.0 (current pin) — `pkg/basecamp/vaults.go:956`, -`pkg/generated/client.gen.go:1625` -**Context:** CLI branch `feat/files-versions`, held rather than merged because -of this - ---- - -## The defect - -`GET /uploads/{id}/versions.json` returns a list of **version events**. The -OpenAPI spec models it as a list of **uploads**: - -```go -// pkg/generated/client.gen.go:1625 -type ListUploadVersionsResponseContent = []Upload -``` - -`UploadsService.ListVersions` then decodes each element with -`uploadFromGenerated`, so events are silently coerced into `Upload` structs. -Nothing errors — the fields simply do not line up. - -## Evidence - -`bc3-api/sections/uploads.md` documents the response as events, and a live -`GET /uploads/10165243019/versions.json` against production agrees: - -```json -[ - { - "id": 16850997498, - "recording_id": 10165243019, - "action": "active", - "details": { "delivered_recipient_ids": [], "status_was": "drafted" }, - "created_at": "2026-08-04T17:15:01.261Z", - "creator": { "id": 39024674, "name": "…" } - } -] -``` - -The bc3 implementation confirms it — -`Uploads::VersionsController#index` renders -`@recording.version_events_since_publication.reverse_chronologically`. - -## What a caller gets today - -Running the same request through `ListVersions` yields an `Upload` that is -mostly empty, and worse, one that is confidently wrong: - -```json -{ - "id": 16850997498, - "created_at": "2026-08-04T17:15:01.261Z", - "creator": { "id": 39024674, "name": "…" }, - "title": "", - "filename": "", - "status": "", - "content_type": "", - "byte_size": 0, - "download_url": "", - "url": "", - "app_url": "", - "updated_at": "0001-01-01T00:00:00Z" -} -``` - -Three distinct problems, in increasing order of harm: - -1. **Dropped fields.** `action`, `details` and `recording_id` — the only fields - that say *what happened* — have nowhere to land and are discarded. -2. **Empty fields that read as data.** `title`, `filename`, `status`, - `download_url`, `byte_size` are zero values, not absent ones. A caller - rendering a versions table gets blank cells and no signal that the blanks - are an artifact. -3. **A misleading `id`.** `16850997498` is the *event* id. Typed as an `Upload`, - it invites `Uploads().Get(ctx, 16850997498)`, which 404s. The real upload id - is in the discarded `recording_id`. - -## Suggested fix - -Model the response as its own type rather than reusing `Upload`: - -```go -// UploadVersion is one entry in an upload's change history. -type UploadVersion struct { - ID int64 // the event id, not an upload id - RecordingID int64 // the upload this version belongs to - Action string // e.g. "active", "blob_changed" - Details map[string]any - CreatedAt time.Time - Creator Person -} - -type UploadVersionListResult struct { - Versions []UploadVersion - Meta ListMeta -} -``` - -This is a breaking change to `UploadVersionListResult.Versions`, but the -current element type carries no usable data, so we do not expect real -callers to be relying on it. - -The spec is the root cause — `ListUploadVersionsResponseContent = []Upload` -comes from the OpenAPI description, so the fix likely belongs upstream of the -generator, with the hand-written service following. - -## What it unblocks - -CLI PR for `basecamp files versions` is written, tested and complete, and is -held unmerged solely on this. As soon as `ListVersions` returns the event -shape, the command ships: `--limit`/`--page`/`--all` semantics, catalog entry, -surface snapshot and smoke coverage are already in place. - -It also matters for `API-COVERAGE.md`. `ListVersions` is the one uploads -endpoint the CLI does not cover, and the reason is this type — not a missing -command. - ---- - -**Note:** no changes were made to the SDK repository. Reporting only. diff --git a/COMMUNIQUE-upload-versions-api.md b/COMMUNIQUE-upload-versions-api.md deleted file mode 100644 index 37e38bdf..00000000 --- a/COMMUNIQUE-upload-versions-api.md +++ /dev/null @@ -1,202 +0,0 @@ -# Communique: Replacing an Upload's File Over the API - -**From:** Basecamp CLI team -**To:** BC3 Rails development -**Re:** Exposing upload file replacement (new versions) as a documented API -**Context:** CLI issue [#404](https://github.com/basecamp/basecamp-cli/issues/404) — replacing a -signed release binary in place - ---- - -## The ask in one line - -Basecamp can already replace an upload's file in place, keeping the recording -id and accumulating versions. The web app does it. The API cannot, and it is -the one thing standing between a release script and a stable download URL. - -## The use case - -A release pipeline uploads a signed Windows `.exe` to a Docs & Files folder and -publishes the link. Next release, it needs the *same* link to serve the new -binary. Today the only API-reachable move is `POST /vaults/:id/uploads.json`, -which creates a **new** recording with a **new** id and a **new** URL. So the -folder accumulates `basecamp-v0.8.1.exe`, `basecamp-v0.8.2.exe`, … and every -previously published link points at a stale build. - -What we want instead is one durable recording whose file is replaced on each -release. `GET /uploads/:id/versions.json` already exposes the *record* of that -happening — the read side of this feature is public. The write side is not. - -One clarification on that endpoint, since the rest of this document leans on it: -it returns `version_events_since_publication`, which is every recording event, -not only replacements. bc3-api currently describes it as "each version event -represents a file replacement," which overstates it — the response example -below carries an `"action": "active"` event with `"status_was": "drafted"`, -a publication transition rather than a new file. File replacement is the -`blob_changed` subset, emitted by `Upload#recorded_as` → `track_blob_change` -when the new recordable's blob differs from the previous one. Whatever write -shape you pick, please don't treat the endpoint as a list of binary revisions — -it isn't one today, and a client filtering for replacements has to select -`blob_changed` itself. - -## What already exists (and why it isn't reachable) - -Replacement is implemented and shipping. It's a two-step flow: - -**1. Stage the file** — `POST /uploads/stage` (`Uploads::StageController#create`): - -```ruby -def create - @upload = Upload.create!(file: params[:file]) - render :show -end -``` - -Creates an `Upload` with no recording, and renders its signed GID. - -**2. Swap it in** — `POST /buckets/:bucket/uploads/:upload_id/versions` -(`Uploads::VersionsController#create`): - -```ruby -def create - Upload.transaction do - subscribers = find_subscribers - @upload.update! base_name: upload_params[:base_name], description: upload_params[:description] - @recording.update! recordable: @upload - @recording.notify subscribers, event: @recording.version_events_since_publication.last - @recording.change_subscribers added: subscribers - end - - redirect_to @recording -end -``` - -The recording keeps its id; `Upload#recorded_as` tracks a `blob_changed` event, -which is what the versions index later lists. This is precisely the behavior -#404 needs. - -Four things keep it out of reach for an API client: - -1. **Undocumented.** Neither route appears in - [bc3-api](https://github.com/basecamp/bc3-api). `sections/uploads.md` - documents `GET /uploads/2/versions.json` and stops there. -2. **No JSON response.** `create` ends in `redirect_to @recording` with no - `respond_to`, so a `.json` request gets a `302`, not the updated upload. -3. **Form-shaped params.** It reads `params.require(:uploads).first`, i.e. an - array of `{sgid, base_name, description}` — shaped for the composer's - multi-file form, not for a single-resource API call. -4. **Different upload convention.** Staging takes a raw multipart - `params[:file]`. Every documented API upload path instead goes through - `POST /attachments.json` and carries the result as `attachable_sgid`. - `Upload::Creation` already accepts `attachable_sgid` — but - `UploadsController` only wires it up on `create`: - - ```ruby - before_action :set_new_upload, only: :create - - def update - @recording.update! recordable: @upload.changing(upload_params), status: status_param - end - - def upload_params - params[:upload]&.permit(:base_name, :description) || {} - end - - def uploadable_params - params.permit(:attachable_sgid, :file) - end - ``` - - So an API client that PUTs `attachable_sgid` to `/uploads/2.json` gets a - `200 OK` and **no replacement** — the parameter is silently dropped, not - rejected. That silence is its own small problem: the caller has no way to - tell a no-op from a success. - -## Two possible shapes - -**Option A — accept `attachable_sgid` on update.** Smallest surface, matches the -convention every other API upload path already uses: - -``` -PUT /uploads/2.json -{ "attachable_sgid": "BAh7CEk...", "base_name": "basecamp-windows-amd64" } -``` - -Permit `:attachable_sgid` in `upload_params` for API requests, resolve it the -way `Upload::Creation` does, and swap the recordable when present. Omitting it -keeps today's metadata-only behavior, so this is additive. - -One wrinkle worth deciding deliberately: `Uploads::VersionsController#create` -does subscriber notification and `change_subscribers` work that -`UploadsController#update` does not. If replacement moves to `update`, that -notification path needs to come along, or the API's replacements will be -quieter than the web app's. - -**Option B — document a JSON `POST /uploads/2/versions.json`.** Keeps -replacement where it already lives, alongside the versions index a client would -poll afterward: - -``` -POST /buckets/1/uploads/2/versions.json -{ "attachable_sgid": "BAh7CEk...", "base_name": "basecamp-windows-amd64" } -``` - -Needs a `respond_to` returning the upload JSON instead of a redirect, and a -single-object param shape next to the existing array form. - -We have no stake in which. Option A is less for a client to learn; Option B -keeps the notification behavior where it is and reads more honestly as "add a -version." - -## A related read-side note - -`GET /uploads/:id/versions.json` returns **version events**, not uploads: - -```json -[ - { - "id": 16850997498, - "recording_id": 10165243019, - "action": "active", - "details": { "status_was": "drafted" }, - "created_at": "2026-08-04T17:15:01.261Z", - "creator": { "id": 39024674, "name": "…" } - } -] -``` - -Note the `action` here is `active`, not `blob_changed` — as above, this listing -is the full event stream since publication, and replacements are one kind of -entry in it. - -The documented example in `sections/uploads.md` agrees that these are events. -The OpenAPI spec behind the Go SDK does not — it models the response as -`[]Upload`, which is a separate report to the SDK team. Flagging it here only -because whichever write shape you pick, a client will want the response and the -versions index to describe the same thing. If a `blob_changed` event carried the -replaced file's `filename`, `byte_size` and `download_url`, "show me this file's -history" would become answerable in one call. - -## What this unblocks - -- **#404** — a release script that replaces a binary in place, so a published - link keeps working across releases -- **CLI** — today `basecamp files versions` can only *observe* replacements made - in the web app. If this ships, it would list `blob_changed` events the CLI - itself caused -- **Any integration** that mirrors an external artifact into Basecamp and wants - one stable recording instead of a growing pile of near-duplicate uploads - -## Impact on existing behavior - -Additive under either option. The web composer's flow is untouched; today's -`PUT /uploads/:id.json` keeps working unchanged when the new parameter is -absent. - ---- - -**Question for BC3:** is the staged-upload requirement -(`require_staged_upload` — the replacement Upload must have no recordings yet) -essential to how versions are tracked, or an artifact of the composer flow? It -shapes whether an API client stages first or hands over an `attachable_sgid` -directly.