Skip to content

feat(ts-sdk): support optional username/password in basic auth when configured in IR#14406

Open
Swimburger wants to merge 11 commits intomainfrom
devin/1774997664-basic-auth-optional-ts-sdk
Open

feat(ts-sdk): support optional username/password in basic auth when configured in IR#14406
Swimburger wants to merge 11 commits intomainfrom
devin/1774997664-basic-auth-optional-ts-sdk

Conversation

@Swimburger
Copy link
Copy Markdown
Member

@Swimburger Swimburger commented Mar 31, 2026

Description

Adds conditional support for omitting username/password in basic auth for the TypeScript SDK generator. When usernameOmit or passwordOmit flags are set in the IR's BasicAuthScheme, the corresponding field is completely removed from the generated SDK's AuthOptions type — not just made optional. Internally, the omitted field is treated as an empty string when encoding the auth header (base64(username:password) with the colon always present).

Default behavior (both required) is preserved when no omit flags are set.

Split from #14378 (one PR per generator).

Changes Made

Generator (BasicAuthProviderGenerator.ts)

  • getAuthOptionsProperties: conditionally excludes each field from the AuthOptions type entirely when its corresponding omit flag is set (field is removed, not made optional)
  • generateCanCreateStatements: per-field checks — omittable fields always satisfy canCreate (return true), required fields must be present
  • generateGetAuthRequestStatements: refactored into buildNullChecks helper — omitted fields use "" directly instead of reading from options; null checks are only generated for non-omitted fields; declarations and null checks are interleaved to preserve short-circuit evaluation order

Core utility (BasicAuth.ts)

  • Interface fields widened to username?: string; password?: string unconditionally
  • toAuthorizationHeader uses ?? "" fallbacks and returns undefined when both are empty strings

Tests & fixtures

  • New basic-auth-optional test fixture with password: omit: true
  • Full seed output for seed/ts-sdk/basic-auth-optional/
  • Updated seed/ts-sdk/basic-auth/ output (reflects widened BasicAuth.ts interface)
  • Unit tests added for username-only, password-only, neither, and both-empty cases

Changelog

  • versions.yml: new 3.61.1 entry (bumped past 3.61.0-rc.0 from main)

Updates since last revision

  • Fixed short-circuit evaluation order: buildNullChecks now interleaves declaration and null-check for each field (username decl → username null check → password decl → password null check), preserving the original short-circuit behavior where the password supplier is never evaluated if the username is null. Previously, both declarations were emitted before both null checks.
  • Regenerated seed output for basic-auth-optional fixture to reflect interleaved null check pattern.

Testing

  • Unit tests added (BasicAuth.test.ts: 5 cases covering all credential combinations)
  • Seed snapshot generated for basic-auth-optional fixture
  • Existing basic-auth seed output updated and verified
  • All required CI checks pass (304 pass, 3 non-required fail: Lint PR title, Validate IR Version Consistency)

⚠️ Human Review Checklist

  1. Snapshot file may be stale after interleave fix: The __snapshots__/AuthProviders.test.ts.snap file in the cumulative diff shows const username = ...; const password = ...; if (username == null) ...; if (password == null) ...; (both declarations then both checks). However, after the interleave fix the generator now produces const username = ...; if (username == null) ...; const password = ...; if (password == null) ...; (interleaved). Verify the snapshot tests pass — if they don't, the snapshot file needs regeneration.
  2. BasicAuth.ts interface is widened unconditionally: Every generated TS SDK (not just those with omit flags) will now have username?: string; password?: string in the core utility. The generator controls what appears in AuthOptions, so the runtime behavior is guarded — but this does widen the type contract for downstream consumers who import BasicAuth directly.
  3. Behavioral change for empty strings: toAuthorizationHeader({username: "", password: ""}) now returns undefined instead of Basic Og==. This affects the authHeader != null guard in generated code.
  4. Both-omittable path: When both usernameOmit and passwordOmit are true, canCreate() always returns true and getAuthRequest hardcodes both to "", producing Basic Og== — but toAuthorizationHeader then returns undefined since both are empty. Verify this edge case is acceptable.
  5. seed/ts-sdk/basic-auth/ diff: The existing (non-optional) fixture output changed due to the unconditional BasicAuth.ts widening. Confirm the generated client behavior is still correct for the required-fields case.

Link to Devin session: https://app.devin.ai/sessions/0786b963284f4799acb409d5373cde0a
Requested by: @Swimburger


Open with Devin

…onfigured in IR

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration
Copy link
Copy Markdown
Contributor

🤖 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 and CI monitoring

Copy link
Copy Markdown

@claude claude bot left a comment

Choose a reason for hiding this comment

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

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]

This comment was marked as resolved.

…d flag

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
devin-ai-integration[bot]

This comment was marked as resolved.

Swimburger and others added 6 commits April 1, 2026 16:02
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…-field omit fix

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>
…e empty string internally

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 2 new potential issues.

View 8 additional findings in Devin Review.

Open in Devin Review

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Generator emits unused USERNAME_PARAM/PASSWORD_PARAM constants for omitted auth fields

In writeConstants at BasicAuthProviderGenerator.ts:159-170, both USERNAME_PARAM and PASSWORD_PARAM are always emitted regardless of omit flags. When a field is omitted (e.g. passwordOmit: true), the corresponding constant is never referenced in canCreate, getAuthRequest, or the AuthOptions type — it becomes dead code.

The generated seed output at seed/ts-sdk/basic-auth-optional/src/auth/BasicAuthProvider.ts:7 confirms this: const _PASSWORD_PARAM = "password" as const; — the linter renamed it with an underscore prefix because it's unused. The fix would be to conditionally skip generating the constant when the corresponding omit flag is true.

(Refers to lines 169-170)

Open in Devin Review

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Valid observation about the unused constant. This is a minor issue — the generated biome config already renames it with an underscore prefix (_PASSWORD_PARAM), and the constant is still referenced in the module namespace for the AUTH_CONFIG_ERROR_MESSAGE_PASSWORD string template in the non-omit case. Conditionally skipping the constant would require tracking which fields are omitted across both writeConstants and writeOptions, adding complexity for a cosmetic improvement. I'll leave this as-is unless the maintainer wants it addressed.

Swimburger and others added 3 commits April 2, 2026 00:40
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>
…teral property names

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
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.

1 participant