Publish Todolist.color and comments_app_url as required (#630) - #637
Conversation
BC3 emits both keys on every projection of this shape, and the published contract said they might be absent. `_todolist.json.jbuilder` calls `json.color` in both branches of its `todolist_group?` conditional, and emits `json.comments_app_url` unconditionally from the `bucket_recording_comments_url` route helper. Optional pushed an absence-check onto every consumer in every language for keys that are never absent. `color` is required AND nullable — `recordings.color` is a nullable integer column, so an uncolored list or group sends an explicit `null`. Native `@required` cannot express that here: `Todolist` carries `@examples` (via `GetTodolistOrGroup`) and Smithy cannot put a `null` into an example for a `String` shape, so requiring the member would force the group example to advertise a color that uncolored groups do not have. The requiredness goes into the OpenAPI projection instead, appended to the `required` array by a `jsonAdd` pointer, leaving the Smithy member and the faithful null example untouched. The sibling shapes carrying the same field (Wormhole, Folder, FolderWithProjects) are natively `@required` only because none of them carries examples. `comments_app_url` is never null — a Rails URL helper returns a String or raises — so it takes native `@required`, which costs the two examples a real comments URL rather than a fiction. The color doc comment argued for optional on fixture-cost grounds; that reasoning is superseded and is replaced by the projection rationale.
Making both keys required turns every Todolist-shaped stub that omits them into a decode failure in the two runtime-strict SDKs: Kotlin threw MissingFieldException on 17 TodolistsServiceTest cases and Swift threw DecodingError.keyNotFound on 22, including tests asserting an unrelated failure mode that never got far enough to see it. Ruby, Python, Go and TypeScript are structurally permissive and stayed green. Each SDK's canonical body helper gains both keys rather than each call site patching them in, so a body that omits one has to be written deliberately. color is threaded as a parameter so the group variants can carry an explicit JSON null — Kotlin takes raw JSON, Swift takes Any so NSNull() spells the null a String? would have turned into an absent key. The seven synthetic mock bodies in conformance/tests/todolists_write.json carried neither key; todolists_read.json already carried both. No captured body was missing either key: all six Todolist-shaped objects under spec/fixtures/ already carry them, which is what #628's reading of _todolist.json.jbuilder predicted. SPEC.md §"the spec now says the same thing" described both fields as sitting alongside the optional discriminator; it now states their requiredness and how each half is expressed.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f9b9c92a55
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bbc5a2768c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex caught it at bbc5a27: appending `color` to `Todolist.required` in the projection made both GetTodolistOrGroup response examples invalid against the schema they illustrate. `smithy validate` could not see it — at the Smithy level the member is still optional, so the examples were fine there, and the requiredness only exists downstream in the OpenAPI document. A published example that violates its own published schema is worse than the looseness this PR set out to remove. The examples get their color the same way the requiredness did, through `jsonAdd`: "blue" for the list, an explicit `null` for the uncolored group, which is what BC3 actually sends. Writing them in examples.smithy is still not an option — Smithy cannot put a `null` into an example for a String shape, and giving the group a made-up color to dodge that trades a faithful example for nothing. Note the seam this exposes: `smithy validate` checks examples against the Smithy model, so any requiredness added to the projection by `jsonAdd` is unchecked against the projected examples. Two pointers do that today, both added here.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12b02cda8c
ℹ️ 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 first sweep fixed what broke. AGENTS.md asks for more than that: Existing tests updated — every test stubbing a changed path must be updated. Inline stubs may omit unrelated response fields, but every response stub for a changed path must include any newly required or behaviorally changed field. This guard covers only the manifest'd shared fixtures — one-off inline stubs remain the reviewer's responsibility under the rule above. Kotlin and Swift threw on the missing keys, so their stubs got fixed. Ruby, Python and TypeScript decode permissively, so theirs stayed green while holding bodies BC3 cannot produce — exactly the case the rule exists for, and exactly the case no gate catches. Ruby's seven minimal response hashes, Python's `_todolist()`, and TypeScript's inline `mockGroup` now carry both keys; `color` is `null` on the group stubs, which is the ordinary case for an uncolored group and is a different wire fact from the key being absent. Each file already carried `description_attachments` in its minimal stubs for the same reason, and Python's helper already documented the principle for `bubble_up_url` — this follows the convention rather than inventing one. Everything else was already complete: Go, and the rest of Ruby, Python and TypeScript, build their todolist bodies from the shared spec/fixtures, and Kotlin's TodolistGroupsServiceTest already carried `"color": null` and `comments_app_url`.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4c36d7b94c
ℹ️ 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 previous pass keyed on `type: "Todolist"` to find inline stubs, and this
one does not carry it — so `mockGroups` in the group-list happy path kept two
bodies BC3 cannot produce. Both elements now carry `color: null` and
comments_app_url.
Re-enumerated by route rather than by field this time, which is the search
that cannot miss a stub for lacking a marker. Every test touching
/todosets/{id}/todolists.json, /todolists/{id}, or /todolists/{id}/groups.json
across all six SDKs and the conformance suite is accounted for: the remaining
hits are /todolists/{id}/todos.json (Todo bodies), `parent` references
(TodoParent, which has no color), and Todoset.todolists_url strings.
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! 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". |
…lishes `spec/smithy-build.json`'s `jsonAdd` can append to a schema's `required` array in the OpenAPI projection — the route #630/#637 took for `Todolist.color`, because the shape carries `@examples` and Smithy cannot express a `null` in an example for a String. `smithy validate` checks `@examples` against the *Smithy* model, where the member is still natively optional; the requiredness arrives afterwards. Nothing validated the projected examples against the projected schema, so #637 shipped both `GetTodolistOrGroup` response examples without `color` while the schema declared it required, and a bot reviewer caught it rather than CI. scripts/check-projected-examples.rb walks every example openapi.json publishes — response, request-body and parameter — and validates each against the sibling schema through the extracted composition-aware walk. `examples` maps hold Example Objects (value under `value`, `$ref`s into components/examples resolved, `externalValue` reported as skipped rather than fetched) while the singular `example` IS the value; conflating the two would silently unwrap any payload with a field named `value`. Two things the walk has to know about the generator. The bare-response mappers rewrite a single-property `*ResponseContent` into the bare payload but leave the wrapper on the example, so the wrapper is unwrapped first; an EMPTY wrapper is Smithy's rendering of an example that declared no `output` at all, so it is skipped and reported by name. Those skips are unvalidated published examples, not clean ones — #644 tracks giving `ListRecordings`, `UpdateProjectAccess` and `UpdateSubscription` a real `output` — so every run prints a standing note citing it. The marker is self-clearing: when those examples gain an `output` their wrappers stop being empty, the validated count rises and the note stops printing, with nothing here to edit. The gate refuses to pass vacuously: validating zero response examples — the class the seam lives in — is reported as a failure, not a green run. The self-test drives 1 positive + 14 negative/skip cases through the real checker via `PROJECTED_EXAMPLES_OPENAPI`, case 1 being the #637 defect reproduced exactly. Case 9 doubles as the proof that #644 lands without touching this file: it makes one of those wrappers non-empty and asserts the gate then validates it. The header carries the measured mutation matrix — which cases go red when each guard is removed from a scratch copy, including that the positive control, not case 1, is what pins the envelope unwrap. Closes #638 Refs #644
…#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.
…lishes `spec/smithy-build.json`'s `jsonAdd` can append to a schema's `required` array in the OpenAPI projection — the route #630/#637 took for `Todolist.color`, because the shape carries `@examples` and Smithy cannot express a `null` in an example for a String. `smithy validate` checks `@examples` against the *Smithy* model, where the member is still natively optional; the requiredness arrives afterwards. Nothing validated the projected examples against the projected schema, so #637 shipped both `GetTodolistOrGroup` response examples without `color` while the schema declared it required, and a bot reviewer caught it rather than CI. scripts/check-projected-examples.rb walks every example openapi.json publishes — response, request-body and parameter — and validates each against the sibling schema through the extracted composition-aware walk. `examples` maps hold Example Objects (value under `value`, `$ref`s into components/examples resolved, `externalValue` reported as skipped rather than fetched) while the singular `example` IS the value; conflating the two would silently unwrap any payload with a field named `value`. It compares exactly what is published against exactly the schema it is published under, and unwraps nothing. That is only possible because #644 landed first: the bare-response mappers used to rewrite a single-property `*ResponseContent` into the bare payload while leaving the wrapper on the example, and `BareResponseExampleMapper` now mirrors the unwrapping onto examples so the two agree. A gate that also accepted the wrapped shape could not tell a correct example from a regression in that mapper — so this gate guards it too, and self-test case 7 puts the wrapper back and asserts the run goes red. The gate refuses to pass vacuously: validating zero response examples — the class the seam lives in — is reported as a failure, not a green run. The self-test drives 1 positive + 14 negative/skip cases through the real checker via `PROJECTED_EXAMPLES_OPENAPI`. Its header carries the measured mutation matrix: which cases go red when each guard is removed from a scratch copy, including a mutation that unwires the validator while leaving every walk intact, so coverage and judgement are pinned separately. Closes #638
…chema merged_constraints resolved a $ref as components[name] and absorbed the result without checking it. A miss handed it nil, which takes the non-Hash early return: no required fields, no type constraints, no items, and nullable true. Absorbed into the enclosing conjunction that does not read as "unknown", it reads as "unconstrained", so every example sitting under a broken pointer validated -- root nulls included -- and the run counted them among the checked and exited 0. Point the GetTodolistOrGroup 200 response schema at a component that does not exist, delete `color` from one example and null the other, and before this change the gate reported "37 checked" and exit 0 while validating neither. Those are the two defects it was built to catch, #637 and the root null, swallowed by a typo in a pointer. Resolution now raises UnresolvableRef, naming the ref: when it is not a components/schemas pointer at all, when the component is absent, and when it is present but not a schema object. instance_errors converts it into an ordinary path-tagged finding, so both callers report it beside every other error and exit non-zero, and the message carries the most specific path the innermost frame had. A $ref back to an already-visited component is still a cycle rather than a failure. The generated openapi.json has 422 distinct refs, all of them resolvable components/schemas pointers to schema objects, so this is strictly a guard: check-fixture-coverage, the other caller, is unaffected. Self-test case 17a is the reproduction, deliberately composite -- the bad pointer plus the missing `color` plus the root null -- so removing the check makes it go red for the reason that matters: the gate finds nothing whatsoever to say about two examples that contradict their schema twice over. 17b covers a ref resolving to a non-object.
…lishes `spec/smithy-build.json`'s `jsonAdd` can append to a schema's `required` array in the OpenAPI projection — the route #630/#637 took for `Todolist.color`, because the shape carries `@examples` and Smithy cannot express a `null` in an example for a String. `smithy validate` checks `@examples` against the *Smithy* model, where the member is still natively optional; the requiredness arrives afterwards. Nothing validated the projected examples against the projected schema, so #637 shipped both `GetTodolistOrGroup` response examples without `color` while the schema declared it required, and a bot reviewer caught it rather than CI. scripts/check-projected-examples.rb walks every example openapi.json publishes — response, request-body and parameter — and validates each against the sibling schema through the extracted composition-aware walk. `examples` maps hold Example Objects (value under `value`, `$ref`s into components/examples resolved, `externalValue` reported as skipped rather than fetched) while the singular `example` IS the value; conflating the two would silently unwrap any payload with a field named `value`. It compares exactly what is published against exactly the schema it is published under, and unwraps nothing. That is only possible because #644 landed first: the bare-response mappers used to rewrite a single-property `*ResponseContent` into the bare payload while leaving the wrapper on the example, and `BareResponseExampleMapper` now mirrors the unwrapping onto examples so the two agree. A gate that also accepted the wrapped shape could not tell a correct example from a regression in that mapper — so this gate guards it too, and self-test case 7 puts the wrapper back and asserts the run goes red. The gate refuses to pass vacuously: validating zero response examples — the class the seam lives in — is reported as a failure, not a green run. The self-test drives 1 positive + 14 negative/skip cases through the real checker via `PROJECTED_EXAMPLES_OPENAPI`. Its header carries the measured mutation matrix: which cases go red when each guard is removed from a scratch copy, including a mutation that unwires the validator while leaving every walk intact, so coverage and judgement are pinned separately. Closes #638
…chema merged_constraints resolved a $ref as components[name] and absorbed the result without checking it. A miss handed it nil, which takes the non-Hash early return: no required fields, no type constraints, no items, and nullable true. Absorbed into the enclosing conjunction that does not read as "unknown", it reads as "unconstrained", so every example sitting under a broken pointer validated -- root nulls included -- and the run counted them among the checked and exited 0. Point the GetTodolistOrGroup 200 response schema at a component that does not exist, delete `color` from one example and null the other, and before this change the gate reported "37 checked" and exit 0 while validating neither. Those are the two defects it was built to catch, #637 and the root null, swallowed by a typo in a pointer. Resolution now raises UnresolvableRef, naming the ref: when it is not a components/schemas pointer at all, when the component is absent, and when it is present but not a schema object. instance_errors converts it into an ordinary path-tagged finding, so both callers report it beside every other error and exit non-zero, and the message carries the most specific path the innermost frame had. A $ref back to an already-visited component is still a cycle rather than a failure. The generated openapi.json has 422 distinct refs, all of them resolvable components/schemas pointers to schema objects, so this is strictly a guard: check-fixture-coverage, the other caller, is unaffected. Self-test case 17a is the reproduction, deliberately composite -- the bad pointer plus the missing `color` plus the root null -- so removing the check makes it go red for the reason that matters: the gate finds nothing whatsoever to say about two examples that contradict their schema twice over. 17b covers a ref resolving to a non-object.
…lishes (#638) (#652) * Extract the composition-aware instance validator so a second gate can reuse it `instance_errors` and the `merged_constraints` walk beneath it were top-level methods inside scripts/check-fixture-coverage.rb, which runs its whole check on load — so a second gate could not require them without also running the fixture guard. Moved verbatim into scripts/schema_instance_validator.rb, in the scripts/bc3_route_normalizer.rb style: one definition, two callers, no parallel validator to drift. `module_function` plus a top-level `include` keeps every existing call site receiverless and unchanged, so this is behaviour-preserving; the guard's own self-test (1 positive + 8 negative + 20 synthetic cases) is what says so. What stays in the fixture guard is what is about the fixture guard: the concrete-instance rule and the #408 rich-text-emitter inventory. * Validate the projected examples against the schema the projection publishes `spec/smithy-build.json`'s `jsonAdd` can append to a schema's `required` array in the OpenAPI projection — the route #630/#637 took for `Todolist.color`, because the shape carries `@examples` and Smithy cannot express a `null` in an example for a String. `smithy validate` checks `@examples` against the *Smithy* model, where the member is still natively optional; the requiredness arrives afterwards. Nothing validated the projected examples against the projected schema, so #637 shipped both `GetTodolistOrGroup` response examples without `color` while the schema declared it required, and a bot reviewer caught it rather than CI. scripts/check-projected-examples.rb walks every example openapi.json publishes — response, request-body and parameter — and validates each against the sibling schema through the extracted composition-aware walk. `examples` maps hold Example Objects (value under `value`, `$ref`s into components/examples resolved, `externalValue` reported as skipped rather than fetched) while the singular `example` IS the value; conflating the two would silently unwrap any payload with a field named `value`. It compares exactly what is published against exactly the schema it is published under, and unwraps nothing. That is only possible because #644 landed first: the bare-response mappers used to rewrite a single-property `*ResponseContent` into the bare payload while leaving the wrapper on the example, and `BareResponseExampleMapper` now mirrors the unwrapping onto examples so the two agree. A gate that also accepted the wrapped shape could not tell a correct example from a regression in that mapper — so this gate guards it too, and self-test case 7 puts the wrapper back and asserts the run goes red. The gate refuses to pass vacuously: validating zero response examples — the class the seam lives in — is reported as a failure, not a green run. The self-test drives 1 positive + 14 negative/skip cases through the real checker via `PROJECTED_EXAMPLES_OPENAPI`. Its header carries the measured mutation matrix: which cases go red when each guard is removed from a scratch copy, including a mutation that unwires the validator while leaving every walk intact, so coverage and judgement are pinned separately. Closes #638 * Run the projected-example gate in `make check` and in CI Two places, because membership in one is not membership in the other: the `spec-gates` job ENUMERATES its targets rather than invoking `make check`, so a target added only to the Makefile would run on developer machines and nowhere else — the #580 gap. Joined to `check-targets:` (the list `check:` sub-makes, since #631 wrapped it in a lockfile snapshot) and added as a `spec-gates` step ahead of that job's lockfile diagnostics, which stay last by design. Ruby and openapi.json are its only inputs, so it needs nothing the job does not already have. Run under LC_ALL=C in CI, matching the fixture-coverage job: the reads are pinned to UTF-8, and running the non-UTF-8-locale path in CI is what keeps them that way. * Judge a root null instead of exempting it `instance_errors` returned no errors for any null value. For a NESTED null that is correct and deliberate: the Smithy-derived OpenAPI under-marks some nullable optionals, and something above has already looked — a required-but-null field is caught by the required loop in its parent, a null element by the items check in its array. The exemption means "the enclosing context already judged this". A root value has no enclosing context, so nothing was standing behind the exemption. A published example of `value: null` under a non-nullable schema returned clean AND was counted as validated — the projected-example gate approving exactly the class of contradiction it was built to catch. Measured before the fix, against a crafted spec: 37 checked, exit 0, with a null `GetTodolistOrGroup` response example and a null `accountId` parameter example both waved through. Root nulls are now checked against the schema's nullability. Two self-test cases pin the guard and a third pins its over-correction: rejecting every root null would pass both negative cases and then start failing legitimate examples for required-and-nullable shapes — the class `Todolist.color` belongs to — so case 16 asserts a null IS accepted where the schema permits one. check-fixture-coverage shares this validator and is unaffected: it rejects a null root before calling in, so the new branch is unreachable from there. Its own self-test (1 positive + 8 negative + 20 synthetic) still passes. * Treat an unresolvable schema $ref as an error, not an unconstrained schema merged_constraints resolved a $ref as components[name] and absorbed the result without checking it. A miss handed it nil, which takes the non-Hash early return: no required fields, no type constraints, no items, and nullable true. Absorbed into the enclosing conjunction that does not read as "unknown", it reads as "unconstrained", so every example sitting under a broken pointer validated -- root nulls included -- and the run counted them among the checked and exited 0. Point the GetTodolistOrGroup 200 response schema at a component that does not exist, delete `color` from one example and null the other, and before this change the gate reported "37 checked" and exit 0 while validating neither. Those are the two defects it was built to catch, #637 and the root null, swallowed by a typo in a pointer. Resolution now raises UnresolvableRef, naming the ref: when it is not a components/schemas pointer at all, when the component is absent, and when it is present but not a schema object. instance_errors converts it into an ordinary path-tagged finding, so both callers report it beside every other error and exit non-zero, and the message carries the most specific path the innermost frame had. A $ref back to an already-visited component is still a cycle rather than a failure. The generated openapi.json has 422 distinct refs, all of them resolvable components/schemas pointers to schema objects, so this is strictly a guard: check-fixture-coverage, the other caller, is unaffected. Self-test case 17a is the reproduction, deliberately composite -- the bad pointer plus the missing `color` plus the root null -- so removing the check makes it go red for the reason that matters: the gate finds nothing whatsoever to say about two examples that contradict their schema twice over. 17b covers a ref resolving to a non-object.
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.
* 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.
Closes #630.
BC3 emits
colorandcomments_app_urlon every projection ofTodolist, and the published contract said they might be absent._todolist.json.jbuildercallsjson.colorin both branches of itstodolist_group?conditional, and emitscomments_app_urlfrombucket_recording_comments_url— a route helper, which returns a String or raises. Optional pushed an absence-check onto every consumer, in every language, for keys that are never absent.Both become required. Operation count is unchanged at 241 (re-derived from
behavior-model.json, which does not move): this is a field-presence change, not a surface change.Why
colorgoes through the projection andcomments_app_urldoes notcoloris required and nullable.recordings.coloris a nullable integer column, so an uncolored list or group sends an explicitnull— for a group, per bc3, that is the ordinary case.Native Smithy
@requiredcannot express that on this shape.Todolistcarries@examples(viaGetTodolistOrGroup), and Smithy cannot put anullinto an example for aStringshape, so@requiredfails validation two ways:Missing required structure member 'color'if the examples stay faithful,Expected string value for string shape 'smithy.api#String'; found null valueif you try to supply the group's true value. The workaround — a fictional non-null example — trades a faithful example for a type the projection can state anyway. The sibling shapes carrying the same field (Wormhole,Folder,FolderWithProjects) are natively@requiredonly because none of them carries examples.So the presence half is layered onto the OpenAPI projection alongside the nullability half that was already there, one pointer in
spec/smithy-build.json:jsonAddapplies JSON-Patchaddsemantics, sorequired/-appends to the array rather than replacing it. The Smithy member stays natively optional and the null example is untouched:comments_app_urlis never null, so it has no null-in-example problem and takes native@required. Its only cost is that both examples must now carry the key — a real in-app comments URL, not a fiction. Confirmed against bc3 at the pinned revision4dd2926f8af0(spec/api-provenance.json),app/views/api/todolists/_todolist.json.jbuilder:The
colordoc comment argued for optional on fixture-cost grounds ("Optional is the weaker, safer claim…"). That reasoning is superseded and is replaced by the projection rationale, so nothing in the model contradicts the model.The projected examples needed the same treatment — and nothing checks that
Appending to
requiredin the projection made bothGetTodolistOrGroupresponse examples invalid against the schema they illustrate, andsmithy validatecannot see it: it validates@examplesagainst the Smithy model, where the member is still natively optional. The requiredness only exists downstream. A bot reviewer caught it, not CI.Fixed the same way, in
12b02cda8:"blue"for the list, an explicitnullfor the uncolored group. Writing them inexamples.smithyis not an option for the same reason@requiredwas not: nonullin aStringexample, and a made-up color for the group trades a faithful example for nothing. Verified by diffing each example againstTodolist.requiredrather than by eye:The seam is real and is now #638: any
required/-jsonAddis unchecked against the projected examples. Two pointers do that today, both added here. A gate is deliberately not in this PR — it holds the v0.13.0 spec train with #629 draft-held behind it, and a new CI check wants its own review plus the self-test this repo asks of its checkers.What consumers see — this is the breaking part
color?: string | null/comments_app_url?: stringcolor: string | null/comments_app_url: stringval color: String? = null/val commentsAppUrl: String? = nullval color: String?/val commentsAppUrl: StringColor *string \json:"color,omitempty"`/CommentsAppUrl *string `…,omitempty``Color *string \json:"color"`/CommentsAppUrl string `json:"comments_app_url"``public var color: String?/public var commentsAppUrl: String?public let color: String?/public let commentsAppUrl: String, both required init paramsNotRequired[str | None]/NotRequired[str]str | None/strrequired_fields, andto_hkeeps an explicit nullcolorNullability is preserved everywhere
coloris concerned — Go stays*string, Kotlin and Swift stay optional-typed. Only presence changed, which is the whole point.comments_app_urlis never null, so it becomes a non-optional type in the four typed SDKs.Two consequences worth naming:
CommentsAppUrlgoes pointer → value. The hand-written wrapper'sderef(gtl.CommentsAppUrl)stopped compiling (type string does not match *T) and is now a plain assignment. That is a compile break for anyone touchinggenerated.Todolistdirectly.to_hno longer drops a nilcolor. The generator switched that struct from.compactto.reject { |k, v| v.nil? && !["color"].include?(k) }, so a null color round-trips as an explicit"color" => nilinstead of vanishing.Migration: a consumer who tolerated an absent
coloris fine at runtime — the wire never changed. What breaks is the generated type: a KotlinString?with a default, a Swift optional, a Go pointer, a TS?:all become required, so exhaustive constructors and struct literals need the field. This is in v0.13.0 becauseoptional → @requiredis itself breaking; deferring it does not defer the cost, it relocates it into a release with no breaking window.Swift's diff is ~110 lines, not one — and it is the generator's documented path
Making
colorrequired-and-nullable pushesTodolistover the threshold whereModelEmitter.emitRequiredNullableCodingreplaces the synthesisedCodablewith an explicitCodingKeys+init(from:)+encode(to:). Per member: required-and-nullable →decode(T?.self)(rejects a missing key, accepts JSON null); required →decode(T.self); optional →decodeIfPresent.The
CodingKeyscases are barecase appUrlwith no raw value, which is correct here and not a bug:BaseServicesets.convertFromSnakeCase/.convertToSnakeCase, which rewrite the wire key before the lookup, so an explicit= "app_url"would be matched against the converted key and fail.swift/Sources/BasecampGenerator/ModelEmitter.swift:270documents exactly that.Todolistis joining an existing set, not inventing a shape:Wormhole,Folder,FolderWithProjects,SearchType,SearchResult,MyNote,DraftandTimelineEventDataalready emit this same block. It is a real generated-surface change, so it is called out rather than waved through as churn.check-fixture-coveragenow covers these fields — proven, not assertedThe guard null-checks only required fields. That is precisely why #628's mismatch —
colorpublished non-nullable while a manifest fixture carried"color": null— reached review instead of being caught. Making these required brings them under it.Red proof. One mutated fixture tree (
colorkey removed fromtodolist_groups/get.json,comments_app_urlnulled intodolists/get.json), run against both specs viaFIXTURE_DIR/FIXTURE_OPENAPI:Identical fixtures, exit 0 before and exit 1 after: the guard was blind to both keys and now is not.
The nullable half still behaves.
spec/fixtures/todolist_groups/get.jsoncarries a genuine"color": nulland passes unmodified — required-and-nullable accepts the null it is supposed to accept, and rejects only the absence.Fixture sweep — 22 failures, and no missing key in any captured body
The two runtime-strict SDKs failed as predicted; the four permissive ones stayed green throughout.
TodolistsServiceTestfailures,MissingFieldExceptionTodolistsServiceExtensionsTests+GeneratedServiceTests,DecodingError.keyNotFound: Key 'color'encoding/jsonis permissiveSeveral Swift failures were tests asserting an unrelated failure mode (
expected DecodingError.typeMismatch, got …keyNotFound) that never got far enough to see it — worth noting, since a shallow read of the log makes them look like new defects.Each SDK's canonical body helper gains both keys rather than each call site patching them in, so omitting one now has to be deliberate.
coloris threaded as a parameter so the group variants carry an explicit JSONnull: Kotlin takes raw JSON, Swift takesAnysoNSNull()can spell the null aString?would have silently turned into an absent key. Seven synthetic mock bodies inconformance/tests/todolists_write.jsoncarried neither key;todolists_read.jsonalready carried both.Fixing only what broke was not enough, and a reviewer caught that twice.
AGENTS.md§"Every Changed Field/Path Requires" says every response stub for a changed path must carry a newly required field, and adds that the automated guard covers only the manifest'd shared fixtures — "one-off inline stubs remain the reviewer's responsibility." The permissive SDKs stayed green while holding bodies BC3 cannot produce.4c36d7b94and5803e9c54fix them: Ruby's seven minimal response hashes, Python's_todolist(), TypeScript'smockGroupand both elements ofmockGroups.colorisnullon the group stubs, not omitted.The second miss is worth naming: that pass searched for stubs by grepping
type: "Todolist", andmockGroupsdoes not carrytype, so a marker-based search skipped it. The final enumeration is by route — every test touching/todosets/{id}/todolists.json,/todolists/{id}or/todolists/{id}/groups.json— which cannot miss a stub for lacking a field. Everything else is audited and complete: Go and the remaining Ruby/Python/TypeScript stubs derive from the shared fixtures, and Kotlin'sTodolistGroupsServiceTest.groupBodyalready carried both, which is why it never failed. The other route matches are not Todolist bodies —/todolists/{id}/todos.jsonreturns Todos, the"type": "Todolist"hits elsewhere areparentreferences (TodoParent, nocolor), andtodosets.test.tsonly holds atodolists_urlstring.No captured body is missing either key. All six
Todolist-shaped objects underspec/fixtures/carry both — which is what #628's reading of the jbuilder predicted, and the finding this PR was told to stop and report did not materialise.Verification
Full
make checkre-run at every head, SHA pinned on both sides of each run. At the final head:Earlier heads, same shape:
bbc5a2768exit 0,12b02cda8exit 0,4c36d7b94exit 0, each withPRE_SHA == POST_SHA.Selected lines from that run:
Two caveats on that log, stated rather than glossed:
jvmTestreportsUP-TO-DATEin this run — cache-hit, not execution evidence. Kotlin actually executed in a separate./gradlew --offline jvmTest(exit 0) after the fixture fix, and authoritatively in the CI Kotlin Tests job, green onbbc5a2768.Executed 353 tests, plusswift-check-driftreportingNo drift detectedagainst the committed generated tree.Branch is 1 commit behind
origin/main(c95d81ceb), a Ruby comment-only change toschedules_extensions.rb— disjoint from every file here.copilot-pull-request-reviewererrored on all five attempts ("Copilot encountered an error and was unable to review this pull request") — the known flake. Codex reviewed at every head and its three findings are addressed above; CI is green.