Validate the projected examples against the schema the projection publishes (#638) - #652
Conversation
github-actions
Bot
added
the
github-actions
Sensitive Change Detection (shadow mode)This PR modifies control-plane files:
|
![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: b7f86d66ee
ℹ️ 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".
| # yet the wire sends null), so flagging them would be a false positive. | ||
| def instance_errors(prefix, value, schema, components, depth = 0) | ||
| return [] if depth > 60 | ||
| return [] if value.nil? # optional-null tolerated; required-/element-null handled in context |
There was a problem hiding this comment.
When a response, request-body, or parameter example has value: null while its sibling schema is non-nullable, this early return reports no errors, so the new checker counts the example as validated and can exit successfully. The null exemption is intended for optional nested fields whose context is checked by the parent, but a top-level example has no such parent; validate root nullability before returning so the gate does not approve a published example that contradicts its schema.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 41a60f35c, which is in the current head e69218c0b. The thread still reads as unresolved only because its anchor went stale — 41a60f35c rewrote the exact lines it was pinned to, so GitHub reports line: null, isOutdated: true.
The early return is gone. scripts/schema_instance_validator.rb:215-223 now judges a root null instead of exempting it:
if value.nil?
return [] unless depth.zero?
_, _, _, root_nullable, = merged_constraints(schema, components)
return [] if root_nullable
label = prefix.empty? ? "(root)" : prefix
return ["#{label}: value is null but the schema is not nullable"]
endNested nulls keep the exemption, and scripts/schema_instance_validator.rb:200-211 now states why that is sound rather than merely convenient: 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. A root value has no enclosing context, which is exactly the case you identified.
It consults nullability rather than banning root nulls, because banning them would start rejecting legitimate examples for required-and-nullable shapes — the class Todolist.color belongs to, whose null example is injected through the same jsonAdd route this gate exists to police.
Pinned in both directions in scripts/test-check-projected-examples.rb:
:400— 15a, a response example set tovalue: nullagainst a non-nullable schema, expected to fail.:407— 15b, the same for a parameter example, because the early return sat below every kind of caller alike.:423— 16, a root null against a schema that permits null, expected to pass. Without this the cheapest way to satisfy 15a/15b would look correct.
Verified locally against the current head: check-projected-examples.rb exits 0 on the real spec (37 checked — 10 response, 5 request-body, 22 parameter) and the self-test reports 2 positive + 16 negative/skip cases, all passing.
There was a problem hiding this comment.
Pull request overview
Adds a CI gate that validates every example published in the generated openapi.json against the sibling schema in the projection, closing the seam where spec/smithy-build.json's jsonAdd .../required/- can append a required field that smithy validate never checks (it only sees the pre-projection Smithy model). This is the gate that would have caught the #637 defect (GetTodolistOrGroup examples missing projection-added color). The change is stacked on #648, whose BareResponseExampleMapper makes response examples comparable as-published without unwrapping.
Changes:
- Extracts the composition-aware structural walk into
scripts/schema_instance_validator.rband reuses it from bothcheck-fixture-coverage.rband the new gate. - Adds
scripts/check-projected-examples.rb(response, request-body, and parameter examples;$ref/externalValuehandling; a liveness floor rejecting a vacuous pass) plus a 1-positive/14-negative self-test. - Wires
check-projected-examplesintocheck-targetsand thespec-gatesCI job (run underLC_ALL=C).
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 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
scripts/schema_instance_validator.rb |
New module holding the shared, composition-aware required/type/nullability walk extracted verbatim from the fixture guard. |
scripts/check-fixture-coverage.rb |
Drops the inline walk; requires and includes the shared validator, keeping receiverless call sites unchanged. |
scripts/check-projected-examples.rb |
New gate validating each projected example against its sibling schema, with a liveness floor against vacuous passes. |
scripts/test-check-projected-examples.rb |
Self-test driving crafted mutations of the real spec through the checker via PROJECTED_EXAMPLES_OPENAPI. |
Makefile |
Adds the check-projected-examples target (checker + self-test) and lists it in .PHONY and check-targets. |
.github/workflows/test.yml |
Adds a spec-gates step running the gate under LC_ALL=C. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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. 👍 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". |
The gate could pass while validating nothingFound by adversarial review of this PR, not by the gate's own self-test — which is the part worth sitting with, since the self-test's guard table is the artifact that is supposed to make that impossible. The defect
name = ref_name(schema)
if name && !visited.include?(name)
visited << name
absorb.call(components[name])A miss hands BeforeUsing the AfterThe fixResolution raises This is strictly a guard on today's document: the generated Self-test: cases 17a / 17b17a is the reproduction above, deliberately composite — bad pointer plus the missing RED, driving the pre-fix validator through the unchanged suite via Exactly and only the two new cases move, and the literal text they fail on is the vacuous pass. GREEN with the fix: The header's guard table and the "third hole" prose are updated: this is the fourth, and all four are the same shape — a path that returns success without examining the thing — and none of the four was found by this self-test. Second consumer
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f8a637607
ℹ️ 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".
| if value.nil? | ||
| return [] unless depth.zero? |
There was a problem hiding this comment.
When an optional object field is present as null, this nested-null exemption returns before merged_constraints resolves the field schema. If a mapper or hand edit leaves that field as $ref: '#/components/schemas/NoSuchSchema' and the published example has "field": null, the projected-example gate still exits 0 and counts the example as checked even though the schema pointer is unreadable—the failure mode the new UnresolvableRef guard is meant to make fatal. Resolve the schema for ref errors before applying the optional-null exemption, while still skipping the nested nullability assertion.
Useful? React with 👍 / 👎.
… 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.
…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
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.
`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.
…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.
8f8a637 to
e27387d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e27387de83
ℹ️ 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".
| # (oneOf is validated as "at least one" — enforcing exactly-one would need full | ||
| # discriminator/enum/const validation to avoid false positives). | ||
| alt_groups.each do |branches| | ||
| next if branches.any? { |branch| instance_errors(prefix, value, branch, components, depth + 1).empty? } |
There was a problem hiding this comment.
When an anyOf/oneOf schema has a broken $ref branch and the example satisfies another branch, this branches.any? check stops as soon as one branch returns no errors and drops the error returned by the unreadable branch. That lets check-projected-examples pass and count the example as validated even though part of the sibling schema could not be read, which undercuts the new fatal $ref guard; surface UnresolvableRef from any alternative before accepting a matching branch.
Useful? React with 👍 / 👎.
|
Merging on independent adversarial review rather than a Codex pass at head — Codex last reviewed That review found a real defect in this gate, which is worth recording since it is the failure mode the gate exists to prevent: an unresolvable schema Fixed by raising The second consumer, Also verified in review, since none of it was implied by CI: the root-null exemption is correctly scoped to nested nulls only and consults nullability rather than banning nulls, so required-and-nullable shapes like Rebased onto the merged #648 with |
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.
* 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 #638.
spec/smithy-build.json'sjsonAddcan append to a schema'srequiredarray in the OpenAPI projection.smithy validatechecks@examplesagainst the Smithy model, where the member is still natively optional — the requiredness only exists downstream of it. Nothing compared the projected examples to the projected schema, so a projection-added required field could be missing from a published example with every gate green.That is not a hypothesis. #637 shipped both
GetTodolistOrGroupresponse examples withoutcolorwhileTodolist.requireddeclared it, and a bot reviewer caught it. This is the gate that would have.What it checks
Every example
openapi.jsonpublishes, against the sibling schema, through the same composition-aware walk the fixture guard uses:Nothing is skipped.
examplesmaps hold Example Objects (value undervalue,$refs intocomponents/examplesresolved,externalValuereported as skipped rather than fetched — an offline gate that reaches the network is a gate that skips when the network is down); the singularexampleis the value. Conflating those would silently unwrap any payload with a field namedvalue.Red proof
The #637 defect reproduced in today's projection —
colordeleted from bothGetTodolistOrGroupexamples,Todolist.requiredstill declaring it:Two errors, both naming
color, nothing else. Unmodified tree:REAL_EXIT=0. Both exit codes were written into the captured log by the runner and grepped back out, not read off a terminal.Why not the historical commits. An earlier revision of this PR proved the gate against
f9b9c92a5directly and bisected it green at12b02cda8. That proof no longer isolates anything and I am not quoting it: those trees predateBareResponseExampleMapper, so under this gate their wrapped examples are themselves the fault — 47 errors atf9b9c92a5and 47 at12b02cda8alike,colornamed in both but buried. A bisect that is red on both sides of the fix is not evidence. The mutation above is the same defect stated in the projection this gate actually guards.Why this depends on #648
The bare-response mappers rewrite a single-property
*ResponseContentschema into the bare payload — BC3 returns bare bodies, Smithy's restJson1 needs a wrapper. Until #644 they rewrote the schema only, so every published response example still carried the wrapper:#648's
BareResponseExampleMappermirrors the unwrapping onto examples, so the two now agree and this gate compares them directly.It deliberately does not unwrap anything itself. A gate that also accepted the wrapped shape could not tell a correct example from a regression in that mapper. Comparing exactly what is published against exactly the schema it is published under is the whole claim — and it means this gate now guards
BareResponseExampleMapperas well. Self-test case 7 puts the wrapper back on one example and asserts the run goes red.A count I had wrong
An earlier revision of this PR reported "2 validated response examples". That number was not measuring what it sounded like. It counted examples that passed because this gate unwrapped them first — the published documents were still self-contradictory, and the gate was validating a payload the spec did not publish.
Post-#648 the number means what it says: 10 of 10 response examples validate as published, no unwrapping, no skips. The correction came from the GetUpcomingSchedule lane; I confirmed it before changing anything by running a no-unwrap build of this gate against both trees — red on
main(47 errors), green on #648 (37 checked, 0 failures).Corrections to the issue
1.
GetTodolistOrGroupis not the only operation with a response example. The issue says the other four are input-only. Four operations carryresponses/200/.../examples:GetTodolistOrGroup,ListRecordings,UpdateProjectAccessandUpdateSubscription. OnlyTrashRecordingis parameters-only. What was true one level up is that onlyGetTodolistOrGroupdeclared anoutput— which is precisely what #644 fixed.2.
conformance/runner/ruby/schema-walker.rbis not reused. The issue suggests it for resolving an operation's response schema. This walk finds examples by descendingpathsitself, so the schema is already in hand at every example — andfind_response_schema(operationId)returns only the first 2xx schema for an operation, not the one attached to the response the example sits under. Less precise, not more.instance_errors— the reuse that matters — is reused.Reuse
instance_errorsand themerged_constraintswalk beneath it were top-level methods insidescripts/check-fixture-coverage.rb, which runs its whole check on load — so nothing could require them without also running the fixture guard. The first commit moves them verbatim intoscripts/schema_instance_validator.rb, in thescripts/bc3_route_normalizer.rbstyle: one definition, two callers, no parallel validator to drift.module_functionplus a top-levelincludeleaves every existing call site unchanged; the fixture guard's own self-test (1 positive + 8 negative + 20 synthetic cases) is what says the move was behaviour-preserving.Self-test
1 positive + 14 negative/skip cases through the real checker via
PROJECTED_EXAMPLES_OPENAPI, each a deep copy of the realopenapi.jsonwith one mutation.Non-vacuity is measured, not asserted — ten guards removed one at a time from scratch copies via
PROJECTED_EXAMPLES_CHECKER. Every guard turns specific cases red; every case is red under at least one:components/examples$refresolutionexternalValueskip:errordistinct from:skipinstance_errorsresult discardedThe last row matters most: it unwires the validator while leaving every walk intact, so a gate that visits all 37 examples and concludes nothing still goes red. Coverage and judgement are pinned separately — a gate that iterates correctly and asserts nothing is the failure a green suite hides best.
The gate also refuses to pass vacuously at runtime: validating zero response examples is a failure, not a green run. Case 10 strips every response example in the document and asserts exactly that.
Wiring
Both places, because membership in one is not membership in the other —
spec-gatesenumerates its targets rather than invokingmake check, which is the #580 gap:check-targets:(the listcheck:sub-makes, since Stopmake checkrewriting typescript/package-lock.json #631 wrapped it in a lockfile snapshot) — 42 → 43, re-derived from the line at wiring time and written down nowhere.spec-gatesstep, placed ahead of that job's lockfile diagnostics so those stay last by design. Ruby andopenapi.jsonare the gate's only inputs, so it needs nothing that job does not already have. Run underLC_ALL=C, matching the fixture-coverage job: the reads are pinned to UTF-8, and exercising the non-UTF-8-locale path in CI is what keeps them that way.Verification
Full
make checkon this branch, exit code written into the captured log by the runner and grepped back out:HEADcaptured either side of the run, so the tree provably did not move underneath it:And the gate demonstrably ran inside that invocation rather than being silently skipped — a no-op gate being the exact failure this one exists to prevent:
Working tree clean afterwards.
make lint-actions(actionlint + zizmor) clean on the workflow change. The gate and its self-test both pass underLC_ALL=Cas well as UTF-8; it runs in ~0.2s and is cwd-independent.Summary by cubic
Adds a gate that validates every projected example in
openapi.jsonagainst its sibling schema to catch projection-addedrequiredmismatches and other contradictions. It also rejects root-null examples when the schema is not nullable and treats unresolvable schema$refas errors. Closes #638.New Features
scripts/check-projected-examples.rbto validate response, request-body, and parameter examples against their sibling schema. Resolves#/components/examplesrefs, reportsexternalValueas skipped, compares examples exactly as published (no unwrapping), and fails if zero response examples are validated. Addedcheck-projected-examplesmake target, wired intocheck-targets, and added a CI step (runs underLC_ALL=C). Self-test covers 2 positive + 18 negative/skip cases.Refactors
scripts/schema_instance_validator.rband reused it incheck-fixture-coverage.rb. Handles$ref(incl. siblings),allOf, andanyOf/oneOfas at-least-one; checks required fields, types, and nullability. Root nulls are now judged against schema nullability, and an unresolvable schema$refis a hard error.Written for commit e27387d. Summary will update on new commits.