fix(sdk): handle nullable unions and browser headers by tothandras · Pull Request #4722 · openmeterio/openmeter

tothandras · 2026-07-16T09:30:00Z

Summary by CodeRabbit

Bug Fixes
- Improved generated schema handling for optional and nullable fields, including events, pagination metadata, costs, and update requests.
- Preserved nullable discriminated-union behavior during schema generation and wire-format conversion.
- Prevented automatic User-Agent headers from being added in browser and worker environments.
- Continued honoring caller-provided User-Agent values across supported environments.

Greptile Summary

This PR improves nullable-union emission and avoids browser telemetry headers. The main changes are:

Emit anonymous null unions with Zod’s .nullable() wrapper.
Add nullable-union and wire round-trip tests.
Restrict the default User-Agent to detected Node runtimes.
Regenerate the JavaScript SDK and conformance tests.

Confidence Score: 5/5

The changed flow looks mergeable after hardening browser runtime detection.

Nullable-union emission preserves the non-null variants and works with the wire mapper.
Browser and worker requests omit the default header when process is absent.
A browser process polyfill can still cause an unnecessary CORS preflight.

api/spec/packages/typespec-typescript/templates/runtime/core.ts

Important Files Changed

Filename	Overview
api/spec/packages/typespec-typescript/src/zodBaseSchema.tsx	Normalizes anonymous unions containing null into nullable Zod schemas while retaining every non-null variant.
api/spec/packages/typespec-typescript/templates/runtime/core.ts	Adds Node-only default User-Agent detection, with a remaining false-positive path for browser process polyfills.
api/spec/packages/typespec-typescript/test/nullable-union.test.ts	Covers nullable references, multi-variant unions, unchanged union forms, and wire conversion.
api/spec/packages/aip-client-javascript/src/models/schemas.ts	Regenerates public and wire schemas to use nullable wrappers for affected fields.
api/spec/packages/typespec-typescript/templates/tests/client.spec.ts	Covers browsers and workers where process is absent, but not browsers with a Node-compatible process polyfill.

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
api/spec/packages/typespec-typescript/templates/runtime/core.ts:34-39
**Polyfilled Process Triggers Browser Header**

A browser bundle that provides a Node-compatible `process.versions.node` value passes this check even though requests are still subject to CORS. The SDK then adds `User-Agent`, causing an otherwise simple cross-origin request to preflight and potentially fail; the browser tests only cover an undefined `process`, not this common polyfilled state.

_{Reviews (1): Last reviewed commit: "fix(sdk): handle nullable unions and bro..." | Re-trigger Greptile}

Greptile also left 1 inline comment on this PR.

Context used (3)

Context used - CLAUDE.md (source)
Context used - AGENTS.md (source)
Context used - api/spec/AGENTS.md (source)

coderabbitai · 2026-07-16T09:30:24Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 52baca76-ccc6-4d88-bce0-dbe7a611661f

📥 Commits

Reviewing files that changed from the base of the PR and between 106e6d4 and a71e096.

📒 Files selected for processing (7)

api/spec/packages/aip-client-javascript/src/core.ts
api/spec/packages/aip-client-javascript/src/models/schemas.ts
api/spec/packages/aip-client-javascript/tests/client.spec.ts
api/spec/packages/typespec-typescript/src/zodBaseSchema.tsx
api/spec/packages/typespec-typescript/templates/runtime/core.ts
api/spec/packages/typespec-typescript/templates/tests/client.spec.ts
api/spec/packages/typespec-typescript/test/nullable-union.test.ts

📝 Walkthrough

Walkthrough

The TypeScript/Zod emitter now models nullable unions with .nullable(), generated schemas update corresponding nullable fields, and SDK clients only add default User-Agent headers in Node-like environments. Tests cover schema emission, wire round trips, and browser/worker telemetry behavior.

Changes

SDK schema and runtime behavior

Layer / File(s)	Summary
Nullable union emission and coverage `api/spec/packages/typespec-typescript/src/zodBaseSchema.tsx`, `api/spec/packages/typespec-typescript/test/nullable-union.test.ts`	Expression unions containing `null` emit nullable schemas while preserving non-null variants; tests cover generated output and wire round trips.
Generated nullable model schemas `api/spec/packages/aip-client-javascript/src/models/schemas.ts`	Cursor, CloudEvents, feature cost, and feature request models use nullable Zod modifiers in public and wire schemas.
Node-only User-Agent injection `api/spec/packages/aip-client-javascript/src/core.ts`, `api/spec/packages/typespec-typescript/templates/runtime/core.ts`, `api/spec/packages/aip-client-javascript/tests/client.spec.ts`, `api/spec/packages/typespec-typescript/templates/tests/client.spec.ts`	Default `User-Agent` injection now requires a Node-like runtime and an absent header; tests cover browser, worker, and caller-provided header behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

openmeterio/openmeter#4687: Modifies the SDK telemetry User-Agent injection behavior and tests.
openmeterio/openmeter#4719: Updates cursor pagination fields toward required nullable schema representations.

Suggested reviewers: turip, borbelyr-kong

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main changes: nullable-union handling and browser/Node User-Agent header behavior.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/javascript-sdk-schema

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

greptile-apps · 2026-07-16T09:32:39Z

+            if (
+              typeof process !== 'undefined' &&
+              process.versions?.node != null &&
+              !request.headers.has('User-Agent')
+            ) {
              request.headers.set('User-Agent', `openmeter-node/${SDK_VERSION}`)


Polyfilled Process Triggers Browser Header

A browser bundle that provides a Node-compatible process.versions.node value passes this check even though requests are still subject to CORS. The SDK then adds User-Agent, causing an otherwise simple cross-origin request to preflight and potentially fail; the browser tests only cover an undefined process, not this common polyfilled state.

Context Used: api/spec/AGENTS.md (source)

Prompt To Fix With AI

This is a comment left during a code review. Path: api/spec/packages/typespec-typescript/templates/runtime/core.ts Line: 34-39 Comment: **Polyfilled Process Triggers Browser Header** A browser bundle that provides a Node-compatible `process.versions.node` value passes this check even though requests are still subject to CORS. The SDK then adds `User-Agent`, causing an otherwise simple cross-origin request to preflight and potentially fail; the browser tests only cover an undefined `process`, not this common polyfilled state. **Context Used:** api/spec/AGENTS.md ([source](https://app.greptile.com/openmeter/github/openmeterio/openmeter/-/custom-context?memory=28ba6068-00f9-4629-9b78-8e49cc802858)) How can I resolve this? If you propose a fix, please make it concise.

fix(sdk): handle nullable unions and browser headers

a71e096

tothandras requested a review from a team as a code owner July 16, 2026 09:30

tothandras added the release-note/bug-fix Release note: Bug Fixes label Jul 16, 2026

tothandras enabled auto-merge (squash) July 16, 2026 09:31

greptile-apps Bot reviewed Jul 16, 2026

View reviewed changes

chrisgacsal approved these changes Jul 16, 2026

View reviewed changes

borosr approved these changes Jul 16, 2026

View reviewed changes

tothandras merged commit 0bf476a into main Jul 16, 2026
45 of 48 checks passed

tothandras deleted the fix/javascript-sdk-schema branch July 16, 2026 09:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(sdk): handle nullable unions and browser headers#4722

fix(sdk): handle nullable unions and browser headers#4722
tothandras merged 1 commit into
mainfrom
fix/javascript-sdk-schema

tothandras commented Jul 16, 2026 •

edited by greptile-apps Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 16, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

greptile-apps Bot Jul 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

tothandras commented Jul 16, 2026 • edited by greptile-apps Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Context used (3)

Uh oh!

coderabbitai Bot commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

greptile-apps Bot Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tothandras commented Jul 16, 2026 •

edited by greptile-apps Bot

Loading

coderabbitai Bot commented Jul 16, 2026 •

edited

Loading