Skip to content

fix: restore typed data on AuthenticationErrorData#1570

Merged
gjtorikian merged 2 commits intomainfrom
restore-data
Apr 23, 2026
Merged

fix: restore typed data on AuthenticationErrorData#1570
gjtorikian merged 2 commits intomainfrom
restore-data

Conversation

@gjtorikian
Copy link
Copy Markdown
Contributor

@gjtorikian gjtorikian commented Apr 23, 2026

Summary

  • Replace the loosely-typed user?: Record<string, unknown> with the concrete UserResponse type on AuthenticationErrorData, restoring type safety for authentication error payloads
  • Add missing connection_ids?: string[] field to AuthenticationErrorData
  • Fix casing typo on SSOIntentOptionsFixture import (PascalCase → camelCase to match other fixture imports)

Test plan

  • Verify AuthenticationErrorData.user resolves to UserResponse in IDE type-checking
  • Verify connection_ids field is accessible on AuthenticationErrorData
  • Run npm test — existing tests pass, including the renamed fixture import in serializers.spec.ts

Summary by CodeRabbit

  • Tests

    • Refactored test fixture variable naming in authentication serialization tests for improved maintainability.
  • Refactor

    • Strengthened type definitions for authentication error handling with more precise user data typing and connection identifier tracking support.

@gjtorikian gjtorikian requested review from a team as code owners April 23, 2026 15:02
@gjtorikian gjtorikian requested a review from imkesin April 23, 2026 15:02
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 23, 2026

📝 Walkthrough

Walkthrough

A test fixture import is renamed in the serializers test file, and the authentication exception error payload interface is refined with stricter typing for the user field and an additional optional connection_ids field.

Changes

Cohort / File(s) Summary
Test fixture update
src/admin-portal/serializers.spec.ts
Fixture import renamed from SSOIntentOptionsFixture to ssoIntentOptionsFixture with corresponding variable updates in type casting and assertions.
Authentication exception typing
src/common/exceptions/authentication.exception.ts
AuthenticationErrorData interface updated with stricter user field typing (Record<string, unknown>UserResponse) and new optional connection_ids?: string[] field.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description includes a clear summary of changes and a test plan. However, it lacks the required 'Documentation' section from the template, which should specify whether docs updates are needed. Add the required 'Documentation' section from the template with a checkbox indicating whether this change requires updates to the WorkOS Docs or API Reference.
✅ Passed checks (4 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title 'fix: restore typed data on AuthenticationErrorData' directly and clearly describes the main change: restoring type safety on the AuthenticationErrorData interface.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch restore-data

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 23, 2026

Greptile Summary

This PR restores type safety on AuthenticationErrorData by replacing the loosely-typed user?: Record<string, unknown> with the concrete UserResponse wire-format interface, adds the missing connection_ids?: string[] field, and fixes an import naming convention (PascalCase → camelCase) in the serializer test file. The changes are minimal, targeted, and consistent with the existing codebase patterns.

Confidence Score: 5/5

Safe to merge — changes are additive, type-only improvements with no runtime behavior changes.

All changes are compile-time type improvements (UserResponse instead of Record<string, unknown>) and an additive new optional field. No logic is altered. The import rename is a cosmetic fix that aligns with existing conventions. No P0 or P1 issues found.

No files require special attention.

Important Files Changed

Filename Overview
src/common/exceptions/authentication.exception.ts Restores type safety by replacing Record<string, unknown> with UserResponse on the user field, and adds the missing connection_ids?: string[] field to AuthenticationErrorData.
src/admin-portal/serializers.spec.ts Fixes the import identifier casing for the SSO intent options fixture from SSOIntentOptionsFixture to ssoIntentOptionsFixture, aligning it with camelCase conventions used by other fixture imports.

Class Diagram

%%{init: {'theme': 'neutral'}}%%
classDiagram
    class WorkOSErrorData {
        +code: string
        +message: string
    }
    class AuthenticationErrorData {
        +code: AuthenticationErrorCode
        +pending_authentication_token?: string
        +user?: UserResponse
        +organizations?: Array~id, name~
        +connection_ids?: string[]
    }
    class UserResponse {
        +object: 'user'
        +id: string
        +email: string
        +email_verified: boolean
        +profile_picture_url: string | null
        +first_name: string | null
        +last_name: string | null
        +last_sign_in_at: string | null
        +locale: string | null
        +created_at: string
        +updated_at: string
        +external_id?: string
        +metadata?: Record~string, string~
    }
    class AuthenticationException {
        +name: 'AuthenticationException'
        +pendingAuthenticationToken: string | undefined
        +rawData: AuthenticationErrorData
        +constructor(status, rawData, requestID)
    }
    WorkOSErrorData <|-- AuthenticationErrorData : extends
    AuthenticationErrorData --> UserResponse : user?
    AuthenticationException --> AuthenticationErrorData : rawData
Loading

Reviews (1): Last reviewed commit: "enhance types on `AuthenticationErrorDat..." | Re-trigger Greptile

@gjtorikian gjtorikian changed the title Restore typed data on AuthenticationErrorData fix: restore typed data on AuthenticationErrorData Apr 23, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/common/exceptions/authentication.exception.ts`:
- Around line 18-20: The type guard isAuthenticationErrorData must validate the
optional fields to match the stricter AuthenticationErrorData shape: update
isAuthenticationErrorData (the runtime check around code) to also ensure that if
user is present it is an object with the expected UserResponse properties (e.g.,
id is a string and other key fields exist), that connection_ids (if present) is
an array whose every item is a string, and that organizations (if present) is an
array of objects each with string id and name; return false if any of these
checks fail so the guard truly narrows to AuthenticationErrorData.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 04149f6e-ac1f-4ff7-9e48-a207128324da

📥 Commits

Reviewing files that changed from the base of the PR and between 1226ad0 and e854cce.

📒 Files selected for processing (2)
  • src/admin-portal/serializers.spec.ts
  • src/common/exceptions/authentication.exception.ts

Comment thread src/common/exceptions/authentication.exception.ts
@gjtorikian gjtorikian merged commit a7a1e6d into main Apr 23, 2026
8 of 10 checks passed
@gjtorikian gjtorikian deleted the restore-data branch April 23, 2026 15:14
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