Correct the API coverage claim: 183/184, not 100% - #621
Conversation
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
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c39164e3be
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
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.
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.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b9a719599c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
jeremy
changed the title
|
Declining this one, and it's worth saying why rather than just resolving.
The priming risk is also smaller than it looks: Rewording the description to hedge would cost skill-trigger quality for no gain in truth. |
API-COVERAGE.mdasserted "100% coverage of tracked in-scope API (184/184 endpoints)". That is false. It also had no way to become accurate: the matrix has only ✅ and ⏭️, so a partly-covered section had to be recorded as fully covered.This adds a deliberately narrow third status —⚠️ blocked: the CLI cannot faithfully cover an endpoint for a reason outside the CLI, and the row must name the blocker. Summary now reads 183/184.
The one gap:
GET /uploads/:id/versions.jsonNot a missing command — the command is written and held in draft #622. The SDK is what blocks it.
A live read-only probe returns version events:
{ "id": 16850997498, "recording_id": 10165243019, "action": "active", "details": { "status_was": "drafted" }, "created_at": "…", "creator": { … } }The OpenAPI spec models the response as
[]Upload, soListVersionsdecodes events intoUploadstructs. The result runs clean and is confidently wrong:title,filename,status,download_urlblank;action,details,recording_iddropped; andidis the event id, sobasecamp files show <id>404s. Shipping that means shipping wrong data.Scope changed since this PR was opened
It originally also added two communiqué files. Those are removed (
b9a71959).basecamp-cliis public andbc3is private, and one of them quoted three verbatim Ruby blocks from bc3 controllers — merging it would have published private source. Beyond that, a public CLI's source tree is the wrong venue for a cross-team ask; those belong as issues on the repos that own the code, where the recipients actually are.The diff is now
API-COVERAGE.mdonly, +26/−4.Merging
This does not depend on the SDK fix — it describes the blocker. Holding it leaves a claim on public
mainthat is verifiably wrong. The tradeoff is churn: 183/184 now, back to 184/184 whenListVersionsis fixed and #622 lands.bin/cigreen (exit code checked, not piped throughtail).