MIGRATING.md: the v0.13.0 upgrade guide, silent breaks first - #642
Conversation
github-actions
Bot
added
typescript
![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: 7dd141feee
ℹ️ 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".
…#643 Addresses both P1 review threads on #642 and folds in the two PRs that landed since the first draft. Pagination (P1). Cross-SDK item 1 claimed `page` was a starting offset in every SDK and told readers to drop it to restore the old walk. For Go that was actively harmful: `git show v0.12.0:go/pkg/basecamp/bookmarks.go` returns before followPagination whenever page > 0, so a positive Page already meant one request, and dropping it converts a bounded call into a full account-wide traversal. The item is now scoped to the five SDKs where it holds — re-checked at the tag rather than assumed, since the universal claim had already failed once — with a Go subsection splitting the two real cases: services where the page number was already honored (Bookmarks, Drafts, Everything*, request unchanged) and the fourteen carrying the "not yet honored" doc, which sent no page at all and returned page 1's rows. Gauges is in neither; it had no page. Raw wire (P1). The Forwards().CreateReply example built a path with fmt.Sprintf and called the raw AccountClient.Post against a route with no upstream coverage, which is what AGENTS.md "Never Do These" 4 and 5 forbid. Removed rather than softened, and replaced with a known-gap section stating what a hand-built path gives up. Swept the document: the one other hit documents a real change to the raw client's error codes, so it stays, but its fabricated path is gone and it now says it is not a suggestion to reach for the escape hatch. #643 landed, so basecamp.Ptr and basecamp.Deref replace the hand-rolled ptr helper throughout, the Go section opens with the 300-pointer census and a command that reproduces it, and ParticipantIDs *[]int64 gets its own note: nil leaves participants alone, a pointer to an empty slice removes every one. #637 landed and does NOT add a break to any SDK. color and comments_app_url did not exist on Todolist at v0.12.0 in any of the six — both arrived with #628 earlier in this same release — so from the guide's baseline nothing turned from optional to required. Counts stay 27/20/16/14/16/14. Documented where it bites: color is required-and-nullable so explicit null decodes, comments_app_url rejects null and absence alike. Also: kotlin/README's append-only source-compat promise contradicted this release repeatedly, so it now describes documented pre-1.0 breaking correctness releases; the binary-compat disclaimer is kept and sharpened. release-github.yml links MIGRATING.md from every release body, guarded on the file, so the link cannot be forgotten at tag time. "Silent" is defined as source/runtime-silent against a live server, since a suite pinning request paths does catch some. Counts are stated as-of 51d0d86 with derivations inline, and each in-flight change names the numbers it invalidates so the pre-tag pass is arithmetic.
Sensitive Change Detection (shadow mode)This PR modifies control-plane files:
|
github-actions
Bot
added
the
github-actions
|
@codex review Both P1s from the
Also folded in since that commit: #637 and #643 (both merged), a rewrite of Worth a specific look: I concluded #637 adds no break to any SDK, because |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce91ee7848
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (1)
MIGRATING.md:1057
- This says "Three new optional members are additive" but names only one (
groupPositionUrl). The next section (Todolist.colorand.commentsAppUrlare new and required, #637) establishes that the other two newTodolistmembers are required, not optional — so there is exactly one new optional member here. The parallel TypeScript section states this correctly as singular ("Also new and optional:group_position_url"). Consider correcting the count to avoid contradicting the following section.
`URLProtocol` stub or cassette or decoding throws. Three new optional members are
additive: `groupPositionUrl`.
…and cards Addresses the remaining P2 and a suppressed Copilot comment on #642, re-derives every count against main, and writes the cards due-date change. The P2 was right, and it was a contradiction with this guide's own definition rather than loose wording: "silent" was defined as "does not raise" and then used to file nil-pointer panics. The section is now "Breaks your compiler will not catch" — the property all of it actually shares — split into class A, no signal at all, and class B, compiles then panics or raises but only when a particular field is absent, so it passes every test where that field is populated. Applying the definition consistently moved four entries, not the three flagged: the three Go pointerization panics plus Ruby's Draft#scheduled_posting_at decode, which raises NoMethodError and TypeError and had the same defect. Two moved entries carry real no-signal residue, kept as sub-notes rather than double-counted. Per SDK: Go 8A/3B, Swift 9A, TypeScript 5A, Python 4A, Ruby 2A/1B, Kotlin 3A — 31 + 4 = 35, unchanged in total. Body counts verified against the table by parsing the section, not by eye. The Swift section claimed three new optional Todolist members and named one; the other two are required. Now singular, matching TypeScript. Counts re-derived at 9de44b2: the inventory is 238 -> 247, not 241, since #629 merged. Added, removed and route-moved lists are computed from openapi.json at both ends rather than hand-edited — 14 IDs added, 5 removed, 11 same-ID moves — and the Folders operations are flagged as drawn at /stacks, not /folders. Cards get their own section. The half that matters most is true in production today and is not caused by upgrading: every released SDK encodes "clear a card due date" as omission, bc3 stopped treating omission as a clear, so that call is a silent no-op right now. That is a reason to upgrade rather than a hazard of it, so it sits in the operator checklist. The SDK-side change is read from bf43715 and marked unmerged: single PUT, "due_on": "" as the clear encoding, UpdateStepRequest.DueOn becomes *string, and the GetCard preservation read goes away. The hook collapse is written as the inverse of the {Todolists,Update} split because it fails the opposite way — allowlists do not start denying, but a denylist on {Cards,Get} silently stops blocking the write it used to take down. Removing the preservation GET also removes three named errorRaised kill cases from cards_write.json; the class stays pinned on Todos, which still does a real read-modify-write, so that is said rather than filed as a redundant-GET cleanup.
github-actions
Bot
added
the
github-actions
|
@codex review All three prior threads are fixed and resolved as of The section is now Breaks your compiler will not catch, split into class A (no signal at all, 31) and class B (compiles, then panics or raises, but only when a particular field is absent — 4). Applying the definition consistently moved four entries, not the three flagged: the three Go pointerization panics plus Ruby's Specific things worth checking:
Also fixed the suppressed comment at |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ac6f4475d
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (1)
README.md:7
- This root-level summary says all 4 runtime breaks fail "only on a payload that omits a particular field," but that only holds for the three Go breaks. Ruby's Class B break fails on the opposite condition — a record where the field is populated (the timestamp decodes to
Time, soStringmethods raise); see MIGRATING.md:484-487 and the Ruby README, which correctly says "one that raises only on a record where the field is populated." The aggregate "omits a particular field" wording mischaracterizes that one case.
**Upgrading?** Read [MIGRATING.md](MIGRATING.md) *before* you bump the version. v0.13.0 breaks all six SDKs, and 35 of those breaks survive a clean build — 31 give no signal at all, and 4 compile and then fail at runtime only on a payload that omits a particular field.
Fourth review round on #642. Four findings, all upheld. The allowlist framing was wrong in the direction that matters. I wrote that fewer hook events are safe for an allowlist. True only if the allowlist named both operations: one that names UpdateCard and deliberately omits GetCard used to reject cards.update at its read, and after the collapse permits it end to end. Both policy shapes now carry the warning, labelled, plus the observation that they are the same hole seen twice — in each, the thing stopping the write was the read, expressed once as an omission and once as an entry. The class-A counting was inconsistent across all six SDKs, not the two flagged. Python and Kotlin excluded changes their own prose called "no signal whatsoever"; auditing every SDK against the definition moved the totals to 47 class A and 4 class B. The counting policy is now stated in the document so it can be checked against a rule rather than an impression: one entry per distinct change per SDK, counted where it bites; class A if any ordinary call-site shape stays silent even when another is compile-caught; second faces annotated as residue and counted once; raises-only-on-malformed-response is class B. Two things fell out that were not counting problems. Ruby's #563 was missing from the guide entirely — no mention of download_url anywhere in the chapter — verified against source rather than prose: v0.12.0 http.get_no_retry, which sent Accept: application/json and did not retry, became get_download calling request_with_retry with retry_on: DOWNLOAD_RETRY_ON and accept: nil. Ruby now has its own section. The same check confirmed Go's omission of #563 is correct, because Go already retried at v0.12.0. Separately, the Go note claiming the compiler catches only the pkg/generated half of Schedules().UpdateEntry was false: UpdateScheduleEntryRequest's fields became pointers, so any pkg/basecamp call site that set a field fails to build. The class-B definition described only half its own membership. It said the trigger is an absent field, but Ruby's entry fires only when the field is populated. It now says both, and says plainly that class B is a property of a call plus a response rather than of the call — the same method against the other shape is not a break at all. Class A has no such dependency. Stale counts in the chapter intros are fixed. The Go intro still said eleven silent and two panics, which is the first thing a #go link shows, and Swift claimed the most no-signal breaks, which stopped being true at Go ten. Also folds in #652 (projected-example gate, stacked on #648, takes check-targets to 43), moves #648 out of draft at cb438ce, and records that #647 is being reworked Smithy-first because the generated UpdateCardStepRequestContent.DueOn is *types.Date and cannot express "". The consumer-facing card shape is unaffected by that rework. Re-derived against #648: 238 -> 247 with 14 added, 5 removed and 11 same-ID route moves survives unchanged.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 438ee2a70d
ℹ️ 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".
v0.13.0 breaks all six SDKs and 35 of those breaks are silent — no compile error, no exception, no decoder failure. Label-generated release notes list what merged; they cannot say what a consumer must react to or what wrong behaviour they get if they ignore it. That had no home in this repo. Adds MIGRATING.md at the root, linked from the root README and all six per-SDK READMEs. Silent breaks lead the document, then one section per SDK ordered by severity, plus an operator checklist, a "coverage: corrected and re-scoped" section for what did not ship, and known gaps. No CHANGELOG is reintroduced. The hand-maintained ones were deleted in #115 as superseded by auto-generated notes, and every release body since is machine-built. CONTRIBUTING records the resulting rule: label-generated notes say what merged, MIGRATING says what to do about it. Corrections to the source drafts, each re-derived rather than repeated: - TrashTodo was not a 404. bc3 draws `resources :todos, only: %i[show edit update destroy]`; DELETE /todos/:id returned 204 and set status to "archived", so every caller was archiving. It is the one #619 removal that takes away a working call, and it now carries its own carve-out. - #619 removed three operations, not nine. Nine were re-pathed. Fusing the two sets is what made the blanket 404 reassurance look safe. - Hook operation identity differs by SDK: Go and Ruby emit a short verb, the other four emit the wire operation ID, where the todolist pair kept its names — so an allowlist holding UpdateTodolistOrGroup passes the write and denies the new read. - 238 -> 241 measured at the v0.12.0 tag and at c95d81c, not assumed. - Kotlin binary compatibility is already disclaimed in kotlin/README.md; Swift has no written policy. Both are now stated rather than left unsaid. recordings.get is documented as a known gap with a list-and-filter recipe and its honest cost. The Go recipe compiles against this tree. #637, #629 and #635/#641 were open at the time of writing and are recorded under "Not in this release" rather than described as shipped.
…#643 Addresses both P1 review threads on #642 and folds in the two PRs that landed since the first draft. Pagination (P1). Cross-SDK item 1 claimed `page` was a starting offset in every SDK and told readers to drop it to restore the old walk. For Go that was actively harmful: `git show v0.12.0:go/pkg/basecamp/bookmarks.go` returns before followPagination whenever page > 0, so a positive Page already meant one request, and dropping it converts a bounded call into a full account-wide traversal. The item is now scoped to the five SDKs where it holds — re-checked at the tag rather than assumed, since the universal claim had already failed once — with a Go subsection splitting the two real cases: services where the page number was already honored (Bookmarks, Drafts, Everything*, request unchanged) and the fourteen carrying the "not yet honored" doc, which sent no page at all and returned page 1's rows. Gauges is in neither; it had no page. Raw wire (P1). The Forwards().CreateReply example built a path with fmt.Sprintf and called the raw AccountClient.Post against a route with no upstream coverage, which is what AGENTS.md "Never Do These" 4 and 5 forbid. Removed rather than softened, and replaced with a known-gap section stating what a hand-built path gives up. Swept the document: the one other hit documents a real change to the raw client's error codes, so it stays, but its fabricated path is gone and it now says it is not a suggestion to reach for the escape hatch. #643 landed, so basecamp.Ptr and basecamp.Deref replace the hand-rolled ptr helper throughout, the Go section opens with the 300-pointer census and a command that reproduces it, and ParticipantIDs *[]int64 gets its own note: nil leaves participants alone, a pointer to an empty slice removes every one. #637 landed and does NOT add a break to any SDK. color and comments_app_url did not exist on Todolist at v0.12.0 in any of the six — both arrived with #628 earlier in this same release — so from the guide's baseline nothing turned from optional to required. Counts stay 27/20/16/14/16/14. Documented where it bites: color is required-and-nullable so explicit null decodes, comments_app_url rejects null and absence alike. Also: kotlin/README's append-only source-compat promise contradicted this release repeatedly, so it now describes documented pre-1.0 breaking correctness releases; the binary-compat disclaimer is kept and sharpened. release-github.yml links MIGRATING.md from every release body, guarded on the file, so the link cannot be forgotten at tag time. "Silent" is defined as source/runtime-silent against a live server, since a suite pinning request paths does catch some. Counts are stated as-of 51d0d86 with derivations inline, and each in-flight change names the numbers it invalidates so the pre-tag pass is arithmetic.
…and cards Addresses the remaining P2 and a suppressed Copilot comment on #642, re-derives every count against main, and writes the cards due-date change. The P2 was right, and it was a contradiction with this guide's own definition rather than loose wording: "silent" was defined as "does not raise" and then used to file nil-pointer panics. The section is now "Breaks your compiler will not catch" — the property all of it actually shares — split into class A, no signal at all, and class B, compiles then panics or raises but only when a particular field is absent, so it passes every test where that field is populated. Applying the definition consistently moved four entries, not the three flagged: the three Go pointerization panics plus Ruby's Draft#scheduled_posting_at decode, which raises NoMethodError and TypeError and had the same defect. Two moved entries carry real no-signal residue, kept as sub-notes rather than double-counted. Per SDK: Go 8A/3B, Swift 9A, TypeScript 5A, Python 4A, Ruby 2A/1B, Kotlin 3A — 31 + 4 = 35, unchanged in total. Body counts verified against the table by parsing the section, not by eye. The Swift section claimed three new optional Todolist members and named one; the other two are required. Now singular, matching TypeScript. Counts re-derived at 9de44b2: the inventory is 238 -> 247, not 241, since #629 merged. Added, removed and route-moved lists are computed from openapi.json at both ends rather than hand-edited — 14 IDs added, 5 removed, 11 same-ID moves — and the Folders operations are flagged as drawn at /stacks, not /folders. Cards get their own section. The half that matters most is true in production today and is not caused by upgrading: every released SDK encodes "clear a card due date" as omission, bc3 stopped treating omission as a clear, so that call is a silent no-op right now. That is a reason to upgrade rather than a hazard of it, so it sits in the operator checklist. The SDK-side change is read from bf43715 and marked unmerged: single PUT, "due_on": "" as the clear encoding, UpdateStepRequest.DueOn becomes *string, and the GetCard preservation read goes away. The hook collapse is written as the inverse of the {Todolists,Update} split because it fails the opposite way — allowlists do not start denying, but a denylist on {Cards,Get} silently stops blocking the write it used to take down. Removing the preservation GET also removes three named errorRaised kill cases from cards_write.json; the class stays pinned on Todos, which still does a real read-modify-write, so that is said rather than filed as a redundant-GET cleanup.
Fourth review round on #642. Four findings, all upheld. The allowlist framing was wrong in the direction that matters. I wrote that fewer hook events are safe for an allowlist. True only if the allowlist named both operations: one that names UpdateCard and deliberately omits GetCard used to reject cards.update at its read, and after the collapse permits it end to end. Both policy shapes now carry the warning, labelled, plus the observation that they are the same hole seen twice — in each, the thing stopping the write was the read, expressed once as an omission and once as an entry. The class-A counting was inconsistent across all six SDKs, not the two flagged. Python and Kotlin excluded changes their own prose called "no signal whatsoever"; auditing every SDK against the definition moved the totals to 47 class A and 4 class B. The counting policy is now stated in the document so it can be checked against a rule rather than an impression: one entry per distinct change per SDK, counted where it bites; class A if any ordinary call-site shape stays silent even when another is compile-caught; second faces annotated as residue and counted once; raises-only-on-malformed-response is class B. Two things fell out that were not counting problems. Ruby's #563 was missing from the guide entirely — no mention of download_url anywhere in the chapter — verified against source rather than prose: v0.12.0 http.get_no_retry, which sent Accept: application/json and did not retry, became get_download calling request_with_retry with retry_on: DOWNLOAD_RETRY_ON and accept: nil. Ruby now has its own section. The same check confirmed Go's omission of #563 is correct, because Go already retried at v0.12.0. Separately, the Go note claiming the compiler catches only the pkg/generated half of Schedules().UpdateEntry was false: UpdateScheduleEntryRequest's fields became pointers, so any pkg/basecamp call site that set a field fails to build. The class-B definition described only half its own membership. It said the trigger is an absent field, but Ruby's entry fires only when the field is populated. It now says both, and says plainly that class B is a property of a call plus a response rather than of the call — the same method against the other shape is not a break at all. Class A has no such dependency. Stale counts in the chapter intros are fixed. The Go intro still said eleven silent and two panics, which is the first thing a #go link shows, and Swift claimed the most no-signal breaks, which stopped being true at Go ten. Also folds in #652 (projected-example gate, stacked on #648, takes check-targets to 43), moves #648 out of draft at cb438ce, and records that #647 is being reworked Smithy-first because the generated UpdateCardStepRequestContent.DueOn is *types.Date and cannot express "". The consumer-facing card shape is unaffected by that rework. Re-derived against #648: 238 -> 247 with 14 added, 5 removed and 11 same-ID route moves survives unchanged.
The opening warning said the runtime failures need a payload where a field is absent. That holds for the three Go entries; Ruby's single class-B entry has the opposite trigger. Draft#scheduled_posting_at and MyNote#created_at/#updated_at run through parse_datetime, which returns nil for nil and a Time otherwise, so .start_with? and Time.parse raise only when the field is populated. A reader following the old text builds the wrong fixture and concludes they are unaffected. Both directions are now named, here and in the root README. Class A was described as breaking on every response. Most of it does, but two groups do not: the error-message and validation entries need an error status to reach the code at all, and the field-map half needs a body of a particular shape; downloadURL's hop-1 retry changes nothing until a network error or one of 429/502/503/504 occurs. Stated as preconditions rather than as a blanket claim. The Go pointer example said only the field selector panics. types.Date.String has a value receiver, so Go rewrites t.DueOn.String() to (*t.DueOn).String() and the nil dereference panics before String is entered. The same holds for IsZero, Before, After and Weekday on Date and for Format, Sub, Unix and Year on time.Time. The summary bullet already said both panic; the example contradicted it. The Accept-header note credited only Python. Ruby dropped it on the same hop: get_download passes accept: nil, and request_headers sets the header only when accept is truthy. Both are named, with the observation that the other four never sent it on that hop at v0.12.0 either. No counts are touched.
Rebased onto 2afc977 and re-measured rather than incremented. Eight PRs merged since the branch was last updated, not the seven that carried the breaking label: #647 was on the "Not in this release" list and had landed. Counts. 55 class A and 6 class B, 61 surviving a clean build, up from 47/4/51. Per SDK the class split is Go 12/4, Swift 10/0, TypeScript 9/0, Python 8/0, Ruby 10/1, Kotlin 6/1, and the breaking-change column moves to 33/22/18/16/20/17. The body parses back to those numbers rather than agreeing with them by hand. The root README's aggregate sentence is re-derived to match, and now states both halves numerically instead of "most" and "a few". The operation inventory is unchanged at 238 -> 247 with the same 14 added, 5 removed and 11 same-ID route moves, computed from openapi.json at both ends. check-targets is 43, and the derivation is inline where the gate count was previously only projected. The release spans 67 merged PRs, 15 labelled breaking; the gh commands that produce both are embedded in the as-of block, with the note that a labelled PR is not the same unit as an entry, which is why the per-SDK columns exceed 15. #658 is class B, not class A. It does to five wrapper timestamps exactly what #615 did to five others: QuestionReminder.RemindAt, ClientApprovalResponse's CreatedAt and UpdatedAt, TimelineEvent.CreatedAt and WebhookDelivery.CreatedAt compile untouched through a value-receiver call and panic on nil. #615's own check could not see them because it keyed on the omitempty tag and these five did not carry one. The audit is ten fields, and the entry names the near-miss siblings that did not move, ClientApproval's pair in particular. #664 splits. The public CreateScheduleEntryRequest fields were already string and still are, so the wrapper half is silent: the RFC3339 ErrUsage guard is gone, a bare date now creates an all-day entry, and a malformed value reaches bc3 instead of failing locally. That is class A. The generated CreateScheduleEntryRequestContent went time.Time to string, which is a compile error for pkg/generated importers. ReplaceScheduleEntryRequestContent is not a migration from v0.12.0 at all; #632 introduced it. TypeScript and Ruby are doc-comment only. #647 is folded in as merged, with two corrections to what was written when it was still a branch. It touches no schema, so the claim that it had to go Smithy-first is withdrawn; UpdateCardStepRequestContent.DueOn was pointerized by #560. And the v0.12.0 preservation GET was conditional, taken only when the caller left due_on unaddressed, so the request-count table is scoped to that path rather than presented as universal. #648 adds no silent break anywhere. bc3's body is byte-identical before and after, so nothing that was populated stops being so; the assignable's title was never sent and is now spelled content. Every rename and retype is caught statically in Go, Swift, TypeScript and Kotlin and raised immediately in Python and Ruby, so it is one compile-or-runtime entry per SDK. Two corrections nobody asked for. The Go class list opened "Go carries every class-B break in the release", which stopped being true when Ruby's decode entry moved into class B; it now claims only the panic-shaped ones. And todos_write.json carries three errorRaised cases, not two, because #660 added a bare-scalar kill. #660 is a Kotlin class-B entry, which is new. Removing the client-wide isLenient means a present, populated, wrong-typed scalar throws SerializationException where it used to coerce to a string, and no signature moved to announce it. It throws in the response decode, so on a write the mutation has already landed, and it is not a BasecampException outside todolists. #656 is Ruby class A, scoped tightly: only max_retries 0, only an ungoverned GET, which means get_absolute and the Launchpad fetch rather than any operation lacking a policy. Every other configuration is bit-identical. Not in this release is now empty, and says so.
438ee2a to
317b5fc
Compare
Rebased onto
|
| before | after | |
|---|---|---|
| class A (no signal at all) | 47 | 55 |
| class B (fails at runtime) | 4 | 6 |
| survive a clean build | 51 | 61 |
| Go — breaking / A / B | 27 / 10 / 3 | 33 / 12 / 4 |
| Swift | 20 / 9 / 0 | 22 / 10 / 0 |
| TypeScript | 16 / 8 / 0 | 18 / 9 / 0 |
| Python | 14 / 7 / 0 | 16 / 8 / 0 |
| Ruby | 17 / 8 / 1 | 20 / 10 / 1 |
| Kotlin | 14 / 5 / 0 | 17 / 6 / 1 |
| operation inventory | 238 → 247 | 238 → 247 (unchanged) |
check-targets |
41 (projected 42, then 43) | 43 (measured) |
| as-of commit | 9de44b2a8 |
2afc97707 |
The class columns are not asserted — they are parsed back out of the document body and matched against the two summary tables and the root README. That check is arithmetic consistency, not inventory completeness; completeness came from reading each merged commit's diff against v0.12.0.
Commands
v12=$(git log -1 --format=%cI v0.12.0)
# merged PRs since v0.12.0 -> 67
gh pr list --repo basecamp/basecamp-sdk --state merged --limit 300 \
--json number,mergedAt --jq "[.[]|select(.mergedAt > \"$v12\")]|length"
# breaking since v0.12.0 -> 15
gh pr list --repo basecamp/basecamp-sdk --state merged --label breaking --limit 300 \
--json number,mergedAt --jq "[.[]|select(.mergedAt > \"$v12\")]|length"
# operations, both ends -> 238 and 247
jq '.operations|length' behavior-model.json
git show v0.12.0:behavior-model.json | jq '.operations|length'
# make check gate count -> 43
sed -n 's/^check-targets: *//p' Makefile | tr ' ' '\n' | grep -c .
# added / removed / same-ID route moves -> 14 / 5 / 11The two gh commands are now embedded in the guide's as-of block, with the note that a labelled PR is not the same unit as an entry — one PR can break four SDKs — which is why the per-SDK columns exceed 15.
How the four new breaking PRs classified
- Go: five wrapper timestamps could not represent absence (#620) #658 — class B, not class A. It does to five wrapper timestamps exactly what Go: pointerize the five optional timestamps that could not represent absence #615 did to five others:
QuestionReminder.RemindAt,ClientApprovalResponse.CreatedAt/.UpdatedAt,TimelineEvent.CreatedAt,WebhookDelivery.CreatedAtcompile untouched through a value-receiver call and panic on nil. Go: pointerize the five optional timestamps that could not represent absence #615's own guard could not see them because it keyed on theomitemptytag and these five did not carry one. The audit is ten fields, not five, and the entry names the near-miss siblings that did not move —ClientApproval's pair in particular, since the pair that moved is onClientApprovalResponse. - Schedule entries: a bare date must be creatable, not only replaceable #664 — splits in two. The public
CreateScheduleEntryRequestfields were alreadystringand still are, so the wrapper half is class A: the RFC3339ErrUsageguard is gone, a bare date now creates an all-day entry, and a malformed value reaches bc3 instead of failing locally. The generatedCreateScheduleEntryRequestContentwenttime.Time→string, a compile error forpkg/generatedimporters.ReplaceScheduleEntryRequestContentis not a migration from v0.12.0 — Stop the sparse schedule-entry PUT from erasing the join link, highlight and participants (#546, #547) #632 introduced it. TypeScript and Ruby are doc-comment only. - Ruby: max_retries: 0 made zero requests on an ungoverned GET (#532) #656 — class A (Ruby), scoped tightly: only
max_retries0, only an ungoverned GET. All 247 operations declare a retry block, so "ungoverned" means a call site passing nooperation:—get_absoluteand the Launchpad fetch. Every other configuration is bit-identical. - GetUpcomingSchedule renders reduced partials, so give it reduced shapes (#635, #641, #644) #648 — no class A or class B anywhere. One compile-or-runtime entry per SDK. bc3's response body is byte-identical before and after, so nothing that was populated silently stops being so; the assignable's
titlewas never sent and is now spelledcontent. Every rename and retype is compile-caught in Go/Swift/TS/Kotlin and raised immediately in Python/Ruby.
Plus #647 (class A ×6, one Go compile-error entry) and #660 — a new Kotlin class B: removing the client-wide isLenient means a present, populated, wrong-typed scalar throws SerializationException where it used to coerce to a string, with no signature change to announce it. It throws in the response decode, so on a write the mutation has already landed, and it is not a BasecampException outside todolists.
Claims that were false and are now fixed
- "Go carries every class-B break in the release." Stopped being true when Ruby's decode entry moved into class B. Now claims only the panic-shaped ones.
- Cards: explicit due-date clears silently no-op against production — send "due_on": "" #647 "is being reworked Smithy-first." The merged commit touches no schema at all.
UpdateCardStepRequestContent.DueOnwas pointerized by Go: optional fields are pointers — absence-capability by type, no waivers #560, not Cards: explicit due-date clears silently no-op against production — send "due_on": "" #647, and the card-step wrapper never uses that struct — it hand-builds amap[string]any. - The cards request-count table was presented as universal. All six SDKs took the preservation GET only when the caller left
due_onunaddressed. A call namingdueOnwas already a single PUT. Table is now scoped to that path. - "
todos_write.jsonkeeps 2errorRaisedcases." It carries 3 — Kotlin: stop coercing a wrong-typed scalar into a String (#598) #660 added a bare-scalar kill. - Not in this release listed three open items, all of which had merged. The section now states that nothing is in flight, and records the gate-count derivation that was previously only a projection.
Verified
make lint-actions check-readme-env-vars doc-constants-check— real exit code written into the log under a marker and grepped back:ZQMARKER_REAL_EXIT=0.MIGRATING.mdlinked from the root README and all six per-SDK READMEs — 1/1/1/1/2/1/1, all non-zero.- Every internal anchor resolves; no duplicate headings introduced.
- No reference anywhere to a file under
docs/— that directory is gitignored and internal-only, and this repo is public.
One thing I could not derive
The breaking changes column (33/22/18/16/20/17) has no derivation stated anywhere in the guide, and I could not reproduce the original 27/20/16/14/17/14 from the document's structure under any consistent rule — Go's 27 reproduces exactly as class A + class B + compile-error entries + pkg/generated bullets, but the same rule gives Swift 17 rather than 20. So I did not invent one. I applied the guide's own pre-committed increment protocol instead ("each entry names the specific derivations it invalidates, so the final pass is arithmetic rather than rewriting") and moved each SDK by the number of entries actually added: Go +6, Swift +2, TypeScript +2, Python +2, Ruby +3, Kotlin +3. Every other number in the table is derived and machine-checked. If that column should be reproducible too, it needs a stated rule, and that is a separate change.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 317b5fc4b7
ℹ️ 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".
The Swift Behavioural bullet said an explicit "" clears any of the five full-state fields. Only description does. "" on summary is accepted and reads back "Untitled"; starts_at and ends_at are under validates_presence_of in Schedule::Entry, so "" is rejected rather than cleared; allDay is a boolean in every SDK, so "" does not typecheck at all. The carve-out half grouped notify with the three clearable fields even though it is a send directive with no state to clear.
The six SDK README banners still carried the counts from before the Go reclassification and the recount that followed it, summing to 51 where MIGRATING.md and the root README say 61. Each banner now matches its row in the class A/B table: Go 12+4, Swift 10, TypeScript 9, Python 8, Ruby 10+1, Kotlin 6+1. Kotlin also gains the runtime clause it was missing, since its one class B entry throws on a present field carrying a JSON number or boolean where the model declares a string.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a782f805e
ℹ️ 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".
The release spans 55 merged pull requests, not 67. The 67 came from comparing GitHub's Z-formatted mergedAt against a git timestamp formatted with a local offset, using jq's string >, which is lexicographic rather than temporal; it wrongly swept in twelve PRs merged in the hours before the v0.12.0 tag instant. The derivation embedded in the guide taught that same broken comparison, so it now uses %ct and fromdateiso8601 and says why. The breaking count of fifteen is unchanged, since all fifteen merged after the tag, so the class A/B split, the per-SDK tables and the six README banners are untouched. The header no longer calls 2afc977 the commit the release is cut from. That commit is the last of the release content and the baseline the counts were measured against, but it predates this guide; the tag is cut from main after this merges, on a tree that contains the file the release body links to. The release-body teaser claimed the guide covers only breaks with no exception and no decoder failure. The guide documents six breaks that do fail at runtime, including Ruby and Kotlin raises and a Kotlin decoder failure, so the teaser now names both the silent class and the runtime one.
|
Correction to a number I stated publicly: this release spans 55 merged PRs, not 67. Fixed in I derived the 67 with a comparison that is not a comparison: v12=$(git log -1 --format=%cI v0.12.0) # 2026-07-31T17:57:46-07:00
gh pr list ... --jq "[.[]|select(.mergedAt > \"$v12\")]|length"
The temporal form, now the one embedded in the guide: epoch=$(git log -1 --format=%ct v0.12.0) # 1785545866
gh pr list --repo basecamp/basecamp-sdk --state merged --limit 300 \
--json number,mergedAt \
--jq "[.[]|select((.mergedAt|fromdateiso8601) > $epoch)]|length" # 55Nothing else moves. The The sentence carrying the bad number was, of course, the one warning that a hand-carried count is how these go wrong. The fix corrects the count and replaces the derivation it taught, with a comment explaining the trap so the document stops propagating it. |
|
Merging last, as the release procedure requires — the tag is cut after this so the tagged tree contains the guide and the generated release-body link resolves. Merging on independent review rather than a Codex pass at head: Codex has not fired at head on any PR this session, and Copilot's check has been failing repo-wide on its own infrastructure throughout. Final audit at exact head Numbers in this guide are now derived, not carried. The one I supplied was wrong and is worth recording: I reported 67 merged PRs since v0.12.0; it is 55. The derivation compared The guide had embedded that broken command, so it would have propagated the bug to whoever maintained the counts next release — that mattered more than the number, and the replacement carries a comment naming the trap. The breaking count of 15 is unaffected, since all 15 breaking PRs merged after the tag instant. So the class A/B split (55 / 6), the per-SDK tables and the six README banners all stand — they derive from the breaking roster, which was correct throughout and is verified byte-identical. Aggregates are machine-checked rather than eyeballed: table columns sum to 55 no-signal / 6 runtime / 61 total, MIGRATING prose and the root README both say 61 = 55 + 6, and the six per-SDK banners sum to 16+9+8+11+7+10 = 61 with the same split. All seven READMEs link the guide. The release-body teaser deliberately carries no counts. That block is emitted for every release, so hard-coded numbers would be stale the moment v0.14.0 ships — the same hand-carried-number failure as above. It names the two classes; the guide carries the figures. |
#673 and #675 landed after the last measurement, so the guide's baseline moves from 931c36a to 70d576b. The counting method changes too, because the one committed here was wrong. `git log v0.12.0..HEAD | wc -l` counts COMMITS, which equals PRs only while every commit is a squash merge. The release-prep commit is pushed straight to main and is not a PR, so from the moment the version is bumped that count runs one high — it reported 59 where 58 PRs had merged. That is the third way this number has been wrong, each differently: a lexicographic string compare on timestamps, then a temporal compare that credited #556 to this release (its squash commit IS the commit v0.12.0 tags), and now a commit count meeting its first non-PR commit. The rule that survives all three is the definition itself: reachable from HEAD and not reachable from v0.12.0, applied to the merge commits of PRs rather than to commits. Both derivations in the guide now use it, and both were run to confirm they reproduce the numbers printed beside them. Committed directly rather than as a pull request, on purpose: a PR carrying this number would increment the number it states. #642 shipped wrong for exactly that reason — a document inside the release cannot count itself.
Adds
MIGRATING.mdat the repo root — the v0.13.0 upgrade guide — and links it from the root README, all six per-SDK READMEs, and every future GitHub Release body.51 of v0.13.0's breaks survive a clean build. 47 give no signal at all — the call keeps working against a live server and does something different. 4 compile and then panic or raise, but only for a particular shape of response, so they pass your tests and fail in production. Label-generated release notes cannot express that, and until now it had no home in this repo.
Why one new file, and why the workflow change
The repo deliberately has no CHANGELOG.
go/CHANGELOG.mdandruby/CHANGELOG.mdwere deleted in #115 ("Auto-generated release notes supersede these"),docs/is gitignored as internal-only, and every release body since is machine-built:release-github.ymlwrites an Installation block andgenerate_release_notes: trueappends the label-derived list. v0.10.0 through v0.12.0 carry not one hand-written line.So there is no existing convention for narrative upgrade notes, and adding a per-version notes file resurrects exactly what #115 removed. Instead:
MIGRATING.md— what shipped, what you must do, what wrong behaviour you get if you don't.release-github.ymlnow emits an## Upgradingblock linkingMIGRATING.mdat the tag, guarded on the file existing. I originally flagged this as a by-hand step; that was the wrong call — a hand-step with no gate is a step that gets skipped.CONTRIBUTING.mdrecords the rule.Structure
Breaks your compiler will not catch leads the document, split into two classes that fail differently enough that merging them would mislead. Then one section per SDK, ordered by severity, plus an operator checklist, a "coverage: corrected and re-scoped" section for what did not ship, and known gaps.
Counts are stated as-of
9de44b2a8with derivations inline, and each in-flight change names exactly which numbers it invalidates, so the pre-tag pass is arithmetic rather than rewriting.Review threads — all three fixed and resolved
P1, pagination.
git show v0.12.0:go/pkg/basecamp/bookmarks.goreturns beforefollowPaginationwhenpage > 0, so Go was never a starting-offset SDK — "droppageto get the old walk" would have turned a bounded request into a full account-wide traversal. Item 1 is now scoped to the five SDKs where it holds (re-verified at the tag), with a#### Go is the exceptionsubsection separating the services where the page number was already honoured from the fourteen carrying the v0.12.0"not yet honored"doc.P1, raw wire. The
fmt.Sprintf+ac.Postexample is gone, replaced by a There is no raw-wire migration path known-gap section citing AGENTS.md §4/§5. Swept the document; the one otherac.Postdocuments a real error-code change on an existing surface, so it stays, but its fabricated path is gone.P2, misclassified panics. This was a contradiction with my own definition, not a wording problem — I wrote that a silent break "does not raise" and then listed panics under it. Applying the definition consistently moved four entries, not the three flagged: the three Go pointerization panics plus Ruby's
Draft#scheduled_posting_atdecode, which raisesNoMethodError/TypeErrorand had the same defect. Two moved entries carry genuine no-signal residue, kept as sub-notes rather than double-counted. Body counts verified against the table by parsing the section, not by hand.That check proves less than it sounds like, and the distinction matters. Parsing the section proves arithmetic consistency — the numbers agree with each other. It cannot prove inventory completeness, that every real break is present. Completeness came from a separate per-SDK audit against the definition, and that is what found Ruby's
#563missing from the guide entirely. No amount of parsing would have surfaced a break that was never written down.Suppressed comment (Copilot),
MIGRATING.md:1057— correct and fixed. The Swift section said "Three new optional members are additive" but named one; the other two are required. Now singular, matching the TypeScript section.Corrections carried into the guide
TrashTodowas not a 404. bc3 drawsresources :todos, only: %i[show edit update destroy];DELETE /todos/:idreturned 204 anddestroy_status_paramdefaults to"archived"— every caller was archiving. The one Route corrections: nine flat routes bc3 only draws bucket-scoped, three operations removed #619 removal that takes away a working call. The nine re-pathed routes are a disjoint set whose 404 reassurance holds; the guide keeps them apart.UpdateTodolistOrGroupstill passes the write and denies the new read.colorandcomments_app_urldid not exist onTodolistat v0.12.0 in any of the six — both arrived with One flat Todolist replaces Todolist, TodolistGroup and TodolistOrGroup #628 earlier in this same release — so from the guide's baseline nothing turned optional→required. I incremented all six counts for it and then reverted.New in this push
a373b004c). Added/removed/moved lists are computed fromopenapi.jsonat both ends, not hand-edited: 14 IDs added, 5 removed, 11 same-ID route moves. Also flags that the Folders operations are drawn at/stacks, not/folders."due_on": "",UpdateStepRequest.DueOn→*string, hook collapse) is read frombf4371534onfix/card-due-on-explicit-clearand marked as not-yet-merged.{Todolists,Update}split, because it fails the opposite way: allowlists do not start denying, but a denylist on{Cards,Get}/GetCardsilently stops blocking the write it used to take down with it.errorRaisedkill cases go with the preservation GET (cards_write.json8→5 cases, 3→0errorRaised). Stated plainly, with the fact that the class stays pinned on Todos (2errorRaised), so Cards is no longer the canary.One infrastructure note
make checkfailed once on this branch withopenapi.json is out of datewhile my spec files were byte-identical to main. Pristineorigin/mainpassed the same gate, so it was stale gitignoredspec/build/state surviving a merge that changed the spec.rm -rf spec/buildcleared it, no tracked file changed, and the suite is green. Worth knowing before someone "fixes" it by runningmake smithy-buildand committing unrelated churn.Verification
238 → 247, the 300-field Go census, and the 41-gate count all reproduce from commands in the guide or fromopenapi.json/Makefiledirectly.make check— 41 gates, exit code grepped from a captured log. (GetUpcomingSchedule renders reduced partials, so give it reduced shapes (#635, #641, #644) #648 takes it to 42.)