Cards: explicit due-date clears silently no-op against production — send "due_on": "" - #647
Merged
Merged
Conversation
basecamp/bc3#12521 merged and deployed to production, making BC3's JSON card update presence-aware: an omitted key is now left UNCHANGED, and only an explicit "" or null clears. All six SDKs encoded an explicit due-date clear by OMITTING due_on, so against production every explicit clear silently no-ops. Encode the clear as "due_on": "" instead. BC3 blank-casts that to nil and pins it by a server test, and it is the only clear spelling all six SDKs can express identically: five strip nulls structurally before the wire, none strip "". With omission now meaning "unchanged", the read-modify-write preservation GET has nothing left to protect, so it goes too. update is a single PUT in every case: one request instead of two, and no read-modify-write race. That removal deletes a defect class rather than merely an optimisation. The three #576 kill cases in conformance/tests/cards_write.json pinned that a malformed due_on read back FROM the preservation GET is refused rather than written forward. With no read-back they are unreachable, so they are removed along with the per-SDK guards they exercised. The class stays pinned on todos_write.json, whose composite still does a genuine read-modify-write. Card steps get the same contract. UpdateStepRequest.DueOn becomes *string in Go so a clear is expressible at all: nil leaves the date alone, Ptr("") clears. basecamp/basecamp-cli#604 argued that pointerizing it "buys nothing" — correct before #12521, when the controller cleared on omission regardless, and obsolete after it. update and updateVerbatim are now behaviourally identical in all six SDKs. Collapsing them is breaking and reaches into six generators' name overrides, so both are kept here with the relationship documented, and the collapse is filed separately. Repins spec/api-provenance.json to bc3 4e34dc83eb, the commit these claims are true at.
…ly the pin moves)
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: 5292d27ea2
ℹ️ 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
This PR fixes a live production correctness break: after basecamp/bc3#12521 made BC3's JSON card update presence-aware, an omitted due_on now means "leave unchanged" rather than "clear". Every SDK encoded an explicit clear by omitting due_on, so clears silently became no-ops against production. The fix spells a clear as "due_on": "" (which BC3 blank-casts to nil, and which none of the six SDKs strip) and removes the Cards preservation GET, collapsing update to a single PUT. It also pointerizes Go's UpdateStepRequest.DueOn (string→*string) so a step clear is expressible, and repins provenance to bc3 4e34dc83eb.
Changes:
- Encode card/step due-date clears as
"due_on": ""; drop the Cards read-modify-write soupdateis one PUT (Cards is no longer a merge-safe composite). - Go
UpdateStepRequest.DueOnbecomes*string(nilleaves unchanged,Ptr("")clears), usingPtr/Dereffrom #643. - Update conformance (
cards_write.json8→5 cases, pinningdue_on == ""and removing the three now-unreachable#576GET kill cases), repin provenance/routes/doc-constant spans, and rewrite SPEC §5.
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
go/pkg/basecamp/cards.go |
Sends due_on unconditionally when non-nil; UpdateStepRequest.DueOn → *string; drops preservation GET. |
go/pkg/basecamp/cards_test.go |
Presence-aware stateful mock; clear/unchanged behavioral tests; step pointer marshal tests. |
typescript/src/services/cards-extensions.ts |
Single PUT; dueOn ?? ""; removes merge-safe import. |
typescript/tests/services/cards.test.ts |
Presence-aware tests; removes #576 block. |
ruby/lib/basecamp/services/cards_extensions.rb |
resolved_due_on (nil→omit, ""→clear); removes preservation GET + escape hatch. |
ruby/test/.../cards_service_test.rb |
Presence-aware tests; drops #576 cases. |
python/src/basecamp/services/cards.py / tests/.../test_cards.py |
Sparse single-PUT update; tri-state due_on; presence-aware mock. |
kotlin/.../services/CardsService.kt (+test) |
Passes dueOn verbatim; single PUT; behavioral tests. |
swift/.../CardsServiceExtensions.swift (+test) |
Switch resolves preserve/clear/on; single PUT. |
conformance/tests/cards_write.json |
8→5 cases; pins due_on == ""; removes 3 GET kill cases. |
spec/api-provenance.json, go/.../api-provenance.json, spec/bc3-routes.json |
Repin to 4e34dc83eb (routes byte-identical). |
spec/api-gaps/README.md, spec/doc-constants.json, COORDINATION.md, SPEC.md |
Range triage, marked-span/pin-citation updates, §5 rewrite. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Member
Author
|
@codex review Head moved to
Flagging explicitly so the re-review is scoped: your P1 on |
jeremy
added a commit
that referenced
this pull request
Aug 4, 2026
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.
|
Codex Review: Didn't find any major issues. Keep them coming! 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 4, 2026
…e-projection * origin/main: Cards: explicit due-date clears silently no-op against production — send "due_on": "" (#647)
jeremy
added a commit
that referenced
this pull request
Aug 4, 2026
…n' into gate/projected-example-validation * origin/feat/upcoming-schedule-projection: Go: thread #641's join link, highlight and status through the public create request Go: an empty upcoming-schedule window bound is a local usage error, not a server 400 Cards: explicit due-date clears silently no-op against production — send "due_on": "" (#647) Ruby generator: emit a bare `#` for a paragraph break in a @PARAM description
jeremy
added a commit
that referenced
this pull request
Aug 4, 2026
DecoderStrictnessTest built its own three Json instances and never touched BasecampClient, so it pinned kotlinx.serialization's flag semantics and nothing about this SDK's configuration. Restoring main's BasecampClient.kt — isLenient and all — and running the whole file was green. The comment claiming it "cannot drift from the semantics" was writing a cheque the test did not cover. `json` is internal and commonTest is the associated compilation, so add a case that decodes through the instance BasecampClient builds. Against main's client it fails and it is the only failure in the file; against this branch's the file is green. Narrow the negative assertions from Exception to SerializationException while here — every one of them throws it. Cards had no wrong-typed-due_on coverage. #598 filed it as a read-modify- write hazard, but #647 had already deleted the preservation GET and the three Cards kill cases along with it, so there is no read-back left to refuse before a PUT. What survives is the response decode: Card.dueOn is String?, and under isLenient a bare-scalar due_on off the wire decoded to "42"/"false" and reached the caller as an ordinary String. Cover that, which is the shape the shared decoder still governs.
jeremy
added a commit
that referenced
this pull request
Aug 4, 2026
* Kotlin: stop coercing a wrong-typed scalar into a String (#598) The client-wide Json carried `isLenient = true`, which relaxes RFC-4627 far enough to read a JSON number or boolean into a declared String. `"description": 42` decoded to "42", and the merge-safe composites then PUT that fabricated value back to a full-replace endpoint on a call that never mentioned the field. The coercion happened inside the decoder, so the per-composite guard that fixed Python, Ruby and TypeScript in #597 was structurally incapable of catching it: by the time the composite ran, the value was an ordinary String. `isLenient` is the culprit and `coerceInputValues` is not. The two are not interchangeable and the release plan blamed the wrong one, so removing it would have shipped a no-op as a fix. `coerceInputValues` rewrites an explicit null to the declared default for a non-nullable property and has nothing to say about a scalar type; it stays. DecoderStrictnessTest pins both behaviours in nine tests so the attribution lives in the tree rather than in anyone's recollection. The evidence that the fix landed is a tripwire flipping. TodolistsServiceTest carried `updateCoercesABareScalarDescription`, whose own doc comment said it recorded the coerced PUT "so closing #576 flips it visibly". It is now `updateRefusesABareScalarDescriptionBeforeWriting` and asserts the refusal — no PUT, a statusless non-retryable BasecampException.Api with a hint — for both a number and a boolean. Todolists is a third affected service beyond the Todos and Cards that #576 and #598 named; Documents and Schedules share the shape. conformance/tests/todos_write.json gains a bare-scalar kill case. It needs no per-language skip: with the decoder fixed, all six SDKs refuse a non-string scalar — Go, Kotlin and Swift in their model decoders, TypeScript, Python and Ruby in the hand-written writableString guards — so the case that had no home before is now a six-runner regression guard. Restoring `isLenient` locally turns it red (`Expected the call to fail, but it succeeded`, make conformance-kotlin exit 2), which is what makes it a kill case rather than decoration. The rest is prose the fix falsified: comments in DocumentsService, SchedulesService and TodolistsService and their tests described the scalar hole as an open cross-service gap. * Pin the decoder the SDK actually configures, and cover Cards DecoderStrictnessTest built its own three Json instances and never touched BasecampClient, so it pinned kotlinx.serialization's flag semantics and nothing about this SDK's configuration. Restoring main's BasecampClient.kt — isLenient and all — and running the whole file was green. The comment claiming it "cannot drift from the semantics" was writing a cheque the test did not cover. `json` is internal and commonTest is the associated compilation, so add a case that decodes through the instance BasecampClient builds. Against main's client it fails and it is the only failure in the file; against this branch's the file is green. Narrow the negative assertions from Exception to SerializationException while here — every one of them throws it. Cards had no wrong-typed-due_on coverage. #598 filed it as a read-modify- write hazard, but #647 had already deleted the preservation GET and the three Cards kill cases along with it, so there is no read-back left to refuse before a PUT. What survives is the response decode: Card.dueOn is String?, and under isLenient a bare-scalar due_on off the wire decoded to "42"/"false" and reached the caller as an ordinary String. Cover that, which is the shape the shared decoder still governs.
jeremy
added a commit
that referenced
this pull request
Aug 4, 2026
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.
jeremy
added a commit
that referenced
this pull request
Aug 4, 2026
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.
jeremy
added a commit
that referenced
this pull request
Aug 4, 2026
* MIGRATING.md: the v0.13.0 upgrade guide, silent breaks first 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. * Fix the Go pagination advice, cut the raw-wire workaround, absorb #637/#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. * Split silent breaks into no-signal and fails-at-runtime; absorb #629 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. * Audit class A across all six SDKs; add Ruby's missing download retry 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. * Correct four claims in the v0.13.0 guide that do not match the source 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. * Re-derive every count against the final release commit 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. * State the schedule-entry clear value per field instead of universally 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. * Re-derive the per-SDK README banners against the final class A/B table 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. * Correct the merged-PR count and the two claims the reviewers caught 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.
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.
Live production correctness break. Every explicit card due-date clear is currently a silent no-op, in all six SDKs, released and unreleased.
What happened
basecamp/bc3#12521 merged and deployed to production at
4e34dc83eb(2026-08-04T06:15Z). It made BC3's JSON card update presence-aware:card_update_paramsnow returns barecard_paramsunderrequest.format.json?instead of merging over{ due_on: nil }. An omitteddue_onused to clear the date; it now means "leave unchanged."Every SDK encoded an explicit clear by omitting
due_onon the wire — presence-bearing in memory, absent on the wire. Against production that PUT now succeeds and does nothing.go/pkg/basecamp/cards.gosaid so in as many words:bc3#12521's own rollout gate specified an SDK compatibility release before the deploy. That did not happen — the deploy went first. This PR is the reactive fix, and it carries the cleanup release with it on the repo owner's authorization, since the sequencing it was waiting on is moot.
The fix
Clear is now spelled
"due_on": "". BC3 blank-casts that to nil and pins it with a server test. It is the only clear spelling all six SDKs can express identically: five strip nulls structurally before the wire (Python_compact, Rubycompact_params, Kotlin?.let, TypeScript'sJSON.stringifydroppingundefined, SwiftencodeIfPresent) and none of them strip"". A literalnullwould also violate the body-compaction rule in SPEC §18.The preservation GET is gone. With omission meaning "unchanged", the read-modify-write had nothing left to protect.
updateis a single PUT in every case — one request instead of two, and no read-modify-write race. Cards stops being a merge-safe composite.The
compatibility.bc3-fourpin does not block this:COORDINATION.mdrecords that BC5 replaced BC4 in production and there is no live BC4 backend, andspec/api-gaps/dock-tool-create-contract.mdcallsfour"the wire-format reference … not a server anyone can call." Four lanes re-derived that in-repo independently rather than taking it on assertion.Read this bit — removing the GET deletes a defect class, not just a request
The three
#576errorRaisedkill cases inconformance/tests/cards_write.jsonpinned that a malformeddue_onread back from the preservation GET is refused rather than written forward. They assert aGETat index 0. With no read-back they are unreachable, so they are removed along with the per-SDK guards they exercised (Ruby dropped 15 corresponding tests, Python and TypeScript their#576blocks).A reviewer skimming "removed an unnecessary GET" will not price that. Stated plainly: the Cards composite no longer does read-modify-write, so the malformed-response guard class no longer applies there. The class is not lost —
conformance/tests/todos_write.jsonstill carries two equivalent kill cases over the Todos composite, which still does a genuine read-modify-write (current, err := s.Get(ctx, todoID)ingo/pkg/basecamp/todos.go), against a full-replace endpoint where a coerced value is the more dangerous case anyway. Both assertrequestMethod: GETat index 0 withrequestCount: 1; that was verified, not assumed.This also narrows #598 (Kotlin's lenient decoder coercing a wrong-typed scalar) to the remaining read-modify-write composites — commented there, not closed, because the root cause is a client-wide decoder policy that this PR does not touch.
Steps
titleis optional on update, omission means unchanged,"assignee_ids": []removes everyone, and an assignee-only body is now a valid partial update where it used to 400. Those are all server-side improvements that need no SDK change — Go's sparse step updates simply stop being destructive.What did need a change is expressiveness:
UpdateStepRequest.DueOnwas a plainstringbehind anif req.DueOn != ""guard, so a clear was only ever expressible implicitly, by omission — which no longer clears. It is now*string, usingPtr/Derefas exported in #643.nilleaves the date alone,Ptr("")clears. Presence is tested asreq.DueOn != nil, never viaDeref, which would collapseniland&""into the same thing and lose the whole distinction.Worth recording, because it looks like a contradiction otherwise: basecamp/basecamp-cli#604 argued that pointerizing
UpdateStepRequest.DueOn"buys nothing … since a nil*stringis omitted from the JSON and the controller clears on that omission regardless." That was correct before #12521 and is obsolete after it. Its premise expired; we are not overturning a settled decision.Red proofs
Each lane proved its tests fail against the un-fixed code and pass with the fix, swapping the original file in via
git show origin/main:<path>(nevergit stash— the stack is shared across ~30 worktrees), capturing to a log and grepping the real exit code back out.explicit clear did not clear: server still holds due_on "2024-02-01",REAL_EXIT=1{}on the wire:PUT … with body '{}' … was made 1 time, andExpected "2024-02-01" to be nil,REAL_EXIT=1AssertionError: the clear must land; an omitted due_on silently no-ops here / assert '2024-02-01' is None,REAL_EXIT=1AssertionError: expected '2024-02-01' to be null,REAL_EXIT=1an explicit clear must land as a clear on the server ==> expected: <null> but was: <2024-02-01>, and the un-fixed wire bytes{"content":"","assignee_ids":[]}with nodue_onat all,REAL_EXIT=1XCTAssertNil failed: "2024-02-01" - the explicit clear must actually clear the stored due date, not no-op,REAL_EXIT=1These are behavioural, not assertion flips: each runs against a stateful mock modelling BC3's real presence rule (omitted → unchanged,
""/null → cleared), and each is paired with a test proving the mock isn't simply clearing on every PUT.Conformance
conformance/tests/cards_write.jsongoes 8 cases → 5, dispatched in all six runners:due_onoff the wire (wasrequestCount: 2, GET-then-PUT resending the fetched date)requestBody due_on == ""(wasrequestBodyAbsent) — this is the cross-SDK pin for the new encodingdue_on == ""alongside their own field#576kill cases are removed, per aboveProvenance
Repins
spec/api-provenance.jsonto bc34e34dc83eb— the claims here are only true at that pin — plus the Go mirror, the@bc3-pinmarked spans, and a hand-written range triage for4dd2926f8a..4e34dc83ebinspec/api-gaps/README.md.spec/bc3-routes.jsonwas rebuilt at the new pin and theroutesarray is byte-identical; only thesourcefield moves, which is the triage's "no route delta" claim verified rather than asserted.Not spec-touching
No change to
spec/basecamp.smithyoropenapi.json, and no regeneration.UpdateCardStepInputandUpdateCardInputalready declaretitle/due_on/assignee_idsoptional, and every generatedupdateVerbatimis a pass-through that strips only null/nil/undefined — so""already reached the wire. This did not need the spec train.Downstream
Per bc3#12521's sequencing, the CLI may now drop the basecamp-cli#496 title re-send workaround once this lands. Removing it before the deploy would have restored the 400 against production; that constraint is cleared by this PR, so someone needs to pull that thread.
GetCard+UpdateCardcollapsing to a bareUpdateCardis a real hook/request-count change observable to consumers — flagged for the MIGRATING.md lane (#642) and worth a release note.Follow-ups filed
updateandupdateVerbatimare now pure duplicates in all six SDKs. Collapsing is breaking and reaches into six generators' name-override tables, so both are kept here with the relationship documented.doc/api/sections/my_assignments.mdstill tells callers that card and step updates clear omitted fields and to resend current values. That now contradicts both canonical endpoint docs and #12521's own controller tests, and it is the page a confused consumer lands on.Summary by cubic
Fixes a production bug where clearing a card or step due date did nothing. SDKs now clear by sending "due_on": "" and drop the preservation GET, making update a single PUT; also cleans up generator comments to match the new presence-aware contract.
Bug Fixes
updatesends one PUT; deletes the malformed-response guard class tied to the old GET.UpdateStepRequest.DueOnbecomes presence-bearing.Migration
updatenow makes one request.UpdateStepRequest.DueOnis now*string— usebasecamp.Ptr("")to clear,nilto leave unchanged.Written for commit a3c7725. Summary will update on new commits.