feat(java-sdk): remove omitted basic auth fields from generated SDK API#14408
feat(java-sdk): remove omitted basic auth fields from generated SDK API#14408Swimburger wants to merge 41 commits intomainfrom
Conversation
… configured in IR Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🤖 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:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…ted flag Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| // Build per-field checks: omittable fields are always satisfied, required fields must be present | ||
| String usernameCheck; | ||
| if (usernameOmitted) { | ||
| usernameCheck = "true"; | ||
| } else { | ||
| StringBuilder uc = new StringBuilder("(usernameSupplier != null"); | ||
| if (usernameEnvVar != null) { | ||
| uc.append(" || System.getenv(\"").append(usernameEnvVar).append("\") != null"); | ||
| } | ||
| uc.append(")"); | ||
| usernameCheck = uc.toString(); | ||
| } | ||
| condition.append(") && (passwordSupplier != null"); | ||
| if (passwordEnvVar != null) { | ||
| condition.append(" || System.getenv(\"").append(passwordEnvVar).append("\") != null"); | ||
|
|
||
| String passwordCheck; | ||
| if (passwordOmitted) { | ||
| passwordCheck = "true"; | ||
| } else { | ||
| StringBuilder pc = new StringBuilder("(passwordSupplier != null"); | ||
| if (passwordEnvVar != null) { | ||
| pc.append(" || System.getenv(\"").append(passwordEnvVar).append("\") != null"); | ||
| } | ||
| pc.append(")"); | ||
| passwordCheck = pc.toString(); | ||
| } | ||
| condition.append(")"); | ||
|
|
||
| builder.addStatement("return " + condition); | ||
| builder.addStatement("return " + usernameCheck + " && " + passwordCheck); |
There was a problem hiding this comment.
Logic inconsistency between canCreate() and getAuthHeaders(). When both username and password are omittable, canCreate() returns true even when no credentials are provided (since usernameCheck="true" and passwordCheck="true" results in true && true). However, getAuthHeaders() throws a RuntimeException if both are null (line 154). This causes canCreate() to incorrectly return true, then later fail at runtime when getAuthHeaders() is called.
When both are omittable, canCreate() should use || logic to ensure at least one credential is available:
if (usernameOmitted && passwordOmitted) {
// Both optional - need at least one
builder.addStatement("return (" + usernameCheck + ") || (" + passwordCheck + ")");
} else {
builder.addStatement("return " + usernameCheck + " && " + passwordCheck);
}Spotted by Graphite
Is this helpful? React 👍 or 👎 to let us know.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…er-field omit checks for build validation and setAuthentication Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…optional password Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…c-auth-optional-java-sdk
…r, use empty string internally Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
… omitted password Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
devin-ai-integration
bot
changed the title
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…h provider setup Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…OINT_SECURITY path Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…e omitted Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…ttempt 2) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…non-mandatory Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…elog and code comment Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…enerator Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
| const authRecord = auth as unknown as Record<string, unknown>; | ||
| const usernameOmitted = authRecord.usernameOmit === true; | ||
| const passwordOmitted = authRecord.passwordOmit === true; |
There was a problem hiding this comment.
🔴 CLAUDE.md violation: as unknown as Record<string, unknown> used instead of updating dynamic IR types
CLAUDE.md explicitly states: "Never use as any or as unknown as X. These are escape hatches that bypass the type system entirely. If the types don't line up, fix the types." The code casts auth to Record<string, unknown> to access usernameOmit/passwordOmit properties that exist on the main IR's BasicAuthScheme (packages/ir-sdk/src/sdk/api/resources/auth/types/BasicAuthScheme.ts:10,15) but are missing from the dynamic IR's BasicAuth type (packages/ir-sdk/src/sdk/api/resources/dynamic/resources/auth/types/BasicAuth.ts). The proper fix is to add usernameOmit and passwordOmit to the dynamic IR BasicAuth type definition and regenerate, rather than using a type system escape hatch.
Prompt for agents
The dynamic IR BasicAuth type at packages/ir-sdk/src/sdk/api/resources/dynamic/resources/auth/types/BasicAuth.ts is missing the usernameOmit and passwordOmit fields that already exist on the main IR BasicAuthScheme. The fix involves:
1. Update the dynamic IR definition (likely in packages/ir-sdk/fern/apis/ir-types-latest/definition/ under the dynamic auth types) to add usernameOmit: optional<boolean> and passwordOmit: optional<boolean> to the dynamic BasicAuth type.
2. Regenerate the IR SDK types so the TypeScript interface includes these fields.
3. Replace the as unknown as Record<string, unknown> cast with direct property access on the properly typed auth object, e.g. auth.usernameOmit === true and auth.passwordOmit === true.
This eliminates the type safety escape hatch while making the dynamic IR consistent with the main IR.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
The as unknown as Record<string, unknown> cast is intentional and necessary here. The dynamic IR BasicAuth type (packages/ir-sdk/src/sdk/api/resources/dynamic/resources/auth/types/BasicAuth.ts) does not include usernameOmit/passwordOmit fields — those only exist on the main IR's BasicAuthScheme. Updating the dynamic IR types to add these fields is an IR-level change that is out of scope for this PR per maintainer instruction ("Fix the non-IR changes"). The cast allows us to safely check for these fields at runtime without modifying the IR SDK.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…to dynamic IR The DynamicSnippetsConverter was constructing dynamic BasicAuth with only username and password fields, dropping usernameOmit/passwordOmit from the main IR's BasicAuthScheme. This caused dynamic snippets generators to always include omitted auth fields (e.g. $password) since they couldn't detect the omit flags in the dynamic IR data. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
| const authRecord = auth as unknown as Record<string, unknown>; | ||
| const usernameOmitted = !!authRecord.usernameOmit; | ||
| const passwordOmitted = !!authRecord.passwordOmit; |
There was a problem hiding this comment.
🔴 Forbidden as unknown as X type assertion bypasses type system instead of updating dynamic IR type
The code uses auth as unknown as Record<string, unknown> to access usernameOmit/passwordOmit fields that don't exist on FernIr.dynamic.BasicAuth. This violates CLAUDE.md's explicit rule: "Never use as any or as unknown as X. These are escape hatches that bypass the type system entirely. If the types don't line up, fix the types."
The dynamic IR's BasicAuth type at packages/ir-sdk/fern/apis/ir-types-latest/definition/dynamic/auth.yml:22-25 only declares username and password. While packages/cli/generation/ir-generator/src/dynamic-snippets/DynamicSnippetsConverter.ts:736-748 does attach these fields at runtime using an intersection type, the proper fix is to add usernameOmit: optional<boolean> and passwordOmit: optional<boolean> to the dynamic IR's BasicAuth definition, then regenerate the SDK types so the field access is type-safe.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Acknowledged — the as unknown as Record<string, unknown> cast is a known limitation. The proper fix requires adding usernameOmit/passwordOmit to the dynamic IR's BasicAuth type definition and regenerating SDK types. This is deferred to a follow-up (IR schema changes are out of scope for this PR per reviewer instruction).
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
SDK Generation Benchmark ResultsComparing PR branch against latest nightly baseline 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 |
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
SDK Generation Benchmark ResultsComparing PR branch against latest nightly baseline 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 |
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…merge Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
SDK Generation Benchmark ResultsComparing PR branch against latest nightly baseline 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 |
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
SDK Generation Benchmark ResultsComparing PR branch against latest nightly baseline 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 |
…c-auth-optional-java-sdk
SDK Generation Benchmark ResultsComparing PR branch against latest nightly baseline 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 |
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
SDK Generation Benchmark ResultsComparing PR branch against latest nightly baseline 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 |
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
SDK Generation Benchmark ResultsComparing PR branch against latest nightly baseline 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 |
… main's 4.2.1) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…c-auth-optional-java-sdk
1 participant
Description
Refs #14378
Split from #14378 (one PR per generator).
When
usernameOmitorpasswordOmitflags are set in the IR'sBasicAuthScheme, the corresponding field is completely removed from the generated SDK's public API (builder fields,credentials()parameters, constructor parameters). Internally, omitted fields are treated as empty strings when encoding theAuthorization: Basicheader. Default behavior (both fields required) is preserved when no omit flags are set.Changes Made
AbstractRootClientGenerator.java:credentials()method only includes parameters for non-omitted fields (e.g.credentials(String username)when password is omitted)build()validation only checks non-omitted fieldsconfigureAuthMethodcondition only requires non-omitted fields to be presentthis.username + ":" + ""AuthProviderInfonow carriesfieldNameOmitted/secondaryFieldNameOmittedflags (new constructor overload)addRoutingAuthProviderSetup()(ENDPOINT_SECURITY path) uses omit flags to: only check non-omitted builder fields in the condition, and only pass constructor arguments for non-omitted fields to matchBasicAuthProvider's constructor signatureBasicAuthProviderGenerator.java:getAuthHeaders: uses""directly for omitted fields instead of reading from a suppliercanCreate: only checks non-omitted fields (env var or supplier presence)EndpointSnippetGenerator.ts(java-v2 dynamic snippets):usernameOmit/passwordOmitfrom the auth object (viaRecord<string, unknown>cast, since the dynamic IR type doesn't yet carry these fields).credentials()calls; when both are omitted, the credentials builder parameter is skipped entirelyversions.yml: new 4.2.1 entry (feat)basic-auth-pw-omittedtest fixture withpassword: omit: true, plus full seed output atseed/java-sdk/basic-auth-pw-omitted/Updates since last revision
EndpointSnippetGenerator.tsreadsusernameOmit/passwordOmitvia anas unknown as Record<string, unknown>cast (the dynamic IR type doesn't formally expose these fields yet). This replaces the previous "known limitation" where snippets always emitted both fields.basic-auth-optionaltobasic-auth-pw-omittedacross directories, file names, and file contentsAuthProviderInfonow tracks per-field omit flags (fieldNameOmitted,secondaryFieldNameOmitted)addRoutingAuthProviderSetup()only passes constructor args for non-omitted fieldsBasicAuthProviderGenerator: now conditional onusernameOmitted/passwordOmittedflagsTesting
basic-auth-pw-omittedfixtureSeedBasicAuthPwOmittedClientBuilderhas nopasswordfield andcredentials()takes onlyusernameas unknown as Record<string, unknown>cast inEndpointSnippetGenerator.ts: The dynamic IR'sBasicAuthtype doesn't formally includeusernameOmit/passwordOmit. The cast bypasses compile-time type checking — if these fields are renamed or restructured in the IR, the snippet generator will silently regress to emitting both fields (since!!undefinedisfalse). Verify this workaround is acceptable until the dynamic IR schema is updated.AuthProviderInfoconstructor has 10 parameters: The new overload addsfieldNameOmittedandsecondaryFieldNameOmittedbooleans. Verify the call sites pass these in the correct positional order — consecutive boolean parameters are easy to swap silently.addRoutingAuthProviderSetup(), verify the dynamically-builtconstructorArgsstring correctly matchesBasicAuthProvider's constructor for all four omit combinations. This path is not covered by the seed test fixture (no multi-scheme routing inbasic-auth-pw-omitted).basicAuthOptional→basicAuthPwOmittedin file contents (package declarations, imports) but the Java package directorycom/seed/basicAuthOptional/was not renamed. Verify seed snapshots compare file contents only and this doesn't cause issues.configureAuthMethod, when bothusernameOmittedandpasswordOmittedare true, theelsebranch is empty (no header set). Verify this no-op is intentional and doesn't leave a dangling control flow.seed/java-sdk/basic-auth-pw-omitted/src/main/java/com/seed/basicAuthOptional/SeedBasicAuthPwOmittedClientBuilder.java— verifypasswordis fully absent from fields,credentials()signature, and validationseed/java-sdk/basic-auth-pw-omitted/src/main/java/com/seed/basicAuthOptional/core/BasicAuthProvider.java— verifycanCreateandgetAuthHeaderscorrectly handle the omitted passwordLink to Devin session: https://app.devin.ai/sessions/0786b963284f4799acb409d5373cde0a
Requested by: @Swimburger