Skip to content

feat(ruby-sdk): remove omitted basic auth fields from SDK API, use empty string internally#14411

Open
Swimburger wants to merge 4 commits intomainfrom
devin/1774997764-basic-auth-optional-ruby-sdk
Open

feat(ruby-sdk): remove omitted basic auth fields from SDK API, use empty string internally#14411
Swimburger wants to merge 4 commits intomainfrom
devin/1774997764-basic-auth-optional-ruby-sdk

Conversation

@Swimburger
Copy link
Copy Markdown
Member

@Swimburger Swimburger commented Mar 31, 2026

Description

Refs #14378

Split from #14378 (one PR per generator).

When usernameOmit or passwordOmit flags are set in the IR's BasicAuthScheme, the generated Ruby SDK now completely removes the omitted field from the constructor parameters (rather than making it optional/nilable). Internally, the omitted field is treated as an empty string when encoding the Authorization header. Default behavior (both fields required) is preserved when no omit flags are set.

Changes Made

  • RootClientGenerator.tsgetAuthenticationParameters(): Per-field omit checks via as unknown as Record<string, unknown> cast. When usernameOmit/passwordOmit is true, the corresponding keyword parameter is excluded entirely from the generated constructor.
  • RootClientGenerator.tsgetInitializeMethod(): Per-field condition and encoding logic:
    • Omitted fields use "" directly in Base64.strict_encode64 (no || "" fallback needed since the field doesn't exist)
    • Nil-check conditions only reference non-omitted fields (e.g., passwordOmit: true → condition is !username.nil? only)
    • Both-omitted edge case uses true condition (header always set with ":" encoded)
  • versions.yml: New 1.1.12 entry
  • New basic-auth-optional test fixture with password: omit: true, plus full seed output at seed/ruby-sdk-v2/basic-auth-optional/
  • IR-to-JSON-schema snapshot for the new fixture

Testing

  • Seed snapshot generated for basic-auth-optional fixture
  • Existing seed fixtures unchanged (no regressions)
  • Biome lint passes locally

⚠️ Review Checklist

  1. IR version mismatch (v61): The Ruby IR SDK (v61) doesn't expose usernameOmit/passwordOmit in its type definitions. The as unknown as Record<string, unknown> cast reads these fields at runtime but has zero compile-time safety. Until the Ruby generator's IR version is bumped to v63+, these flags will always be undefined at runtime, meaning the seed output still shows both fields present. The generator code is correct and forward-compatible — it will activate once the IR version is bumped separately.
  2. Per-field removal vs. old eitherOmitted: Previous revision used a coarse eitherOmitted flag that made both fields optional when either had the omit flag. This has been replaced with independent per-field checks — only the specifically flagged field is removed.
  3. Both-omitted edge case: If both usernameOmit and passwordOmit are true, the condition becomes true and the header is always set with Base64(":"). This is intentional per the spec ("internally use empty string").

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


Open with Devin

… configured 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.

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 potential issues.

View 3 additional findings in Devin Review.

Open in Devin Review

omitted entirely.
type: feat
createdAt: "2026-03-31"
irVersion: 61
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot Mar 31, 2026

Choose a reason for hiding this comment

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

🔴 irVersion not bumped to v63 — usernameOmit/passwordOmit fields are stripped before reaching the generator

The entire feature is non-functional because versions.yml declares irVersion: 61, but usernameOmit and passwordOmit were introduced in IR v63. When the CLI runs this generator, it migrates the latest IR down to v61, and the v63-to-v62 migration (packages/cli/generation/ir-migrations/src/migrations/v63-to-v62/migrateFromV63ToV62.ts:140-147) explicitly strips these fields from the BasicAuthScheme. As a result, the as unknown as Record<string, unknown> cast at RootClientGenerator.ts:126-128 will always see undefined for both usernameOmit and passwordOmit, so the omit logic never activates.

The generated seed output at seed/ruby-sdk-v2/basic-auth-optional/lib/seed/client.rb:10 confirms the bug: the constructor still requires both username: and password: as parameters, even though the test definition (test-definitions/fern/apis/basic-auth-optional/definition/api.yml:10) sets omit: true on password. The irVersion must be bumped to at least 63 in both versions.yml and seed.yml.

Per REVIEW.md: "IR version bumps must be coordinated: update irVersion in the relevant versions.yml entry."

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.

The IR version mismatch (v61 vs v63) is a known limitation and out of scope for this PR per discussion with the maintainer. The generator code is forward-compatible and will activate once the IR version is bumped in a separate PR.

Swimburger and others added 3 commits April 1, 2026 15:53
…instead of coarse eitherOmitted flag

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

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…, use empty string internally

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration devin-ai-integration bot changed the title feat(ruby-sdk): support optional username/password in basic auth when configured in IR feat(ruby-sdk): remove omitted basic auth fields from SDK API, use empty string internally Apr 2, 2026
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 1 new potential issue.

View 7 additional findings in Devin Review.

Open in Devin Review

Comment on lines +126 to +128
const scheme = basicAuthScheme as unknown as Record<string, unknown>;
const usernameOmitted = scheme.usernameOmit === true;
const passwordOmitted = scheme.passwordOmit === true;
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.

🟡 Unnecessary unsafe cast bypasses type-safe access to usernameOmit/passwordOmit

The code casts basicAuthScheme to Record<string, unknown> via as unknown as Record<string, unknown> to access usernameOmit/passwordOmit, but FernIr.BasicAuthScheme already defines these as boolean | undefined (packages/ir-sdk/src/sdk/api/resources/auth/types/BasicAuthScheme.ts:10,15). The same pattern is repeated in getAuthenticationParameters() at line 364. Using scheme.usernameOmit and scheme.passwordOmit directly would provide type safety and make it obvious that these fields are part of the IR type — which would have surfaced the irVersion mismatch as a design concern earlier.

Suggested change
const scheme = basicAuthScheme as unknown as Record<string, unknown>;
const usernameOmitted = scheme.usernameOmit === true;
const passwordOmitted = scheme.passwordOmit === true;
const usernameOmitted = basicAuthScheme.usernameOmit === true;
const passwordOmitted = basicAuthScheme.passwordOmit === true;
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.

Good catch — the Ruby generator imports BasicAuthScheme from @fern-fern/ir-sdk v61 which doesn't have usernameOmit/passwordOmit in its type definitions, so basicAuthScheme.usernameOmit would be a type error at compile time. The as unknown as Record<string, unknown> cast is necessary for this IR version. Once the IR version is bumped to v63+, this cast can be replaced with direct access.

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