Skip to content

fix(csharp): make additional OAuth token-request properties optional constructor params#16655

Open
devin-ai-integration[bot] wants to merge 2 commits into
mainfrom
devin/1782160363-csharp-oauth-custom-token-properties
Open

fix(csharp): make additional OAuth token-request properties optional constructor params#16655
devin-ai-integration[bot] wants to merge 2 commits into
mainfrom
devin/1782160363-csharp-oauth-custom-token-properties

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Description

Fixes the oauth-client-credentials-custom C# SDK seed fixture, which was in allowedFailures.

Root cause: For an OAuth client-credentials scheme whose token endpoint has required, non-literal request properties beyond client id/secret (here entity_id, plus the mapped scopes/scp), RootClientGenerator.getOAuthAdditionalConstructorParams added them as required constructor parameters:

public SeedOauthClientCredentialsClient(
    string clientId, string clientSecret,
    string entityId, string scp,            // required
    ClientOptions? clientOptions = null)

The dynamic-snippets generator only emits clientId/clientSecret (the dynamic IR OAuth type carries nothing else), so the generated Snippets/Example*.cs failed to compile (CS7036: no argument given for required parameter 'entityId'), which broke the seed build step. The Java SDK exposes these same properties as optional builder setters defaulting to null — the C# generator had diverged by making them required.

Fix: emit these additional OAuth params as optional (string? = null), matching Java. The root client can now be constructed with just clientId/clientSecret, callers can still pass the extras, and snippets compile.

public SeedOauthClientCredentialsClient(
    string clientId, string clientSecret,
    string? entityId = null, string? scp = null,
    ClientOptions? clientOptions = null)

Changes Made

  • generators/csharp/sdk/src/root-client/RootClientGenerator.ts: getOAuthAdditionalConstructorParams now marks additional OAuth token-request params (isOptional: true); dropped the now-unused isOptional argument and updated both callers.
  • Regenerated seed/csharp-sdk/oauth-client-credentials-custom/ (only the root client signature changed; the snippet was already only passing client id/secret).
  • Removed oauth-client-credentials-custom from allowedFailures in seed/csharp-sdk/seed.yml.
  • Added changelog entry under generators/csharp/sdk/changes/unreleased/.

Testing

  • node packages/seed/dist/cli.cjs test --generator csharp-sdk --fixture oauth-client-credentials-custom --local → build + tests pass (previously failed the Snippets build).
  • Regenerated the other OAuth fixtures (oauth-client-credentials, oauth-client-credentials-mandatory-auth) — no .cs changes, confirming no regression.
  • pnpm run check (biome) passes.

Link to Devin session: https://app.devin.ai/sessions/a994c7825e1c4e44a3c8beddb59cbda4
Requested by: @iamnamananand996


Open in Devin Review

@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 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

Open in Devin Review

name,
docs: `The ${name} for OAuth authentication.`,
isOptional,
isOptional: true,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 OAuthTokenProvider constructor not updated to accept nullable params, causing type mismatch with root client

The root client now generates additional OAuth parameters as nullable optional (string? entityId = null, string? scp = null) at RootClientGenerator.ts:1069, but the OauthTokenProviderGenerator was not updated and still generates non-nullable parameters (string EntityId, string Scp) in the OAuthTokenProvider constructor (OauthTokenProviderGenerator.ts:161-163). The root client passes these nullable values directly to OAuthTokenProvider (lines 549-558), resulting in a string?string type mismatch (CS8604 warning). At runtime, if a user constructs the client without providing these values, null flows into the non-nullable fields (_entityId, _scp) and is then used in the token request body, likely causing a server-side error or NullReferenceException.

Generated code showing the mismatch

Root client passes nullable:

var tokenProvider = new OAuthTokenProvider(
    clientId, clientSecret,
    entityId,  // string? (nullable)
    scp,       // string? (nullable)
    new AuthClient(new RawClient(clientOptions))
);

But OAuthTokenProvider expects non-nullable:

public OAuthTokenProvider(
    string clientId, string clientSecret,
    string EntityId,  // non-nullable
    string Scp,       // non-nullable
    AuthClient client)
Prompt for agents
The root client now marks additional OAuth constructor params (custom properties, scopes) as isOptional: true, making them nullable in the generated C# code. However, the OauthTokenProviderGenerator (generators/csharp/sdk/src/oauth/OauthTokenProviderGenerator.ts) was not updated to match. Its constructor still declares these parameters as non-nullable (line 163: ctor.addParameter with field.type which is non-optional), and its backing fields are also non-nullable (line 128: type: typeRef without .asOptional()).

The OauthTokenProviderGenerator needs to be updated so that the additional request fields (customProperties and scopes) use nullable types in both the fields and constructor parameters. Specifically:
1. At line 128, change type: typeRef to type: typeRef.asOptional() for the field declaration
2. At line 163, the parameter type will inherit from field.type, so it will become optional automatically
3. The fields at lines 123-130 and 139-146 should also become nullable

Alternatively, the root client could be updated to pass a non-null fallback (e.g. empty string or throw) when these values are null, but making the OAuthTokenProvider accept nullable values is the cleaner approach that matches the stated intent of making these parameters truly optional.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — fixed in fc217c1. OauthTokenProviderGenerator now generates these additional fields/params as nullable (string? _entityId, string? _scp), so the root client passes string?string? with no CS8604 mismatch. The regenerated project now builds with 0 warnings, 0 errors.

On the runtime note: emitting null for an unset value is the intended, Java-matching behavior — the Java SDK exposes these same token-request properties as optional builder setters defaulting to null. They have to be optional on the client because the dynamic-snippets generator can only supply clientId/clientSecret (the dynamic IR OAuth type carries nothing else), so a required-param constructor made the generated snippets uncompilable. A user whose token endpoint needs entity_id still passes it explicitly.

@github-actions

github-actions Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

SDK Generation Benchmark Results

Comparing PR branch against median of 5 nightly run(s) on main (latest: 2026-06-22T05:47:30Z).

Full benchmark table (click to expand)
Generator Spec main (generator) main (E2E) PR (generator) Delta
csharp-sdk square 67s (n=5) 108s (n=5) 66s -1s (-1.5%)

main (generator): generator-only time via --skip-scripts (includes Docker image build, container startup, IR parsing, and code generation — this is the same Docker-based flow customers use via fern generate). main (E2E): full customer-observable time including build/test scripts (nightly baseline, informational). Delta is computed against generator-only baseline.
⚠️ = generation exited with a non-zero exit code (timing may not reflect a successful run).
Baseline from nightly runs on main (latest: 2026-06-22T05:47:30Z). Trigger benchmark-baseline to refresh.
Last updated: 2026-06-22 20:49 UTC

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