Skip to content

fix: correct the defects a pre-release review found in this cycle's work - #1161

Merged
rmyndharis merged 2 commits into
mainfrom
fix/sdk-null-semantics-and-live-docs
Aug 8, 2026
Merged

fix: correct the defects a pre-release review found in this cycle's work#1161
rmyndharis merged 2 commits into
mainfrom
fix/sdk-null-semantics-and-live-docs

Conversation

@rmyndharis

Copy link
Copy Markdown
Owner

A systematic regression review of everything since v0.14.5 — 90 files across three repositories — found four shipping defects and several smaller ones, all introduced by this cycle. None had reached a release. This fixes the ones in core.

The session-config route needs three states; two SDKs could send two

The route takes an absent key (leave unchanged), an explicit null (clear to default) and a value (set it). Its DTO says why the middle one is not decoration:

maxReconnectAttempts defaults to unlimited, which no in-range number can express, so without it a session could never be returned to unlimited retries once capped.

Go declared *int with omitempty — a nil pointer is omitted, never written as null. Proven by running it: json.Marshal of the zero value produced {}.
Java used new Gson(), which drops nulls by default.

So through either SDK, a capped session could never be returned to unlimited — the one operation the route exists for.

Both now carry explicit clear* flags: Go through a MarshalJSON, Java through a serializer registered for that one type.

Why not just serializeNulls() in Java: it would turn every unset field of every other request body into an explicit null — which this same route reads as "reset to default". That is a worse bug than the one it fixes, so the shared Gson still omits nulls and only this body type uses the null-emitting one.

Worth recording: the first attempt at the Java fix reproduced the original defect one layer down. The custom serializer built the right tree and Gson's writer silently dropped the JsonNull anyway. The test caught it; reading the code had not.

Three response types dropped fields the API sends

  • the per-participant group result omitted message — the same defect class the commit that introduced it set out to fix
  • the product-send response omitted timestamp, which the contract marks required
  • Python's new 503 class was never exported from the package root, so the documented import failed for exactly the class that had been added

A running gateway served an invalid document

The schema-validity pass went into scripts/export-openapi.ts only. The document is produced in two places — that, and src/main.ts for the live /api/docs. So the committed openapi.json was clean while a running gateway kept serving a document that fails validation, which is precisely the scenario the original PR described.

Both producers now run the same passes in the same order, held there by a structural test. Mutation-checked: removing the pass from main.ts fails it.

Smaller, same cycle

  • the four path parameters this cycle added were the only ones in the document without a schema type (225 others had one); now typed, with a test
  • two Go doc comments and one Java javadoc were orphaned by insertions — godoc was attributing text to the wrong function
  • a Python comment had drifted from the class it describes
  • the JavaScript SessionConfig doc claimed two nullable fields where the DTO has one
  • every SDK error taxonomy and the design doc's three tables omitted the new 503 class
  • the two exclusion lists the README asserts "have to agree" disagreed in both directions

Verification

All five SDKs build and test clean. lint, format:check, openapi:check, check:sdk-routes, check:versions, build, tsc --noEmit, and the full unit suite — 5118 tests — green.

⚠️ One full-suite run during this work reported 9 failures. That is a pre-existing flake in chat-media-archive.service.spec.ts, reproduced on the 4cb7aa10 tree from before any of this cycle's commits (2 failures in 3 runs there). Two consecutive clean runs followed here. It is tracked separately and is not caused by these changes.

A systematic review of everything since v0.14.5 found four shipping defects and
several smaller ones, all introduced by this cycle. None had reached a release.

The session-config route takes THREE states per field: an absent key leaves the
value unchanged, an explicit null clears it to the default, a value sets it. The
Go and Java SDKs could emit only two. Go's `*int` with `omitempty` omits a nil
pointer rather than writing null; Gson drops nulls by default. So restoring
`maxReconnectAttempts` to unlimited — which the DTO says no in-range number can
express, and which is the reason the route exists — was unreachable through
either. Both now carry explicit `clear*` flags, Go through a MarshalJSON and
Java through a serializer registered for that one type on a null-emitting Gson.
The shared Gson keeps omitting nulls: applying serializeNulls() globally would
turn every unset field of every other body into an explicit null, which this
same route reads as "reset to default" — a worse bug than the one it fixes.

Three response types dropped fields the API sends: the per-participant group
result omitted `message`, the product-send response omitted `timestamp`, and
Python's new 503 class was never exported from the package root, so the
documented import failed for exactly the class that had been added.

The OpenAPI validity pass was added to the export script only. The document is
produced in two places, and a running gateway kept serving one that fails schema
validation while the committed artifact was clean. Both producers now run the
same passes in the same order, held there by a test.

Also: the four path parameters this cycle added were the only ones in the
document without a schema type, and a structural test now covers that; two Go
doc comments and one Java javadoc had been orphaned by insertions; a Python
comment had drifted from the class it describes; the JavaScript SessionConfig
doc claimed two nullable fields where the DTO has one; every SDK error taxonomy
and the design doc's three tables omitted the new 503 class; and the two
exclusion lists that the README asserts "have to agree" disagreed in both
directions.
Four PRs landed into the same unreleased section, and each conflict was resolved by
appending its own `### Fixed` rather than merging into the existing one. The block
ended with `Fixed`, `Added`, `Fixed`, `Fixed` — three headings for one type, with
`Added` buried in the middle, so the release notes read as four unrelated lists and a
reader scanning for fixes would stop at the first block.

Keep a Changelog allows one section per change type per release, in a fixed order.
The entries are unchanged: they were regrouped mechanically, and every one is present
byte-for-byte — the only content difference is that the single `Added` entry now sits
above the fixes instead of between them.
@rmyndharis
rmyndharis merged commit eee4881 into main Aug 8, 2026
16 checks passed
@rmyndharis
rmyndharis deleted the fix/sdk-null-semantics-and-live-docs branch August 8, 2026 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant