Skip to content

fix: Preserve explicit nil in request bodies as JSON null - #522

Merged
gjtorikian merged 2 commits into
mainfrom
devin/nullable-clearing-runtime
Jul 21, 2026
Merged

fix: Preserve explicit nil in request bodies as JSON null#522
gjtorikian merged 2 commits into
mainfrom
devin/nullable-clearing-runtime

Conversation

@devin-ai-integration

Copy link
Copy Markdown
Contributor

Description

Hand-maintained runtime half of the cross-SDK nullable-clearing feature. Touches only the @oagen-ignore-file lib/workos/base_client.rb (plus a test), so it won't be auto-closed by workos-sdk-automation.

base_client previously ran body.compact.to_json on every POST/PUT/PATCH/DELETE body, which strips every nil — so an explicit nil on a nullable field (e.g. external_id) could never reach the wire as JSON null. This drops the .compact:

# before
req.body = body.nil? ? "" : body.compact.to_json
# after
req.body = body.nil? ? "" : body.to_json

Omission ("leave unchanged") is not affected: it's handled upstream in the generated methods, which build the body with .compact on the non-nullable literal and only conditionally assign nullable fields via a WorkOS::OMIT sentinel default. Those generated changes come from regeneration against workos/oagen-emitters#189; this PR is the runtime prerequisite. On its own it's a safe no-op against today's generated code (which already pre-compacts its bodies), and it enables the feature once regeneration lands.

Verified no existing hand-maintained caller relied on base_client to strip its nils (full suite green, 694 runs).

Testing

Added test_base_client cases asserting POST/PUT serialize an explicit nil value as JSON null.

Documentation

Does this require changes to the WorkOS Docs? E.g. the API Reference or code snippets need updates.

[ ] Yes

If yes, link a related docs PR and add a docs maintainer as a reviewer. Their approval is required.

Link to Devin session: https://app.devin.ai/sessions/127c630f46c54bc0be571814c75047e8

base_client no longer strips nil from mutating request bodies, so an
explicit nil survives as JSON null. This is the hand-maintained runtime
half of the nullable-clearing feature; the generated methods that pass
nullable fields through arrive via regeneration from oagen-emitters#189.
@devin-ai-integration
devin-ai-integration Bot requested review from a team as code owners July 20, 2026 22:05
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author
Original prompt from heather

SYSTEM:
=== BEGIN THREAD HISTORY (in #dse-pre-triage) ===
<most_recent_message>
Heather Faerber (U08CUNLUBT9): @Devin can you update the rubygem based on this feedback?

&gt; Is there a way via the API or rubygem to clear the external_id for an organization (or user)?
&gt;
&gt; In the web UI, we can just delete the field. (It's under Settings | Organization details | Edit details | External ID.)
&gt;
&gt; But when I tried to set it with the rubygem, it doesn't work:
&gt; • nil gets stripped. The request succeeds, but the value doesn't change.
&gt; • "" returns Validation failed
&gt; • Doing the request directly also gets 200, but the value also doesn't change.
&gt;
&gt; ```&gt; org = WorkOS.client.organizations.create_organization(name: "ExtId Clear Test A", external_id: "ext-clear-test-a")
&gt; =&gt; #&lt;WorkOS::Organization object="organization" id="org_01KXT2M2RQYV33VRKJC7CKKNMY" name="ExtId Clear Test A" domains=[] metadata={} external_id="ext-clear-test-a" created_at="2026-07-18T07:39:00.241Z" updated_at="2026-07-18T07:39:00.241Z" allow_profiles_outside_organization=false&gt;
&gt;
&gt; &gt; WorkOS.client.organizations.update_organization(id: org.id, external_id: "")
&gt; (artemis):48:in '&lt;main&gt;': Validation failed (WorkOS::UnprocessableEntityError)
&gt;
&gt; &gt; WorkOS.client.organizations.update_organization(id: org.id, external_id: nil)
&gt; =&gt; `#`&lt;WorkOS::Organization object="organization" id="org_01KXT2M2RQYV33VRKJC7CKKNMY" name="ExtId Clear Test A" domains=[] metadata={} external_id="ext-clear-test-a" created_at="2026-07-18T07:39:00.241Z" updated_at="2026-07-18T07:39:18.505Z" allow_profiles_outside_organization=false&gt;
&gt;
&gt; &gt; WorkOS.client.organizations.get_organization(id: org.id).external_id.inspect
&gt; =&gt; ""ext-clear-test-a""
&gt;
&gt; &gt; WorkOS.client.request(method: :put, path: "/organizations/`#`{org.id}", body: { "external_id" =&gt; nil })
&gt; =&gt; `#`&lt;Net::HTTPOK 200 OK readbody=true&gt;
&gt;
&gt; &gt; Wo... (3272 chars truncated...)

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@devin-ai-integration devin-ai-integration Bot changed the title Preserve explicit nil in request bodies as JSON null fix: Preserve explicit nil in request bodies as JSON null Jul 20, 2026
@greptile-apps

greptile-apps Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR removes .compact from request body serialization in all four mutating HTTP methods (POST, PUT, PATCH, DELETE) in BaseClient, so that an explicitly-supplied nil value reaches the wire as JSON null instead of being silently dropped. The change is the runtime prerequisite for the upcoming nullable-clearing feature driven by oagen regeneration.

  • lib/workos/base_client.rb: Four one-line changes — body.compact.to_jsonbody.to_json — applied symmetrically to post_request, put_request, patch_request, and delete_request. Omission of optional fields is still compacted upstream in generated callers, so this is a safe no-op against today's generated code.
  • test/workos/test_base_client.rb: Four new tests, one per HTTP verb, each asserting that {"external_id" => nil} round-trips through JSON.parse as {"external_id" => nil}, giving symmetric coverage across all mutating verbs.

Confidence Score: 5/5

Safe to merge — the change is a four-line removal of .compact applied symmetrically across all mutating verbs, with no regressions against existing callers and full test coverage for the new behavior.

The removal of .compact is intentional and well-scoped: the two remaining .compact calls in append_query and the log formatter are unrelated and deliberately untouched. Generated callers already pre-compact their bodies, so today's behavior is unchanged. The four new tests provide symmetric coverage across POST, PUT, PATCH, and DELETE, and the full suite (694 runs) passed.

No files require special attention.

Important Files Changed

Filename Overview
lib/workos/base_client.rb Drops .compact from body serialization in all four mutating methods; change is minimal, symmetric, and correct — unrelated .compact calls in append_query and log are intentionally untouched.
test/workos/test_base_client.rb Adds four new nil-serialization tests, one per HTTP verb (POST, PUT, PATCH, DELETE), providing symmetric coverage against accidental reintroduction of .compact.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Caller as Generated/Hand-Maintained Caller
    participant BC as BaseClient
    participant Wire as HTTP Wire

    Note over Caller,BC: Before this PR
    Caller->>BC: "post_request(body: {external_id: nil, name: Acme})"
    BC->>BC: "body.compact → {name: Acme}"
    BC->>Wire: "{name:Acme} (nil silently dropped)"

    Note over Caller,BC: After this PR
    Caller->>BC: "post_request(body: {external_id: nil, name: Acme})"
    BC->>BC: body.to_json (no compact)
    BC->>Wire: "{external_id:null, name:Acme} (nil preserved as JSON null)"

    Note over Caller,BC: Omission path — unchanged
    Caller->>Caller: Generated method compacts non-nullable hash
    Caller->>BC: "post_request(body: {name: Acme})"
    BC->>Wire: "{name:Acme}"
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Caller as Generated/Hand-Maintained Caller
    participant BC as BaseClient
    participant Wire as HTTP Wire

    Note over Caller,BC: Before this PR
    Caller->>BC: "post_request(body: {external_id: nil, name: Acme})"
    BC->>BC: "body.compact → {name: Acme}"
    BC->>Wire: "{name:Acme} (nil silently dropped)"

    Note over Caller,BC: After this PR
    Caller->>BC: "post_request(body: {external_id: nil, name: Acme})"
    BC->>BC: body.to_json (no compact)
    BC->>Wire: "{external_id:null, name:Acme} (nil preserved as JSON null)"

    Note over Caller,BC: Omission path — unchanged
    Caller->>Caller: Generated method compacts non-nullable hash
    Caller->>BC: "post_request(body: {name: Acme})"
    BC->>Wire: "{name:Acme}"
Loading

Reviews (2): Last reviewed commit: "Add nil-serialization tests for PATCH an..." | Re-trigger Greptile

Comment thread test/workos/test_base_client.rb
@gjtorikian
gjtorikian merged commit 5b06e32 into main Jul 21, 2026
8 checks passed
@gjtorikian
gjtorikian deleted the devin/nullable-clearing-runtime branch July 21, 2026 03:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants