Repin bc3 to 7fe1c63ab3 and absorb project archive/unarchive - #679
Merged
Conversation
The 4e34dc83eb..a26c2e479f range is 70 commits. Classifying all 70 by touched
path — not filtering on doc/api + config/routes.rb + app/views/api, which is a
filter and not a certification — finds five API-contract changes, only two of
which touch doc/api. Dispositions, one per item, before the repin asserts the
range is triaged:
1. BC3 #12550 (6f4781bbd4) project status JSON — absorbed in this PR.
2. BC3 #12555 (the pin commit) upload file replacement — registered. It closes
the write side upload-new-version.md already described, so this is a status
flip to addressed-in-bc3-pr-12555, not a new brief. bc3 chose
POST /uploads/:id/versions.json (201), not the PUT /uploads/{id}.json shape
basecamp-cli#404 hypothesized and that brief disproved. Its 507 is a STORAGE
limit with its own message; ProjectLimitError is not it and the brief says so.
3. BC3 #12396 (98eb24b22f) sort_pings_first — new brief, partial-coverage. The
trap this brief exists to prevent: GetMyNotifications is NOT this endpoint. It
points at /my/readings.json, the notification feed. #12396 changed the
separate settings resource /my/notifications.json, which the SDK models
nothing for. bc3 documents none of it, yet it renders under app/views/api, so
it is API surface — hence partial-coverage.
4. BC3 #9471 (eac8b2b476) OAuth 2.1 — two halves, two dispositions. Discovery
MATCHES, no action: bc3's new .well-known/oauth-authorization-server (RFC
8414) and .well-known/oauth-protected-resource (RFC 9728) are the exact paths
the SDK's hand-written discovery already fetches in Go, TypeScript, Python and
Ruby. The authorization document DIVERGES — new brief. bc3 now draws
resource :authorization and renders app/views/api/authorizations/show.json.jbuilder,
whose shape is not Launchpad's: identity carries only id, accounts carry
resource but neither product nor app_href, and expires_at is integer epoch
seconds rather than ISO-8601. It is reachable today, because Ruby's
Http#get_authorization_document binds to the discovered BC5 issuer. TypeScript
is the exposed SDK: three required string fields would be undefined,
filterProduct silently matches nothing, and new Date(epochSeconds) reads the
integer as milliseconds and yields 1970 rather than throwing. Go already
tolerates it via FlexTime; Python and Ruby return untyped maps.
5. 5c0e774b0d and three siblings reroute Recording.completed to
completed_recently_first, reordering three MODELLED operations
(GetEverythingCompletedTodos, GetEverythingCompletedCards,
GetMyCompletedAssignments) with no field added, removed or retyped and no
route diff. No SDK action: none of the three declares an ordering in its
Smithy docs, so there is no claim to correct. Recorded because it is invisible
to every path-based filter — this is the residual class the sweep exists for.
The paired my/assignings and my/assignments controllers are NOT a rename: both
exist before and after, one line each.
The residual bucket is empty. api_request.rb is touched, but only to ADD a
private app_url_options helper for OAuth's HTML redirects; api_request? and
restrict_view_paths_to_api_root are untouched, so what counts as an API route is
unchanged. #12475 registers Gallery as a dock_tool, which is additive because
DockItem.name is a plain String and not an enum, and no app/views/api/galleries
template exists. The cable Origin whitelist guard (4536d5952a) touches the host
the SPEC §23 connector dials but not the dial contract.
project-archive-unarchive.md lands in its end state since this PR absorbs it,
with one correction to the registration draft: forbid_clients is an UNSCOPED
before_action (app/controllers/concerns/permissions.rb:22-24), so 403 is
reachable on unarchive too. The asymmetry is in the cause — the admin/creator
guard is archive-only — not in reachability, and bc3's own tests split on exactly
that line.
spec/api-provenance.json moves to a26c2e479f (2026-08-05), bc3 origin/master. compatibility.bc3-four is deliberately untouched: that pin marks the last VERIFIED API-surface state of the four branch, and nothing in this PR re-verified it. Repinning is what makes the absorption legitimate rather than the other way round. doc/api/sections/projects.md gains "Archive a project" and "Unarchive a project" only in #12550, so at the old pin the two routes are absent from spec/bc3-routes.json entirely. Absorbing without repinning would model a contract the pin says isn't there; repinning without absorbing would leave two documented routes with no operation behind them. Regenerating the table at the new pin is what lets direction 1 of bc3-route-parity go green on the truth instead of on a waiver — and it did: the gate reports zero new sdk_routes_absent_from_bc3_docs entries. make bc3-routes adds exactly four rows and nothing else, which is the mechanical confirmation that the range classification in the previous commit was complete: the two project-status PUTs, and #12555's flat and bucket-scoped upload-version POSTs. 373 routes, 64 sections. The upload rows get ONE allowlist entry, not two. Direction 2's failure named both spellings, so two entries looked right; the bucket-scoped one then failed as "matches nothing", because direction 2 collapses a leading /buckets/:id and both rows normalize to the same key. Established by running the gate rather than by asserting a row count, and the comment records it so the next reader does not repeat the guess. spec/api-gaps/README.md is hand-edited, as its .writerExcludes entry requires: the marked @bc3-pin span advances, the previous pin's triage is demoted into the past-tense record above the older ones, and the new range triage takes its place. .unmarkedPinCitations stays at 2 but the reason is rewritten to describe the new citations rather than the old ones — the range naming its own endpoint, and #12555 cited as the commit that shipped upload file replacement. Both are as-of facts bound to this triage. Two pin restatements this PR nearly shipped as class-A claims were caught by doc-constants-check and rewritten to reference form instead: the bc3_refs introduced_in line in upload-new-version.md, and two allowlist comments. Naming the pin by reference — "the repin that registered it" — stays true across the next repin and restates no constant. The /authorization allowlist entry keeps its out_of_scope disposition and gets its reasoning corrected. It asserted the route is served only from Launchpad, which was true when written; #9471 has bc3 drawing and rendering its own.
Two operations next to TrashProject, copied from ArchiveRecording /
UnarchiveRecording — same verbs, same URI suffixes, same 204 — with
recordingId: RecordingId swapped for projectId: ProjectId. 247 -> 249 operations.
ArchiveProject PUT /{accountId}/projects/{projectId}/status/archived.json 204
UnarchiveProject PUT /{accountId}/projects/{projectId}/status/active.json 204
Both are @idempotent + @basecampIdempotent(natural: true): re-archiving an
archived project is a no-op that still answers 204.
Error lists decided rather than copied. Both take
[NotFoundError, UnauthorizedError, ForbiddenError, InternalServerError] — matching
TrashProject, the nearest project-status operation, and NOT ArchiveRecording's
list, which carries a ValidationError these cannot raise because they have no
request body. UnarchiveProject adds the 507.
ForbiddenError on BOTH is deliberate and was verified in bc3 rather than
inferred: forbid_clients is an unscoped before_action on
Projects::StatusController and is `head :forbidden if Current.user.client?`
(app/controllers/concerns/permissions.rb:22-24), so it fires on active exactly as
on archived. Only the admin/creator guard is archive-scoped.
ProjectLimitError is a new @HttpError(507) shape modelled on WebhookLimitError,
wired into UnarchiveProject AND CreateProject — the same body was already
reachable from create (ensure_account_can_create_projects renders it with
status: :insufficient_storage, app/controllers/concerns/resource_limits.rb) and
unmodelled, so one shape closes two gaps. Two shapes sharing 507 is safe because
no SDK maps status -> shape; all six switch on the status code into a fixed
taxonomy, and 404/422/400 already carry several shapes each. The invariant that
does matter holds: no operation's error list contains two shapes with the same
status, and CreateProject had no 507 before this.
The doc comment first lines are complete sentences, because generators truncate
method docs to line 1. Unarchive's says it restores from trash as well as the
archive — the one thing about it a caller cannot guess, since it is the inverse of
both ArchiveProject and TrashProject.
Registering the operations in the service shape's `operations:` list is the step
that actually makes them exist. Tagging them in overlays/tags.smithy and adding
the operations alone regenerated cleanly and silently emitted 247 operations with
ProjectsService still at 5 — the andon-cord case for an operation count mismatch.
No generator-mapping change was needed: Projects already exists in every
TAG_TO_SERVICE, and Python's verb table already maps Archive/Unarchive.
Regeneration adds a JSON507 field to Go's CreateProjectResponse, which is
additive.
Go wrappers are mandatory, not optional: EXCLUDED_OPS is empty by policy and go-check-drift reports an unwrapped generated operation as an error, which it did (247 / 249, 99%). Archive and Unarchive copy the Trash wrapper exactly — gating hook, OnOperationStart/OnOperationEnd, checkResponse — and coverage is back to 249 / 249. Nothing to wire in any client: Projects is an existing service in all six SDKs. Tests in all six, since no generator emits tests. Happy path is 204 -> void/nil/ None; Swift and Go additionally assert the method and URL suffix, because a PUT to the wrong status segment is the failure mode that would otherwise pass. Python had no trash test to copy, so this establishes the pattern there, covering both the sync and async classes the generator emits. Go had none either; its new table test covers Trash alongside the two new wrappers. Two error cases per SDK, chosen for what they prove: - 403 on archive — the admin/creator restriction, which bc3 answers with head :forbidden. - 507 on unarchive — the ONLY behavioural evidence for the new ProjectLimitError shape. It surfaces as a generic api_error carrying http_status 507 (SPEC.md §7), because no SDK gives 507 a named class. The Python 507 assertion deliberately asserts `retryable is False` while the Ruby and Kotlin ones assert retryable true. That is not an inconsistency in the tests: Python's fallback arm builds a bare ApiError whose default is retryable=False, where the other five mark every unclassified 5xx retryable. It is a pre-existing divergence from SPEC §7, asserted as-is and left alone rather than fixed in passing; the comment in the test says so. Error assertions check http_status and the error code, not just the exception class — a class-only assertion here would pass against a wrong status. Nothing was added to conformance/tests/live-my-surface.json: it is a read-only live surface and archiving is destructive.
Every number re-derived from behavior-model.json and openapi.json rather than incremented, and they cross-check: 52 PUTs + 24 DELETEs + 7 idempotent POSTs = 83 idempotent, and 83 + 125 readonly = 208 union. scripts/check-idempotency-parity is the one ENFORCED count: 81 -> 83 idempotent, 206 -> 208 union, with a comment in the existing house style recording that both new operations are naturally-idempotent PUTs and neither is readonly, so both counts move by the same 2. SPEC.md: total 247 -> 249 and idempotent 81 -> 83 in §Operation Counts; non-idempotent stays 166. The two "all 247 operations" mentions become 249. The §2 retry distributions are the easily-missed ones: 203 -> 205 at max:3 (44 at max:2 unchanged), the other 195 -> 197 retry-eligible ops unaffected at the default cap, and all 206 -> 208 retry-eligible operations clamped above it. The 11 idempotent max:2 operations are unchanged. SECURITY.md: 247 -> 249, 81 -> 83 mutations, 50 -> 52 PUTs. GET (125), DELETE (24) and POST (7 idempotent / 41 not) are all unchanged. AGENTS.md: both 247s. MIGRATING.md deliberately gets nothing. Its four 247 mentions are historical v0.13.0 measurements, it is written at release time, and new operations break nobody, so no entry is due at PR time (CONTRIBUTING.md:260-269). typescript/README.md and go/README.md enumerate the projects methods, as does the service doc comment at typescript/src/client.ts; all three gain archive and unarchive. Ruby, Swift and Kotlin READMEs are description-only and Python's has no table, so there is nothing to extend there. spec/fixtures/projects/error-limit.json already held exactly the ProjectLimitError body, documented as the CreateProject 507 — it was waiting for this shape. Left out of the fixtures manifest: adding it there would oblige a covered_schemas representative, and the six SDK tests are the behavioural evidence.
Upstream moved again while this branch was being finished — the third time during
this change, after 4e34dc83eb -> 6f4781bbd4 -> a26c2e479f. The range is now
4e34dc83eb..7fe1c63ab3, 71 commits, six API-contract-or-documentation changes.
BC3 #12566 (7fe1c63ab3) is prose only: two added lines of
doc/api/sections/projects.md saying a project's status is read-only on Update a
project — passing one has no effect and still answers 200 — with a pointer to
Archive, Unarchive and Trash instead. No route, no bullet, no payload field.
Registered rather than absorbed, because there is nothing to absorb, and the
reason is worth stating: the SDK already declines to model status as writable on
UpdateProject, and project-archive-unarchive.md justified that by reading
create_project_params and observing the field is not permitted. #12566 makes it
bc3's documented contract, so the omission is now backed by upstream prose rather
than by a controller read — and #12566's "see Archive/Unarchive" pointer resolves
to the two operations this PR adds. The silent-drop behaviour itself is unchanged;
#12566 documents it, it does not fix it.
Proof it is documentation and not contract: spec/bc3-routes.json regenerates at
373 routes across 64 sections with only its revision line changing. That is the
one distinction the triage prose now makes explicitly, because a doc/api diff is
necessary but NOT sufficient for a route delta — two of the three doc/api commits
in this range move the table, and the third does not.
.unmarkedPinCitations stays at 2, and the reason records why the number survived a
repin inside its own triage without changing meaning: a26c2e479f and its #12555
citation stopped being checked the moment the pin advanced past them, so the two
citations now granted are the new endpoint's — the range itself, and #12566.
One sentence went stale the moment the pin moved and is fixed here:
upload-new-version.md's bc3_refs claimed #12555's merge commit was "the merge
commit the SDK repinned to", which repinning past it falsified within the same PR.
Reference-form phrasing that names no revision ("the repin that registered BC3
#12555", in the allowlist) survived untouched — which is the argument for it.
Gates rerun on the new pin: doc-constants-check, provenance-check,
sync-api-version-check, bc3-route-parity, validate-api-gaps and
test-bc3-route-parity all exit 0, as does bc3-routes-check with BC3_REPO_PATH set
(it is absent from check-targets and from CI precisely because it needs one).
Full make: All checks passed.
github-actions
Bot
added
typescript
jeremy
changed the title
![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: 1b0ddd40e7
ℹ️ 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".
Codex caught this on #679 and it is a real gap, not a false positive. The low-level grouped surface `generated.Client.Projects()` is emitted from an explicit per-operation switch in go/templates/client.tmpl, NOT from the operation list — so adding an operation to the spec does not add it there. The Projects branch stopped at TrashProject, leaving ProjectsService with List/Get/Create/Update/Trash and no Archive/Unarchive. Verified rather than taken on trust, and the decider is the sibling: the same grouped client DOES expose RecordingsService.Archive and .Unarchive — the exact two operations ArchiveProject/UnarchiveProject were copied from. So the grouped surface is a curated subset globally (84 methods across 15 services out of 249 operations) but is clearly meant to cover a service's own transitions, and Projects was left asymmetric with Recordings. Two template cases added next to TrashProject, client.gen.go regenerated. Nothing would have caught this. go-check-drift compares generated operations against this repo's go/pkg/basecamp wrappers and never looks at the grouped surface, which is why the omission survived a green `make`. A completeness gate is NOT cheap here — asserting every operation appears in the grouped client would fail immediately for the 165 operations deliberately absent from it, so a real gate would have to encode which services are grouped and which of their operations must appear. Rather than fake that, the guard is scoped to what regressed: three compile-time method-value references in projects_test.go. They cost nothing at runtime and turn a regressed template into a build failure instead of a silently unreachable operation. On the strength of that guard: a Go reference to a non-existent method is a compile error by definition, and both halves are verified — HEAD's client.gen.go has no ProjectsService.Archive, the regenerated one does, and the package compiles. I did not stage a synthetic failing build, because for a compile-time reference there is nothing for one to establish beyond that.
Member
Author
|
@codex review Re-requesting on Copilot has errored out twice at its own |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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
added a commit
that referenced
this pull request
Aug 6, 2026
Release prep for v0.13.0. Documentation only; no code. Counts re-derived from origin/main by PR-merge-commit ancestry, not incremented: 58 -> 64 merged pull requests 15 -> 16 labelled breaking (#678 earned it) 238 -> 247 becomes 238 -> 249 (#679 added ArchiveProject/UnarchiveProject) 14 added / 5 removed becomes 16 added / 5 removed; at capability level 12 additions becomes 14. 11 route-moved is unchanged, and verified. Baseline `70d576bd8` -> `9a819e44d`, the last commit of release content. The 61 = 55 + 6 split is unchanged, and reconciles across MIGRATING.md, the root README and all six per-SDK banners (Go 12+4, TS 9, Ruby 10+1, Swift 10, Kotlin 6+1, Python 8). Two `247`s were deliberately NOT touched. Both are as-of facts about a specific PR, true forever, and rewriting them would have made two correct sentences false: #648 did leave the inventory at 247 on both sides, and #629 did take it from 241 to 247. Only #679 moved it to 249. Same distinction the provenance-pin convention draws between a current-value claim and an as-of one. The third `247`, "every one of the N operations in metadata.json declares a retry block", IS a current-value claim and did move — verified that all 249 still declare one rather than assuming it. The derivation snippet embedded in the guide had the trap that produced a wrong number here: `--limit 300`. `gh pr list` orders by CREATED, so a long-open PR that merged late can fall off the end and go silently uncounted. Raised to 1000 and documented as trap 3, alongside reading from origin/main rather than HEAD. SPEC §2 step 5 said Go returns an `error` on a bad `max_pages` and that "Swift alone is not recoverable". Both false. Go panics `"basecamp: max pages must be positive"`, and the generated client carries no MaxPages at all, so there is no other Go path that could return one. Go and Swift are both non-recoverable, each because the constructor taking the cap cannot report a failure — Go's NewClient has no error return, Swift's init is public and non-throwing. §3 step 5 already said Go panics on config failure, so the paragraph contradicted the document around it. Every SDK's behaviour was read out of source before this was rewritten; this was the third factual error in that one paragraph. Also records what #680 changed there: Python's bool exclusion, and the rule that a guard feeding a `??` fallback must test `!= null` to match it. Codex correctly pointed out on #680 that the spec never said whether an explicit null counts as a supplied cap. Now it does. MIGRATING.md gains a prose entry for the maxPages validation, stated as NOT part of the 61: those are breaks a compiler will not catch, class A silent and class B needing a particular server response, and this is neither — it fails at construction, deterministically, before any request. The entry is per-SDK because the six did not start level: Go and Ruby already rejected a non-positive cap at v0.12.0 and move not at all, and Python's 0 and negatives already raised, so only its type check is new. The six PRs that landed after the guide's first draft are recorded rather than left for a reviewer to reconcile against git log.
jeremy
added a commit
that referenced
this pull request
Aug 6, 2026
…chor
Codex caught a real coverage hole, and the number makes it plain: the inventory
recorded 63 methods where the generated client has 86. The 23 missing are
exactly the typed body variants — Todos().Create, Projects().Update and the
rest, which are the methods people actually call.
A body-bearing arm emits two methods:
func (s *TodosService) CreateWithBody(...) <- literal
func (s *TodosService) Create{{.Suffix}}(...) <- inside {{range .Bodies}}
I recorded only the first, reasoning that the WithBody form "is enough to prove
the arm fired". It proves the arm fired; it does not prove the arm still emits
the public method. Delete the {{range .Bodies}} block and regeneration drops
Todos().Create while CreateWithBody survives — template, inventory and generated
output all still agree, and the gate passes having lost a public API method.
That is the same shape as #679, one level down, in the gate built to catch #679.
The dynamic name cannot be read literally, only by prefix. So: capture the
prefix from the template, require the inventory to record a concrete method
under it beyond the WithBody form, and let the existing generated-output
cross-check prove that method still exists. The 23 resolved names are now
recorded and the accounting is 86/86.
Two self-test cases, both directions, each shown red first: 12 drops the typed
name from the inventory, 13 deletes the {{range .Bodies}} block from a temp copy
of the template and gets "records method `Create` but client.tmpl's arm does not
emit it".
The guard-to-case table is the measured mapping, not the tidy one I first wrote:
the phantom check turns out to own cases 8 AND 13, and the `explained` filter
beside it is pinned by the POSITIVE control, because a filter whose job is to
prevent false positives can only be pinned by a case meant to pass.
jeremy
added a commit
that referenced
this pull request
Aug 6, 2026
Codex again, and again correct. The previous fix made recorded-implies-generated true, but left the reverse open: a method the generated client exposes and the inventory does not record was invisible. An unrecorded method is one that can later disappear with nothing noticing, which is the #679 failure mode at method granularity — the exact thing this gate exists to stop. It is reachable without anyone touching this gate. A grouped operation that gains a second supported request content type makes {{range .Bodies}} emit an extra suffixed method; the dynamic-prefix check added last commit is already satisfied by the variant recorded earlier, so the new one slips in unrecorded and the gate stays green. So the method accounting is now total in both directions, for the same reason the operation accounting is: one-way containment is not accounting. Both sides balance at 86/86 today, so this is a no-op on the current tree and a gate on every future one. Case 14 pins it and is red against the pre-fix checker — the only case that overrides GROUPED_CLIENT_GENERATED, splicing one extra method into a temp copy of client.gen.go. The mutation sweep confirms it turns exactly case 14 red.
jeremy
added a commit
that referenced
this pull request
Aug 6, 2026
ArchiveProject and UnarchiveProject shipped missing from the grouped generated client (generated.Client.Projects()) and survived TWO fully green `make` runs. Codex review caught it; a gate should have. go-check-drift compares generated operations against the hand-written go/pkg/basecamp wrappers and go-check-wrapper-drift compares their fields — neither looks at the $opid chain in go/templates/client.tmpl. The next operation added to a grouped service hits the same hole. The obvious invariant does not work. "Every operation in a grouped service's tag must be exposed" is false for 14 of the 15 services today: only Projects is exhaustive over its tag, Todos exposes 6 of 18, Card Tables 5 of 21, and 9 tags have no grouped presence at all. Curation is the design. Nor is a plain inventory of what IS exposed enough — a newly-added operation would be absent from both the inventory and the template, they would agree, and the gate would pass. That is the #679 miss exactly. So: total accounting. Every one of the 249 operations appears exactly once across go/grouped-client-inventory.yml's grouped: and not_grouped:. A new operation lands in neither and fails loudly, forcing a decision. Same device as spec/doc-constants.json's committed marker counts, where both adding and deleting fail until someone restates the number. The ~186-entry not_grouped: list is bulk but pure data, and it is what makes the accounting total. The gate cannot derive method names from operationIds — arms deliberately rename (MoveCard -> Move, UpdateCard -> UpdateVerbatim) — so it keys on the operationId and carries the method name as recorded data. It anchors the generated cross-check on the accessor block rather than a bare `type .*Service struct` grep, because CloudFileService and DoorService are unrelated schema types in the same file. Ruby, following scripts/check-go-optional-pointers, so spec-gates needs no new CI setup. Extraction floors on all three inputs: a regex that stops matching returns an empty set, which reads as "all clear" and is how a gate silently stops gating. The self-test is not optional here. The gate's live run only ever exercises the passing case, so nothing there proves it rejects anything. Ten adversarial cases plus a positive control drive the real checker through env seams against synthetic inputs in a tmpdir; the tracked tree is never written to. Every guard is pinned by exactly one case and every case by exactly one guard — measured by mutating a copy of the checker per guard, not assumed. The interim compile-time guard in projects_test.go stays, with its comment retargeted: it costs nothing and proves a different thing, that the method is actually callable. The gate reads text; only the compiler can say that.
jeremy
added a commit
that referenced
this pull request
Aug 6, 2026
…chor
Codex caught a real coverage hole, and the number makes it plain: the inventory
recorded 63 methods where the generated client has 86. The 23 missing are
exactly the typed body variants — Todos().Create, Projects().Update and the
rest, which are the methods people actually call.
A body-bearing arm emits two methods:
func (s *TodosService) CreateWithBody(...) <- literal
func (s *TodosService) Create{{.Suffix}}(...) <- inside {{range .Bodies}}
I recorded only the first, reasoning that the WithBody form "is enough to prove
the arm fired". It proves the arm fired; it does not prove the arm still emits
the public method. Delete the {{range .Bodies}} block and regeneration drops
Todos().Create while CreateWithBody survives — template, inventory and generated
output all still agree, and the gate passes having lost a public API method.
That is the same shape as #679, one level down, in the gate built to catch #679.
The dynamic name cannot be read literally, only by prefix. So: capture the
prefix from the template, require the inventory to record a concrete method
under it beyond the WithBody form, and let the existing generated-output
cross-check prove that method still exists. The 23 resolved names are now
recorded and the accounting is 86/86.
Two self-test cases, both directions, each shown red first: 12 drops the typed
name from the inventory, 13 deletes the {{range .Bodies}} block from a temp copy
of the template and gets "records method `Create` but client.tmpl's arm does not
emit it".
The guard-to-case table is the measured mapping, not the tidy one I first wrote:
the phantom check turns out to own cases 8 AND 13, and the `explained` filter
beside it is pinned by the POSITIVE control, because a filter whose job is to
prevent false positives can only be pinned by a case meant to pass.
jeremy
added a commit
that referenced
this pull request
Aug 6, 2026
Codex again, and again correct. The previous fix made recorded-implies-generated true, but left the reverse open: a method the generated client exposes and the inventory does not record was invisible. An unrecorded method is one that can later disappear with nothing noticing, which is the #679 failure mode at method granularity — the exact thing this gate exists to stop. It is reachable without anyone touching this gate. A grouped operation that gains a second supported request content type makes {{range .Bodies}} emit an extra suffixed method; the dynamic-prefix check added last commit is already satisfied by the variant recorded earlier, so the new one slips in unrecorded and the gate stays green. So the method accounting is now total in both directions, for the same reason the operation accounting is: one-way containment is not accounting. Both sides balance at 86/86 today, so this is a no-op on the current tree and a gate on every future one. Case 14 pins it and is red against the pre-fix checker — the only case that overrides GROUPED_CLIENT_GENERATED, splicing one extra method into a temp copy of client.gen.go. The mutation sweep confirms it turns exactly case 14 red.
jeremy
added a commit
that referenced
this pull request
Aug 7, 2026
* Gate the Go grouped client, which nothing was watching ArchiveProject and UnarchiveProject shipped missing from the grouped generated client (generated.Client.Projects()) and survived TWO fully green `make` runs. Codex review caught it; a gate should have. go-check-drift compares generated operations against the hand-written go/pkg/basecamp wrappers and go-check-wrapper-drift compares their fields — neither looks at the $opid chain in go/templates/client.tmpl. The next operation added to a grouped service hits the same hole. The obvious invariant does not work. "Every operation in a grouped service's tag must be exposed" is false for 14 of the 15 services today: only Projects is exhaustive over its tag, Todos exposes 6 of 18, Card Tables 5 of 21, and 9 tags have no grouped presence at all. Curation is the design. Nor is a plain inventory of what IS exposed enough — a newly-added operation would be absent from both the inventory and the template, they would agree, and the gate would pass. That is the #679 miss exactly. So: total accounting. Every one of the 249 operations appears exactly once across go/grouped-client-inventory.yml's grouped: and not_grouped:. A new operation lands in neither and fails loudly, forcing a decision. Same device as spec/doc-constants.json's committed marker counts, where both adding and deleting fail until someone restates the number. The ~186-entry not_grouped: list is bulk but pure data, and it is what makes the accounting total. The gate cannot derive method names from operationIds — arms deliberately rename (MoveCard -> Move, UpdateCard -> UpdateVerbatim) — so it keys on the operationId and carries the method name as recorded data. It anchors the generated cross-check on the accessor block rather than a bare `type .*Service struct` grep, because CloudFileService and DoorService are unrelated schema types in the same file. Ruby, following scripts/check-go-optional-pointers, so spec-gates needs no new CI setup. Extraction floors on all three inputs: a regex that stops matching returns an empty set, which reads as "all clear" and is how a gate silently stops gating. The self-test is not optional here. The gate's live run only ever exercises the passing case, so nothing there proves it rejects anything. Ten adversarial cases plus a positive control drive the real checker through env seams against synthetic inputs in a tmpdir; the tracked tree is never written to. Every guard is pinned by exactly one case and every case by exactly one guard — measured by mutating a copy of the checker per guard, not assumed. The interim compile-time guard in projects_test.go stays, with its comment retargeted: it costs nothing and proves a different thing, that the method is actually callable. The gate reads text; only the compiler can say that. * Require "set" explicitly rather than leaning on the 3.2 autoload Codex flagged this as a NoMethodError on the checked spec-gates environment. That specific claim is wrong — Set has been a core autoload since Ruby 3.2, so Enumerable#to_set resolves with no require on 3.3, which is what spec-gates pins; both new steps ran green there, and the gate and self-test pass under a local 3.3.10 too. The underlying point still stands. On 3.1 to_set is genuinely absent, and resting on a version-dependent autoload is an implicit dependency whether or not it happens to hold. The require costs nothing and removes the question, which is cheaper than the argument. Verified on 3.1.6, 3.3.10 and 4.0.6. * Reject template arms for operations the spec no longer has Codex caught a gap between two of the checks, and the bug was in my reasoning rather than my code: the arm-not-in-inventory loop skipped anything absent from openapi.json with the comment "already reported as stale above otherwise". It is not. The stale-inventory check walks `accounted`, so once an operation is removed from BOTH openapi.json and the inventory, its surviving $opid arm is in no set any check compares. It would sit in client.tmpl indefinitely with the gate green — a dead branch of exactly the kind this gate exists to notice. Now its own failure class with its own message, and case 11 pins it. Shown red against the pre-fix checker with the other ten cases still passing, and the guard-mutation sweep re-run: neutering the new branch turns exactly case 11 red and nothing else, so the one-guard-one-case property holds across all eleven. * Pin the gate's reads to UTF-8, and run it under LC_ALL=C in CI Codex caught this and it is real. Under a non-UTF-8 locale Ruby tags file contents US-ASCII, and client.tmpl, openapi.json and the inventory all carry non-ASCII text, so the first scan raised InvalidByteSequenceError and the gate failed before validating anything: $ LC_ALL=C ruby3.3 scripts/check-grouped-client-coverage json/common.rb:364:in 'encode': "\xE2" on US-ASCII (Encoding::InvalidByteSequenceError) from scripts/check-grouped-client-coverage:81 The diagnosis was right; the first casualty is openapi.json rather than client.tmpl, which changes nothing about the fix. All four reads are now pinned. CI could never have caught this, because my two steps did not run under LC_ALL=C — and check-fixture-coverage and check-projected-examples both do, with the comment "the reads are pinned to UTF-8; this proves it stays that way". Pinning without that step is an assertion; with it, it is enforced. Both steps now carry LC_ALL: C, so a future unpinned read fails rather than waiting for a reviewer. The self-test needed it too, and that is the part worth keeping: under LC_ALL=C Open3 returns the checker's output tagged US-ASCII, and every expected fragment contains UTF-8 punctuation, so out.include?(fragment) raised Encoding::CompatibilityError before comparing anything — every negative case would have died for a reason unrelated to what it tests, while looking like the gate was broken. Forced to UTF-8 at the capture; the bytes were always UTF-8 and only the tag was wrong. Verified: gate and all 12 self-test cases pass under LC_ALL=C on 3.3.10, and under a normal locale on 4.0.6. * Track the typed {{range .Bodies}} methods, not just their WithBody anchor Codex caught a real coverage hole, and the number makes it plain: the inventory recorded 63 methods where the generated client has 86. The 23 missing are exactly the typed body variants — Todos().Create, Projects().Update and the rest, which are the methods people actually call. A body-bearing arm emits two methods: func (s *TodosService) CreateWithBody(...) <- literal func (s *TodosService) Create{{.Suffix}}(...) <- inside {{range .Bodies}} I recorded only the first, reasoning that the WithBody form "is enough to prove the arm fired". It proves the arm fired; it does not prove the arm still emits the public method. Delete the {{range .Bodies}} block and regeneration drops Todos().Create while CreateWithBody survives — template, inventory and generated output all still agree, and the gate passes having lost a public API method. That is the same shape as #679, one level down, in the gate built to catch #679. The dynamic name cannot be read literally, only by prefix. So: capture the prefix from the template, require the inventory to record a concrete method under it beyond the WithBody form, and let the existing generated-output cross-check prove that method still exists. The 23 resolved names are now recorded and the accounting is 86/86. Two self-test cases, both directions, each shown red first: 12 drops the typed name from the inventory, 13 deletes the {{range .Bodies}} block from a temp copy of the template and gets "records method `Create` but client.tmpl's arm does not emit it". The guard-to-case table is the measured mapping, not the tidy one I first wrote: the phantom check turns out to own cases 8 AND 13, and the `explained` filter beside it is pinned by the POSITIVE control, because a filter whose job is to prevent false positives can only be pinned by a case meant to pass. * Make the method accounting total in both directions Codex again, and again correct. The previous fix made recorded-implies-generated true, but left the reverse open: a method the generated client exposes and the inventory does not record was invisible. An unrecorded method is one that can later disappear with nothing noticing, which is the #679 failure mode at method granularity — the exact thing this gate exists to stop. It is reachable without anyone touching this gate. A grouped operation that gains a second supported request content type makes {{range .Bodies}} emit an extra suffixed method; the dynamic-prefix check added last commit is already satisfied by the variant recorded earlier, so the new one slips in unrecorded and the gate stays green. So the method accounting is now total in both directions, for the same reason the operation accounting is: one-way containment is not accounting. Both sides balance at 86/86 today, so this is a no-op on the current tree and a gate on every future one. Case 14 pins it and is red against the pre-fix checker — the only case that overrides GROUPED_CLIENT_GENERATED, splicing one extra method into a temp copy of client.gen.go. The mutation sweep confirms it turns exactly case 14 red. * Reject duplicate $opid arms, symmetrically with the inventory Codex again. I check for a duplicate `grouped:` entry in the inventory and never checked the same thing in the template — the symmetric gap, and I wrote both sides. Two failures fall out of `template_arms[opid] = current`. Go evaluates the FIRST matching branch of an {{if}}/{{else if}} chain, so a copy-pasted arm is unreachable dead template; and assigning into the hash keeps the LAST one, so the gate would compare the inventory against a branch that never runs. An identical duplicate is invisible either way. Now `||=`, matching Go's first-branch-wins so the retained arm is the one that actually executes, plus an explicit failure naming the operation. Case 15 pins it and is red against the pre-fix checker; the mutation sweep confirms restoring last-wins assignment turns exactly case 15 red. Fifteen cases now, and five of them exist because a reviewer found a hole this suite did not. Worth stating plainly in the file that the suite is not the proof of coverage it was written to look like. * Say plainly what the self-test does and does not prove The header claimed every guard is pinned by EXACTLY one case. That stopped being true two commits ago and the table already contradicted it, so state the measured shape instead: mostly one-to-one, with the exceptions named, and both exceptions found by measuring after writing down the tidy version. Added what the suite is not. Five of its fifteen cases exist because a reviewer found a hole it had not thought of, on a gate premised on ungated gates being no gates — each one the same species the gate exists to catch. Passing means these fifteen things are checked; it has never meant the list is complete, and the list's own history is the argument against reading it that way.
jeremy
added a commit
that referenced
this pull request
Aug 7, 2026
#679 landed while this branch was in flight. It repinned to 7fe1c63ab3 and absorbed project archive/unarchive, and it explicitly left the upload replacement to this branch — registering #12555 and parking its routes behind `registry:` waivers in the allowlist. So this repin's range is not the 72 commits it was when the branch started. 7fe1c63ab3 already contains #12555, and the range is now exactly one commit: #12565, the input contract. The triage is rewritten to say that, and the two waivers #679 parked are DELETED — absorption is what removes them, and a waiver matching nothing is a hard failure rather than a shrug. Three of the entries this branch wrote are gone because #679 wrote better ones from the same evidence: project-status-writes duplicated project-archive-unarchive, oauth-21-stack duplicated bc5-authorization-document-shape (which also splits #9471 into a discovery half that already matches and an authorization-document half that does not — a distinction this branch missed), and the pings preference is notifications-sort-pings-first. What did NOT duplicate is the taxonomy. #679 added ProjectLimitError for the project-limit 507 and correctly refused to reuse it for the storage one — but neither shape was classified. SPEC §6 had no 507 step, so both fell through `status >= 500` to api_error/retryable, and five SDK tests pinned that as intended behaviour, each with a comment saying "No SDK gives 507 a named class". They do now, and those tests assert limit_exceeded and non-retryable instead. Python's comment there recorded a real cross-SDK divergence: its fallback arm produced retryable=False while the other five marked every unclassified 5xx retryable, "asserted here as-is rather than fixed in passing". That divergence is gone — all six agree on False, and False is now the answer the spec gives rather than an accident of which arm caught the status. Operation count 249 -> 250: #679's two plus CreateUploadVersion.
jeremy
added a commit
that referenced
this pull request
Aug 7, 2026
…683) * Spec: replace an upload's file, and say which file each version is BC3 #12555 added POST /uploads/{id}/versions.json — replace an upload's file in place, keeping the recording's id, URL and comments — and gave each version event a nested upload object. #12565 then settled the endpoint's input contract. Absorb both, and repin to the revision containing them. CreateUploadVersion models attachable_sgid, base_name, description, notify and subscriptions. notify and subscriptions are documented contracts as of audience arrives either through notify naming a mode or through a bare subscriptions array. visible_to_clients is deliberately absent — #12565 removed it from the endpoint's reachable surface, because it never set visibility, only widened the notification audience. The read side is the half that fixes #649. ListUploadVersionsOutput declared uploads: UploadList, but the endpoint returns events: 11 of Upload's 14 required members are absent from every response, which is why the CLI's versions command and the MCP server's list_upload_versions render blank fields. It now returns UploadVersion — an event plus the file it recorded — built from _version.json.jbuilder rather than from the Upload shape it was pretending to be. New shapes rather than EventList plus a member, for the reason bc3's own commit gives for using a purpose-built partial: upload fields would otherwise leak onto todo, message and card events. StorageLimitError declares the 507 that ensure_account_can_upload_files has always been able to raise. It fronts four modeled operations, not one, so all four take it; absorbing the contract for the new operation alone would leave three declaring a status they can return and don't model. WebhookLimitError is left alone — it has the right body shape, but hanging a webhook-named error on an upload is the same kind of typed lie as #649. SPEC §6 gains limit_exceeded, and the 507 step is ordered ahead of the 5xx catch-all. Until now a 507 fell through to `status >= 500` and surfaced as api_error with retryable: true — a plan limit no retry can satisfy, reported as a transient server error. Nothing retried it in practice, since no retryOn list names 507; what was wrong was what the caller was told. Repin triage, recorded in spec/api-gaps/README.md: eight of the range's 72 commits touch the API surface. Two are absorbed here. Three are registered — project status writes (#12550 finally made archive and unarchive answer 204 instead of a 302 that 406'd), the OAuth 2.1 stack (#9471), and the CreateUploadInput.subscriptions field the endpoint has never read. Two need no SDK change, and #12566 confirms existing modelling rather than exposing drift. Two have no wire effect. One item no gate could have caught: #9471 added `resource :authorization` to routes.rb, so bc3 now serves /authorization.json itself. Its waiver still matches, because the parity gate reads doc/api and doc/api still names Launchpad's host — but the waiver's justification has decayed, which is exactly how waiver lists rot. Registered rather than left. * Go: CreateVersion, a real UploadVersion, and a reachable description clear ListVersions returned []Upload, decoding an event payload into a struct whose required fields it does not carry. It now returns []UploadVersion, with the nested file as UploadVersionFile. Details is carried through with the same present-empty semantics Event uses — the versions partial renders details via recordings/events/_event, which emits "details": {} for an event with no membership changes, so mapping that to nil would lose the difference between "no changes recorded" and "no details at all". Description becomes *string on both CreateUploadVersionRequest and UpdateUploadRequest. BC3 reads description with key?, so omitted carries forward and "" clears; behind a plain string and omitzero() the clear was unreachable, which is the divergence SPEC.md:369 documents. Fixing only the new request type would have put one request that can clear and one that silently cannot inside the same service. BaseName stays a plain string on both, and says why in its doc comment: Upload#base_name= guards on new_base_name.present?, so "" and absent are the same write server-side. There is no third state for a pointer to express — the asymmetry is a verified server fact, not an oversight. TestUpdateUploadRequest_HasNoFileReplacementField stays. #12555 added a dedicated route rather than widening PUT /uploads/{id}.json, so the guard now pins a design choice rather than a missing feature; its comment names CreateVersion as the sanctioned path, and a positive counterpart asserts CreateUploadVersionRequest carries the field the update refuses. Each new assertion was run against the un-fixed code first. The tri-state test fails with "the clear spelling must reach the wire"; the details test fails with "a present but empty details object must survive as non-nil". * All six SDKs: a 507 is a limit, not a retryable server error SPEC §6 gained limit_exceeded with the spec commit; this is the six implementations plus the tests that drove them. Every SDK mapped 507 through its 5xx catch-all, so an account out of file storage got api_error with retryable: true — indistinguishable, to a caller deciding whether to back off, from a 500. Each mapper now decides 507 before that catch-all, since both arms match and only order separates them. Nothing retried a 507 in practice (no retryOn list names it), so this changes what the caller is told, not what the client does. Ruby gains LimitExceededError, Python LimitExceededError (exported), Swift a .limitExceeded case, Kotlin a LimitExceeded class, Go CodeLimitExceeded, TS a "limit_exceeded" ErrorCode. Exit code 10 across all six. Kotlin's exhaustive `when` caught the omission in ErrorTest at compile time, which is the whole reason that test enumerates every subclass. The uploads tests came first and failed as intended — TypeScript reported "expected 'api_error' to be 'limit_exceeded'" before the mapper changed. Also here: the shared versions fixture, built from _version.json.jbuilder over recordings/events/_event.json.jbuilder rather than invented. Three elements covering all three actions, exactly one "current": true (the renderer passes current_event: @events.first), and one element with no upload object at all — its recordable no longer resolves, which is the optionality UploadVersion.upload declares. That element is deliberately not the one covering UploadVersionFile, since the manifest's concrete-instance rule needs a non-null representative. The TS, Ruby and Python listVersions tests were all asserting against Upload-shaped stubs the endpoint has never returned. They now read the fixture and assert on filename, byte_size, current, and the per-version download URL — the fields #649 was filed about. * Conformance: both uploads write paths, across all six runners Named for the surface rather than the operation, because it has to cover both writes: CreateUploadVersion and UpdateUpload land on the same serialized ActionText attribute, and only one of them had its presence semantics pinned anywhere. The uploads surface had zero requestBodyAbsent assertions before this. Each one rides with a requestBody presence assertion in the same case, following schedule_entries_write: a requestBodyAbsent alone is satisfied by an empty body, so it would pass while the SDK sent nothing at all. Seven cases — the three presence states for the replacement, the two UpdateUpload states its changed Go semantics require and no existing case covered, the read-side decode, and the 507. The 507 case is the one that had to go red first, and did: every SDK reported api_error with retryable: true before the taxonomy change. It pins requestCount at 1 as well, so a future retryOn list that added 507 fails here rather than silently burning an account's retry budget on a limit no backoff can lift. Two things the runners needed. The versions endpoint returns an ARRAY and every runner resolves a responseBody path as a top-level key only, so each flattens to the summary shape summarize_upcoming established rather than teaching six path resolvers to index arrays. And Swift's strict decoder rejected hand-rolled Upload bodies for missing `bucket` — the mock bodies are now built from spec/fixtures/uploads/get.json, so every byte a runner decodes traces to a coverage-guarded fixture. Swift's conformanceCode needed the new case, and knownErrorTypes with it: that set is documented as having to stay in sync, since a member missing from it silently forbids a real error rather than catching a typo. * Docs: the counts, the two breaks, and the divergence that narrowed Operation count 247 -> 248 in the four places it is restated. SPEC §5 gains uploads to the presence-aware list, naming both controllers and the BC3 tests that pin them, and says why base_name is deliberately NOT presence-bearing: Upload#base_name= guards on .present?, so "" and absent are the same server write and there is no third state to model. Saying so keeps the asymmetry legible as a verified fact rather than an oversight someone will "fix" later. The Go "use Edit or Replace to clear" divergence is narrowed rather than deleted — it still holds for UpdateTodolistRequest and UpdateDocumentRequest, and now explicitly does not for uploads. §10 One Renderer, One Schema gains UploadVersion beside the UpcomingScheduleEntry worked example. It is the second instance of the same failure and the sharper one: 11 of Upload's 14 required members are absent from every versions response. It also demonstrates the section's first corollary from the other direction — a reduced projection carrying a member (`upload`) no other event projection has. Appendix D gains four rows for uploads_write.json. MIGRATING gets an Unreleased section rather than edits to v0.13.0, whose counts are measurements at a fixed commit. Two breaks: the ListUploadVersions retype, which is a compiler-silent break in Ruby, Python and Kotlin because the type does not change — only which keys are actually present — and Go's Description pointer, which the compiler does catch. The 507 reclassification is flagged there too: not breaking, but a caller branching on api_error to decide whether to back off will no longer see storage limits in that branch, which is the entire point. API-GAP-404.md gets a resolution note that keeps its finding intact. PUT /uploads/{id}.json still ignores attachable_sgid — the hypothesis that document tested is still false, which is why the guard survives. Gate fixes from the full run: JSON tags on the new Go wrapper structs (the wrapper-drift checker maps fields by tag, and untagged ones read as dropped), a doc comment that merged with its neighbour's, gofmt, one rubocop refute->assert_not, and ruff import order and formatting. * Reconcile with #679's repin, and finish the 507 it left half-classified #679 landed while this branch was in flight. It repinned to 7fe1c63ab3 and absorbed project archive/unarchive, and it explicitly left the upload replacement to this branch — registering #12555 and parking its routes behind `registry:` waivers in the allowlist. So this repin's range is not the 72 commits it was when the branch started. 7fe1c63ab3 already contains #12555, and the range is now exactly one commit: #12565, the input contract. The triage is rewritten to say that, and the two waivers #679 parked are DELETED — absorption is what removes them, and a waiver matching nothing is a hard failure rather than a shrug. Three of the entries this branch wrote are gone because #679 wrote better ones from the same evidence: project-status-writes duplicated project-archive-unarchive, oauth-21-stack duplicated bc5-authorization-document-shape (which also splits #9471 into a discovery half that already matches and an authorization-document half that does not — a distinction this branch missed), and the pings preference is notifications-sort-pings-first. What did NOT duplicate is the taxonomy. #679 added ProjectLimitError for the project-limit 507 and correctly refused to reuse it for the storage one — but neither shape was classified. SPEC §6 had no 507 step, so both fell through `status >= 500` to api_error/retryable, and five SDK tests pinned that as intended behaviour, each with a comment saying "No SDK gives 507 a named class". They do now, and those tests assert limit_exceeded and non-retryable instead. Python's comment there recorded a real cross-SDK divergence: its fallback arm produced retryable=False while the other five marked every unclassified 5xx retryable, "asserted here as-is rather than fixed in passing". That divergence is gone — all six agree on False, and False is now the answer the spec gives rather than an accident of which arm caught the status. Operation count 249 -> 250: #679's two plus CreateUploadVersion. * Address Codex review: keep listVersions typed in TS and Kotlin Codex was right on all three, and the second one caught a claim I had verified backwards. Registering UploadVersion in the TypeScript and Kotlin generators' TYPE_ALIASES — not patching the generated files, which AGENTS.md forbids and which the next regeneration would undo. TypeScript went from Promise<ListResult<Upload>> to the raw response-schema type. The runtime still returned a ListResult, so .meta.totalCount kept working while no longer type-checking — a silent break my own tests missed because they only index the array. It is ListResult<UploadVersion> now. Kotlin went from ListResult<Upload> to ListResult<JsonElement>, a real downgrade to untyped. MIGRATING claimed that row was "unchanged — bare-array responses were already untyped here"; that was wrong, and wrong because I grepped the file AFTER the change and read the result as the prior state. Checking origin/main was the whole job. Kotlin now emits UploadVersion and UploadVersionFile models and returns ListResult<UploadVersion>, so the conformance summarizer reads decoded models — which makes that case a decode test in Kotlin as it already was in Swift, since kotlinx.serialization rejects a body missing any non-nullable member. Third: the new error code is source-breaking in three SDKs and MIGRATING called the whole 507 change non-breaking. Adding to TypeScript's ErrorCode union, Swift's enum and Kotlin's sealed class each breaks exhaustive handling — this repo's own Kotlin ErrorTest failed to compile on exactly that, which is what that test exists to produce. Documented per SDK, with the reason Go/Ruby/Python still need reading for: a default arm now catches storage and project limits. Also corrected there: the 507 reclassification reaches CreateProject and UnarchiveProject, which shipped in v0.13.0 as retryable api_error. The mapping is by status, not by operation. * Address Codex round 2: export the new TS types, and Python does break Both correct. typescript/src/index.ts exported Upload, CreateUploadRequest and UpdateUploadRequest but none of the new types, so UploadVersion and CreateVersionUploadRequest appeared in public method signatures with no supported import through which a consumer could name them. That is the client-wiring step of this work, and I checked the method was exported without checking its types were. And MIGRATING said Python could not break a build. ErrorCode is a StrEnum, so a consumer matching it exhaustively and closing with typing.assert_never gets a mypy failure on the new member — a break that needs a type-checker to surface rather than an interpreter, which makes it easier to miss than the three that stop a compiler, not less real. Four SDKs now, with the distinction spelled out. * Address Codex round 3: map 507 in Go's raw request path too Go has two response handlers and I only changed one. checkResponse covers the generated service layer; doRequest backs the raw Client.Get/Post/Put/ Delete escape hatch, and its default arm was still returning api_error for a 507. The 400/422 arm sitting just above the new one exists because exactly this happened before — its comment says the raw path "used to fall through to the default arm and report api_error with the field-keyed detail dropped". Same shape, same fix, and the arm is ordered ahead of the 5xx cases for the same reason as everywhere else. The test fails against the un-mapped path with `code = "api_error", want "limit_exceeded"`. * Address Codex round 4: filtering on blob_changed drops the original file The guidance was backwards, and the fixture in this PR shows it. An upload's ORIGINAL file is recorded by the created or active event; only replacements are blob_changed. So "filter on blob_changed to list past versions" returns the replacements — which excludes the original and INCLUDES the current file, the exact inverse of what it reads like. bc3's own test names it: "versions carry the file each version recorded" destructures `replacement, original` and asserts original["action"] is "created". Past versions are the entries carrying an upload whose current is false, which is what the Smithy doc, the Go doc comment and MIGRATING now say. Pinned by a test over the shared fixture, which has exactly the shape that makes the two selections differ: current-false picks the active entry, and the blob_changed shortcut picks the current file and no history at all. * Say what `current` actually is, without weakening it Codex read the two sentences on `current` as contradictory and concluded the exactly-one guarantee was false. The guarantee holds; the prose was what misled. `current` is `event == @events.first` over a reverse-chronological list — positional, so exactly one element of any non-empty response carries it, never zero and never plural. bc3 pins the specific case the old wording made sound impossible, in a test named "exactly one version is current after a metadata-only update", asserting the count is 1. What the caveat was actually about is a different question: `current` does not mean "the file the upload's own download_url serves". A metadata-only PUT swaps in a recordable carrying the same blob and emits no event, so afterwards no event references the upload's current recordable — and exactly one element is still current. Both docs now separate the two claims instead of running them together. Not taking the suggested fix. Describing `current` as "whether this event references the current recordable" and allowing zero matches would document an invariant weaker than the server's, and push every consumer to handle a case that cannot occur — the conformance case asserting current_count == 1 would have to go with it. * Address Codex round 5: the six READMEs are public error references too All six error tables omitted limit_exceeded / exit code 10, and the Swift and Kotlin READMEs print exhaustive examples — a `switch` with no default and a `when` over the sealed class — that would no longer compile if a reader pasted them. Those are the same break MIGRATING now documents, shipped in the docs that teach it. Both examples gain a LimitExceeded arm, each saying the thing a caller needs at the point of handling: 507 is an account limit, not a transient failure, so do not retry. Also corrected while in there: the TypeScript, Swift and Kotlin tables said api_error covers "5xx", which stopped being true the moment 507 got its own code. They now name the statuses. * Account for CreateUploadVersion on the grouped client (#682) #682's new gate caught this branch, which is what it was built for: the inventory's invariant is that every operationId appears exactly once across `grouped:` or `not_grouped:`, so a new operation lands in neither and fails loudly rather than shipping missing — the way ArchiveProject and UnarchiveProject did through two green `make` runs. Filed under `not_grouped:`, next to `ListUploadVersions`. The grouped surface is the low-level generated client and is deliberately curated rather than exhaustive; the versions read is already off it, and putting the write on while its sibling read stays off would be the inconsistency the inventory exists to prevent. The ergonomic surface for this is UploadsService.CreateVersion in go/pkg/basecamp, which is unaffected. * Correct SPEC §2's retry.max distribution: 44 -> 45 at max 2 CreateUploadVersion carries retry.max 2, so the behavior-model breakdown is 205/45. §2 still said 205/44, which summed to 249 against a document that says 250 nine hundred lines later — a self-inconsistency an operation-count audit would read as drift. Derived from behavior-model.json rather than incremented by hand: `Counter((o['retry'] or {})['max'] for o in operations)` gives {3: 205, 2: 45}. Only that count moves. The 11 / 197 / 208 figures in the two bullets below it are about IDEMPOTENT operations — the ones that get retried at all — and CreateUploadVersion is a non-idempotent POST like CreateUpload, so it joins neither the named eleven nor the 208. Checked rather than assumed: behavior-model marks Subscribe and MarkAsRead idempotent and both CreateUpload and CreateUploadVersion not. * Reconcile the rest of the derived counts, by sweeping instead of patching Non-idempotent 166 -> 167 in §2's Operation Counts. My last commit said "only that count moves" and was wrong: I checked the two bullets next to the number I was fixing and not the section nine hundred lines down that derives from the same file. So this time the whole set was derived from behavior-model.json and compared against every stated figure, rather than the one Codex pointed at: total 250, idempotent 83, non-idempotent 167, retry.max {3: 205, 2: 45}. All now reconcile, and 83 + 167 = 250 the way 83 + 166 did not. MIGRATING's Unreleased section gains the inventory line the file's own convention asks for — 249 -> 250, with the two commands to derive each end rather than trust either. Its earlier 247/249 figures are untouched: those are as-of facts about shipped releases, not claims about today. * List all eight 507 operations, not the four this PR touched The reclassification is keyed on status, so it reaches every operation the spec gives a 507 — including CreateWebhook and UpdateWebhook, which have declared WebhookLimitError since long before this branch and were reporting it as a retryable api_error the whole time. Webhook and project callers therefore need the same new branch even though nothing about those endpoints changed, which is exactly the kind of thing a migration guide exists to say and mine did not. The list is now a table of all eight across the three limits, derived from openapi.json rather than recalled, with the query to re-derive it. That is also what makes the omission embarrassing rather than subtle: the six README tables this PR already updated say "file storage, projects, webhooks" — I knew the set and then wrote a shorter one here. * Sweep the whole tree for stale operation counts, not the named files Third round on this class, so this time the sweep is the fix rather than the two references Codex pointed at. SECURITY.md said 249 operations and 41 non-idempotent POSTs; both move to 250 and 42. The rest of that paragraph checks out and is untouched — 125 GETs, 83 idempotent mutations, 52 PUTs, 24 DELETEs, 7 flagged POSTs, and the seven named are exactly the seven behavior-model flags. Verified rather than assumed, since the whole point of the last two rounds is that I patched what I was shown and not what was true. scripts/check-grouped-client-coverage's floor comment cited 249 as the real value. Not load-bearing — MIN_OPERATIONS is 200 and catches extraction collapse, not drift — but it is a current-value claim in prose and it was wrong. The sweep: every tracked file except MIGRATING (as-of history), generated output and lockfiles, scanned for 247/248/249 within ninety characters of "operation", "POST", "route" or "surface". Zero remaining.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Customer support asked for project archiving through the API. The routes always existed, but
Projects::StatusControllerhad norespond_to, so a JSON request got a 302 to an HTML URL and following it returned 406. BC3 #12550 added the JSON branch (head :no_content) plus 32 lines ofdoc/api/sections/projects.md; this absorbs it.The repin and the absorption are one change on purpose, following the
9459083eaFolders precedent.doc/api/sections/projects.mdgains "Archive a project" and "Unarchive a project" only in #12550, so at the old pin the two routes are absent fromspec/bc3-routes.jsonentirely. Absorbing alone would model a contract the pin says isn't there; repinning alone would leave two documented routes with no operation behind them. Regenerating the table at the new pin is what lets direction 1 ofbc3-route-paritypass on the truth instead of on a waiver — and it does, with zero newsdk_routes_absent_from_bc3_docsentries.247 → 249 operations.
The repin:
4e34dc83eb..7fe1c63ab3, 71 commitsUpstream moved four times across planning and implementation —
4e34dc83eb→6f4781bbd4→a26c2e479f→7fe1c63ab3— every one adoc/api/commit, and the last landed after the absorption was complete and committed. Hence two repin commits in the history rather than one; the final commit narrates the sequence.Classifying all 71 commits by touched path — a filter on
doc/api/+config/routes.rb+app/views/api/is a filter, not a certification — finds six contract-or-documentation changes. Three touchdoc/api/; only two move the route table. The residual bucket is empty.6f4781bbd4) project status JSONa26c2e479f) upload file replacementupload-versions-api98eb24b22f)sort_pings_firstpartial-coverageeac8b2b476) OAuth 2.15c0e774b0d+ 3 siblings, completions denormalization7fe1c63ab3)statusread-only on updateTwo of the six are the reason the sweep is a classification and not a glob:
doc/api, noconfig/routes.rband noapp/views/apidiff at all, yet reroutesRecording.completed→completed_recently_first(reorder("completions.created_at DESC")), reordering three modelled operations:GetEverythingCompletedTodos,GetEverythingCompletedCards,GetMyCompletedAssignments. No field added, removed or retyped, and none of the three declares an ordering in its Smithy docs, so there is nothing to correct — but a path filter would have missed it entirely. The pairedmy/assigningsandmy/assignmentscontrollers are not a rename: both exist before and after, one line each.doc/apiwithout moving the route table — two lines of prose on an endpoint already documented. So "threedoc/apicommits" and "four added rows" are both correct; the triage now says so explicitly rather than leaving a reader to reconcile them.api_request.rbis touched by #9471, but only to add a privateapp_url_optionshelper for OAuth's HTML redirects —api_request?andrestrict_view_paths_to_api_rootare untouched, so what counts as an API route is unchanged. #12475 registersGalleryas adock_tool, which is additive becauseDockItem.nameis a plainStringand not an enum, and noapp/views/api/galleriestemplate exists. Full sweep inspec/api-gaps/README.md.OAuth was the gate item, and it did not come back clean
Discovery matches, no action: bc3's new
.well-known/oauth-authorization-server(RFC 8414) and.well-known/oauth-protected-resource(RFC 9728) are the exact paths the SDK's hand-written discovery already fetches —go/pkg/basecamp/oauth/discovery.go:24-25,typescript/src/oauth/discovery.ts:400,514,python/src/basecamp/oauth/discovery.py:223,301,ruby/lib/basecamp/oauth/discovery.rb:88.The authorization document diverges, and it is reachable today. bc3 now draws
resource :authorization, only: :showand rendersapp/views/api/authorizations/show.json.jbuilder, whose shape is not Launchpad's:identitycarries onlyid, accounts carryresourcebut neitherproductnorapp_href, andexpires_atis integer epoch seconds rather than ISO-8601. Ruby'sHttp#get_authorization_documentbinds to the discovered BC5 issuer and fetches{issuer_origin}/authorization.json, so this is not hypothetical.TypeScript is the exposed SDK: three required
stringfields becomeundefined,filterProductsilently matches nothing (breaking the documented "pickproduct: bc3" flow), andnew Date(epochSeconds)reads the integer as milliseconds and yields a 1970 date instead of throwing. Go already tolerates the timestamp viaFlexTime; Python and Ruby return untyped maps. Registered asspec/api-gaps/bc5-authorization-document-shape.md— OAuth is outside the OpenAPI spec by design, so nothing here drifted from Smithy.The absorption
ArchiveProject/UnarchiveProjectcopyArchiveRecording/UnarchiveRecording— same verbs, same URI suffixes, same 204 — withrecordingId: RecordingId→projectId: ProjectId. Both@idempotent+@basecampIdempotent(natural: true): re-archiving an archived project is a no-op that still answers 204.Error lists decided, not copied. Both take
[NotFoundError, UnauthorizedError, ForbiddenError, InternalServerError]— matchingTrashProject, the nearest project-status operation, and notArchiveRecording's list, which carries aValidationErrorthese cannot raise because they have no request body.UnarchiveProjectadds the 507.403 on both is deliberate, and corrects the registration draft.
forbid_clientsis an unscopedbefore_action(app/controllers/concerns/permissions.rb:22-24), so it fires onactiveexactly as onarchived. The asymmetry is in the cause — onlyensure_can_archive_or_trash_projectisonly: %i[archived trashed]— not in reachability, and bc3's own tests split on exactly that line ("unarchive is permitted for a non-admin or creator").ProjectLimitErroris a new@httpError(507)shape wired intoUnarchiveProjectandCreateProject— the same body was already reachable from create and unmodelled, so one shape closes two gaps. Two shapes sharing 507 is safe because no SDK maps status → shape; all six switch on the status code into a fixed taxonomy. The invariant that matters holds: no operation's error list contains two shapes with the same status.statusis deliberately not modelled as writable onUpdateProject— it's absent fromcreate_project_params, so bc3 silently drops it. #12566 has since made that bc3's documented contract, so the omission is now backed by upstream prose rather than a controller read.Tests: all six SDKs
No generator emits tests. Happy path 204 → void/nil/None, plus two error cases each: 403 on archive and 507 on unarchive — the latter is the new shape's only behavioural evidence, surfacing as a generic
api_errorwithhttp_status: 507(SPEC §7), since no SDK gives 507 a named class. Assertions check status and error code, not just the exception class.Python had no trash test to copy, so this establishes the pattern there across both the sync and async classes the generator emits. Go had none either; its table test covers
Trashalongside the two new wrappers. Go wrappers are mandatory, not optional —EXCLUDED_OPSis empty by policy andgo-check-driftflagged 247/249 until they landed.Python's 507 test asserts
retryable is Falsewhere Ruby's and Kotlin's assert true. That's not an inconsistency in the tests: Python's fallback arm builds a bareApiErrordefaulting toretryable=Falsewhile the other five mark every unclassified 5xx retryable. Pre-existing divergence from SPEC §7, asserted as-is and left alone.Nothing added to
conformance/tests/live-my-surface.json— read-only live surface, and archiving is destructive.Counts
Re-derived from
behavior-model.jsonandopenapi.jsonrather than incremented, and they cross-check: 52 PUTs + 24 DELETEs + 7 idempotent POSTs = 83 idempotent; 83 + 125 readonly = 208 union.scripts/check-idempotency-parityis the one enforced count (81 → 83, 206 → 208). Prose:SPEC.md§Operation Counts and its two "all 247 operations" mentions, the easily-missed §2 retry distributions (203 → 205 atmax:3; 195 → 197 unaffected at the default cap; 206 → 208 clamped above it),SECURITY.md(50 → 52 PUTs), andAGENTS.md.MIGRATING.mddeliberately gets nothing — its four247s are historical v0.13.0 measurements, and new operations break nobody.Two traps worth knowing
Generation succeeds at the wrong count. Defining and tagging the operations regenerated cleanly at exit 0 and emitted 247 with
ProjectsServicestill at 5 — the new operations must also join the Smithy service shape'soperations:list, and nothing in the output names the omission. Check the operation count before trusting a clean regeneration.One allowlist entry covers both upload spellings. Direction 2's failure named both the flat and bucket-scoped
POST .../versionsroutes, so two entries looked right; the bucket-scoped one then hard-failed asmatches nothing, because direction 2 collapses a leading/buckets/:id. Established by running the gate, and the comment records it.Also:
bc3-route-parity's staleness guard sits upstream of both directions, so there is no observable direction-1 failure between repinning andmake bc3-routes— it compares nothing and just says the table is stale. The direction-1 proof is therefore the absence on the green run: zero new waivers, with the two PUTs arriving as additions in thespec/bc3-routes.jsondiff.Verification
make is green locally (all 43 check-targets). CI run 31069828400 reports the following authoritative test totals:
The CodeQL Advanced Analyze (swift) job was still in progress when this description was updated; all other required CI checks were passing.
make bc3-routes-checkis not incheck-targetsand not in CI because it needsBC3_REPO_PATH; it was run by hand at each repin and reports the table up to date at7fe1c63ab3(373 routes, 64 sections).Nothing in CI makes a live HTTP call, so no green result here is evidence the endpoints work. The end-to-end proof is bc3's, split across two files that prove different things — cite both, neither covers the other's case:
test/api/projects/status_controller_api_test.rb— the 204s, the 403s, and the behaviour itself, including "unarchive restores a trashed project".gems/saas/test/api/projects/status_controller_api_test.rb— the only 507 proof: "unarchive enforces the project limit" on a free-plan account, asserting:insufficient_storageand the exact bodyProjectLimitErrormodels.Summary by cubic
Adds project archive and unarchive to the Projects service across all six SDKs with idempotent PUT endpoints that return 204. Repins the BC3 API to 2026-08-05 and regenerates clients, tests, and docs; also ensures Go’s grouped generated client now exposes these methods.
New Features
archive(projectId)andunarchive(projectId)in Go, TypeScript, Python, Ruby, Swift, and Kotlin./projects/{projectId}/status/archived.jsonand PUT/projects/{projectId}/status/active.json(204 No Content).ProjectLimitError(507) response; surfaced byCreateProjectandUnarchiveProject.Bug Fixes
ArchiveandUnarchiveongenerated.Client.Projects(); template updated to include these methods.Written for commit ca67bb1. Summary will update on new commits.