Skip to content

fix(sdk-generator): stop dropping nullable on composite schemas - #76

Merged
rob-archastro merged 1 commit into
mainfrom
fix/sdk-generator-nullable-composites
Aug 16, 2026
Merged

fix(sdk-generator): stop dropping nullable on composite schemas#76
rob-archastro merged 1 commit into
mainfrom
fix/sdk-generator-nullable-composites

Conversation

@rob-archastro

Copy link
Copy Markdown
Contributor

Problem

The Elixir backend silently drops OpenAPI 3.0 nullable: true whenever it rides a composite schema. Proof from the released artifact (archastro-elixir v0.3.2, lib/archastro/generated/types/common.ex): an inline-expanded ref field decodes as {:optional, {:nullable, {:ref, …}}} while Message.acl — spec shape {"allOf": [{"$ref": …}], "nullable": true} — generates as bare {:optional, {:ref, ArchAstro.SDK.Types.Acl}}, so the generated decoder crashes on the null the platform legitimately sends. A live conformance audit found the same drop on three shapes: allOf + component-$ref (Sandbox.org_logo, TeamMembership.user, StatusPing.user, Message.acl, Task.current_lease, Agent.acl/source_solution, Actor.profile_picture, SolutionSummary.org_logo, SolutionTemplateSummary.details, …), oneOf unions (Message.user on all four message surfaces, REST and channel), and enum (Message.agent_mode). Scalar typed nullables worked.

The root cause is ordering in the shared frontend, packages/sdk-generator/src/frontend/schema-parser.ts (jsonSchemaToTypeRef): the $ref, enum, anyOf/oneOf, and allOf branches all return before the nullable check runs. Scalars fall through to the check and wrap correctly; composites never reach it. That's why the drop showed on both REST and channel surfaces — operation returns, body fields, and channel join/message/push schemas all flow through this one function.

Fix

Peel nullable off first, before any shape branch, using the exact mechanism the scalar path already used (strip the flag, recurse, wrap in {kind: "nullable"}) — the check simply moved to the top of jsonSchemaToTypeRef; no parallel mechanism was added. Every downstream emitter already handles the nullable TypeRef kind (Elixir emits {:nullable, descriptor} and | nil typespecs), so no backend changes were needed. Schemas without nullable: true are untouched — there is no blanket wrapping.

Diff stays inside packages/sdk-generator: the one reordering in schema-parser.ts plus tests.

Proof

Unit tests (written first; all 5 nullable-composite assertions failed on the old code with the exact dropped-wrapper shapes, then passed):

  • Frontend (__tests__/frontend/parse-spec.test.ts): nullable allOf-ref → {kind: "nullable", inner: {kind: "ref"}}, nullable oneOf → nullable union, nullable enum → nullable enum — plus a non-nullable control for each shape proving no blanket wrapping.
  • Elixir backend (__tests__/backends/elixir.test.ts): generated output contains {:nullable, {:ref, …}}, {:nullable, {:union, …}}, {:nullable, {:enum, …}}; non-nullable ref/enum controls stay unwrapped; and a channel push payload with a nullable ref emits {:nullable, {:ref, …}} — covering the channel emission path explicitly.

Regeneration against specs/platform-openapi.json (--lang elixir), before → after for the audit's four named fields:

Field Before After
Message.acl {:optional, {:ref, ArchAstro.SDK.Types.Acl}} {:optional, {:nullable, {:ref, ArchAstro.SDK.Types.Acl}}}
Message.user {:optional, {:union, [:string, {:ref, ArchAstro.SDK.Types.Message.Nested.UserVariant2}]}} {:optional, {:nullable, {:union, [:string, {:ref, ArchAstro.SDK.Types.Message.Nested.UserVariant2}]}}}
Message.agent_mode {:optional, {:enum, ["cli", "embedded"]}} {:optional, {:nullable, {:enum, ["cli", "embedded"]}}}
StatusPing.user {:optional, {:ref, ArchAstro.SDK.Types.User}} {:optional, {:nullable, {:ref, ArchAstro.SDK.Types.User}}}

