fix(csharp): make additional OAuth token-request properties optional constructor params#16655
Conversation
…constructor params
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
| name, | ||
| docs: `The ${name} for OAuth authentication.`, | ||
| isOptional, | ||
| isOptional: true, |
There was a problem hiding this comment.
🔴 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.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
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.
SDK Generation Benchmark ResultsComparing PR branch against median of 5 nightly run(s) on Full benchmark table (click to expand)
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 |
Description
Fixes the
oauth-client-credentials-customC# SDK seed fixture, which was inallowedFailures.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 mappedscopes/scp),RootClientGenerator.getOAuthAdditionalConstructorParamsadded them as required constructor parameters:The dynamic-snippets generator only emits
clientId/clientSecret(the dynamic IROAuthtype carries nothing else), so the generatedSnippets/Example*.csfailed 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 tonull— 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 justclientId/clientSecret, callers can still pass the extras, and snippets compile.Changes Made
generators/csharp/sdk/src/root-client/RootClientGenerator.ts:getOAuthAdditionalConstructorParamsnow marks additional OAuth token-request params (isOptional: true); dropped the now-unusedisOptionalargument and updated both callers.seed/csharp-sdk/oauth-client-credentials-custom/(only the root client signature changed; the snippet was already only passing client id/secret).oauth-client-credentials-customfromallowedFailuresinseed/csharp-sdk/seed.yml.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).oauth-client-credentials,oauth-client-credentials-mandatory-auth) — no.cschanges, confirming no regression.pnpm run check(biome) passes.Link to Devin session: https://app.devin.ai/sessions/a994c7825e1c4e44a3c8beddb59cbda4
Requested by: @iamnamananand996