fix(typescript): OAuth client-credentials codegen (strict-mode canCreate + README credential shape) - #17262
fix(typescript): OAuth client-credentials codegen (strict-mode canCreate + README credential shape)#17262cadesark wants to merge 2 commits into
Conversation
…ate + README credential shape)
Fix two OAuth client-credentials codegen bugs that surface in the multi-scheme
(wrapper) case.
Bug 1a: When OAuth is combined with another auth scheme (auth: any) and OAuth has
no client-id/secret env-var fallback (so the nested credentials are required), the
generated BaseClient.ts failed `tsc --strict` with TS2322 — OAuthAuthProvider.canCreate
was not assignable to AnyAuthProvider's InstantiatableAuthProvider contract
`(opts: NormalizedClientOptions) => boolean`. The parameter type is now widened in the
wrapper case (using the same Supplier | EndpointSupplier type the emitted options use)
so the assignment type-checks. Runtime behavior is unchanged.
Bug 1b: The generated README OAuth quickstart emitted top-level clientId/clientSecret
(and token override) while the auth provider reads them nested under the wrapper
property (e.g. `oauth: { ... }`) whenever the auth requirement is `any` or
endpoint-scoped, so the documented usage never authenticated. buildAuthenticationDescription
now nests credentials under the wrapper in that case, mirroring the auth-provider and
wire-test generators. Wire tests were already correct on main (TestGenerator's
requiresNestedAuth), so only the README needed fixing.
Single-scheme OAuth output (top-level credentials) is byte-identical.
Co-Authored-By: Claude <noreply@anthropic.com>
![nitpickybot[bot]](https://avatars.githubusercontent.com/in/2901896?s=60&v=4){kind=small}
There was a problem hiding this comment.
AI Review Summary
The PR fixes two real OAuth codegen bugs in the multi-scheme wrapper case: a strict-mode contravariance failure in canCreate and README credentials documented at the wrong nesting level. The main concern is inconsistency in how the wrapper-detection logic is derived — the generator uses keepIfWrapper as a fragile proxy in one place and re-derives the wrapper name via toCamelCase in another, while a canonical getWrapperPropertyName reportedly already exists.
- 🟡 2 warning(s)
- 🔵 1 suggestion(s)
| * body (which only reads `clientId`/`clientSecret`) is unchanged. | ||
| */ | ||
| private getCanCreateParameterType(context: FileContext): string { | ||
| if (this.keepIfWrapper("x") === "") { |
There was a problem hiding this comment.
🟡 warning
this.keepIfWrapper("x") === "" is an obscure way to detect the single-scheme case — you're calling a string-mangling helper on a dummy argument and comparing the result to empty string. This is fragile: if keepIfWrapper's implementation ever changes (e.g. returns a differently-formatted prefix), this silently flips behavior. Prefer the same explicit requiresNestedAuth-style check the README generator uses (AuthSchemesRequirement._visit), or a dedicated boolean helper. Add a comment at minimum explaining the magic string.
| // Wrapper case: unify ClientCredentials + TokenOverride into an all-optional | ||
| // wrapper shape so the parameter is a supertype of the collapsed | ||
| // NormalizedClientOptions wrapper property. | ||
| return `Partial<BaseClientOptions> & { [WRAPPER_PROPERTY]?: { [CLIENT_ID_PARAM]?: ${supplierType}; [CLIENT_SECRET_PARAM]?: ${supplierType}; [TOKEN_PARAM]?: ${supplierType} } }`; |
There was a problem hiding this comment.
🔵 suggestion
This template string references WRAPPER_PROPERTY, CLIENT_ID_PARAM, CLIENT_SECRET_PARAM, and TOKEN_PARAM as literal identifiers emitted into the generated file. Confirm these consts are always in scope in every emitted OAuthAuthProvider.ts (they are in the single-scheme case too, but that branch returns early). The seed fixtures show them present, so this is fine — just flagging that the type now depends on those module-level consts being emitted unconditionally in the wrapper case.
| // Mirror OAuthAuthProviderGenerator.getWrapperPropertyName (the source of | ||
| // truth for WRAPPER_PROPERTY) so the README nests under the exact key the | ||
| // auth provider reads. | ||
| return toCamelCase(oauthScheme.key); |
There was a problem hiding this comment.
🟡 warning
The comment claims toCamelCase(oauthScheme.key) mirrors OAuthAuthProviderGenerator.getWrapperPropertyName (the source of truth). Duplicating this derivation risks drift — if the generator's wrapper-key logic ever changes (casing rules, keyword escaping, smart-casing), the README will silently document the wrong key. Consider extracting a single shared helper so both call sites use the identical derivation rather than re-implementing toCamelCase(key) here.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
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 |
- Replace fragile `keepIfWrapper("x") === ""` single-scheme detection in
OAuthAuthProviderGenerator.getCanCreateParameterType with an explicit
`shouldUseWrapper` boolean field (identical behavior).
- Extract shared getOAuthWrapperPropertyName helper into
@fern-typescript/commons and call it from both
OAuthAuthProviderGenerator.getWrapperPropertyName and
ReadmeSnippetBuilder so the wrapper-key derivation cannot drift.
- Document that the widened canCreate wrapper type depends on the
WRAPPER_PROPERTY/CLIENT_ID_PARAM/CLIENT_SECRET_PARAM/TOKEN_PARAM consts
being emitted in the wrapper branch.
Pure internal cleanup: generated output is byte-identical for both the
wrapper (any-auth) and single-scheme (oauth-client-credentials) fixtures.
Co-Authored-By: Claude <noreply@anthropic.com>
1 participant
Description
Fixes two OAuth client-credentials codegen bugs in the TypeScript SDK generator, both of which only affect the multi-scheme / wrapper case (OAuth combined with another scheme via
auth: any, or endpoint-scoped auth). Single-scheme OAuth output is byte-identical. Reported by a customer (perkss) againstfern-typescript-sdk 3.82.3; both bugs still reproduced on currentmain.Changes Made
Bug 1a —
tsc --strictfailure on multi-scheme OAuthoauth.clientId/clientSecretare required), the generatedBaseClient.tsfailedtsc --strictwithTS2322:OAuthAuthProvider.canCreatewas not assignable toAnyAuthProvider'sInstantiatableAuthProvidercontract(opts: NormalizedClientOptions) => boolean.NormalizedClientOptionscollapses the wrapper property to the token-override shape ({ token? }) viaAtLeastOneOf/UnionToIntersection, so under function-parameter contravariance the narrowPartial<ClientCredentials & BaseClientOptions>parameter was rejected.OAuthAuthProviderGeneratornow widens thecanCreateparameter in the wrapper case to an all-optional wrapper shape ({ [WRAPPER_PROPERTY]?: { clientId?; clientSecret?; token? } }), using the sameSupplier | EndpointSuppliertype the emitted options use. Runtime body is unchanged.Bug 1b — README documented un-authenticating credentials
The generated README OAuth quickstart emitted top-level
clientId/clientSecret(and token override), but in the wrapper case the auth provider only reads them nested under the wrapper property (e.g.oauth: { ... }) — so the documented usage never authenticated.Fix:
ReadmeSnippetBuilder.buildAuthenticationDescription()now nests credentials under the wrapper when the auth requirement isanyor endpoint-scoped, mirroring the samerequiresNestedAuthsignal used by the auth-provider and wire-test generators. The wrapper key is derived viatoCamelCase(oauthScheme.key)— the exact source of truth forWRAPPER_PROPERTY.The wire-test /
mockAuthhalf of 1b was already correct onmain(TestGenerator.getAuthClientOptions()already branches onrequiresNestedAuth), so only the README needed fixing.Updated README.md generator
Testing
AuthProviders.test.ts: new type-diagnostic suite that reproduces the 1a contravariance failure with the old narrow parameter and proves the widened parameter satisfies the contract.ReadmeSnippetBuilder.test.ts: new test asserting nestedoauth: { ... }forANYrequirement, plus an assertion that single-scheme (ALL) stays top-level.tsc 5.9.3 --strict→ exactTS2322); confirmed fix type-checks.any-auth,endpoint-security-auth, and single-schemeoauth-client-credentials*seed fixtures via local generator. All pass the validator's strict typecheck. Only the 4 wrapper fixtures'README.md+OAuthAuthProvider.tschanged; every single-scheme fixture is byte-identical (metadata-only churn reverted).fixentry undergenerators/typescript/sdk/changes/unreleased/.Generated with Claude Code