The full regenerated diff touches 8 files — the REST type modules, v1/api.ex, and both channel modules (api_chat_channel.ex, api_activity_feed_channel.ex) — and every changed line is either a {:nullable, …} descriptor wrap, its | nil typespec counterpart, or a content-hash header. Already-working scalar nullables (e.g. Message.agent as {:nullable, :string}) are byte-identical before and after.

Existing tests pass: full sdk-generator suite 12 files / 404 tests green; channel-harness (which consumes parseOpenApiSpec) 8 files / 74 tests green; eslint clean on the changed files.

Not merged or published — the archastro-elixir regeneration/0.3.3 release happens separately.

🤖 Generated with Claude Code

OpenAPI 3.0 marks nullability with 'nullable: true', which legitimately
rides composite shapes without a sibling type: allOf + component $ref
(Message.acl, StatusPing.user, ...), oneOf unions (Message.user), and
enum (Message.agent_mode). jsonSchemaToTypeRef checked nullable only
after the $ref/enum/oneOf/allOf branches had already returned, so the
flag was silently dropped for every composite shape and the generated
Elixir decoders crashed on nulls the platform legitimately sends
(archastro-elixir v0.3.2 decode audit). Scalar nullables fell through
to the check and worked.

Peel nullable off first, before any shape branch, so every shape wraps
in {kind: nullable} through the one existing mechanism. Covers REST and
channel emission alike — both flow through jsonSchemaToTypeRef.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@rob-archastro

Copy link
Copy Markdown
Contributor Author

Review — round 1 (head 28b1b62)

Problem and author intent: SDK generator silently drops nullable: true on composite schema shapes (allOf+$ref, oneOf union, enum), so generated Elixir decoders crash on nulls the platform legitimately sends — the bug that froze remote-eval slice 4 at its decode-audit gate. Fix reorders jsonSchemaToTypeRef to peel nullable before any shape branch, through the existing wrap mechanism.

Verdict: no findings — meets bar. Every claim in the PR body verified independently at source:

  • Root cause confirmed in the diff: the $ref/enum/oneOf/allOf branches all returned before the nullable check; the fix moves the identical peel-and-recurse block to the top and removes the old one (no double-wrap; recursion strips the flag so no loop).
  • Walker safety: collectRefsFromTypeRef and the Elixir backend's rewriteType both recurse through nullable, and every backend emitter has a nullable case — the newly-nested composite refs can't lose dependency tracking or emission.
  • Tests re-run locally at head: 12 files / 404 tests green; frontend tests cover all three composite shapes with non-nullable controls; backend tests assert the emitted descriptors incl. a channel push payload with a nullable ref.
  • Regeneration re-executed independently (built generator at head, --lang elixir vs specs/platform-openapi.json, diffed against a regen built at parent commit 23131e5): the before→after table reproduces exactly for Message.acl / Message.user / Message.agent_mode / StatusPing.user; 8 files differ incl. both channel modules; every added line is a {:nullable, …} wrap, its | nil typespec counterpart, or a content-hash header — no other change class exists in the diff.
  • No blanket wrapping: fields whose spec declares no nullable (e.g. AgentRoutine.acl, a bare sibling-$ref) stay byte-identical; spot-checked spec declarations line up 1:1 with wrap decisions.

Note for the release chain: this frontend is shared by all language backends, so the next TS/Python/Go regens will also (correctly) gain nullable on these fields — widening-only there too, but worth knowing when those repos regen.

Ready for merge from my side. After merge: publish sdk-generator to npm (release.yml, package=sdk-generator, patch) before the archastro-elixir regen — regen installs the generator from npm @latest, so merge alone is insufficient.

@rob-archastro
rob-archastro merged commit 705ab61 into main Aug 16, 2026
3 checks passed
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