feat(connect): Add Connect module#1597
Conversation
Expose a Connect client for managing OAuth and M2M applications, application secrets, and the external auth login/consent flow. Generated from the latest oagen spec and wired into the WorkOS client alongside the existing resource clients.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughThis PR adds a generated Connect product client: TypeScript interfaces and serializers, a Connect class with methods for OAuth completion, application CRUD (OAuth/M2M), pagination, and client-secret management; fixtures and Jest tests for serializers and client methods; and exposes Connect in the SDK public API and WorkOS class. ChangesConnect API Client Implementation
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR introduces a new
Confidence Score: 4/5Merging is low-risk for most callers, but the auto-pagination path and the two convenience-method helpers in connect.ts have known functional defects that have been flagged in prior review rounds and remain unaddressed. The listApplications auto-pagination callback passes raw camelCase options (including organizationId) instead of serialized snake_case, so any paginated traversal with an organization filter silently drops the filter after the first page. The createOAuthApplication and createM2MApplication convenience methods build the wire body by hand, bypassing the typed serializers, which creates a null-vs-absent inconsistency for optional fields. Both issues are present in the code today and affect observable behavior for callers who use those paths. src/connect/connect.ts — the listApplications pagination callback and the two convenience-method helpers warrant a closer look before this lands. Important Files Changed
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
src/connect/connect.ts (1)
210-228: ⚡ Quick winManual body construction bypasses serialization logic.
Similar to
createOAuthApplication, this method manually builds the request body instead of usingserializeCreateM2MApplication(imported at line 51). While M2M applications don't have nested objects requiring serialization like OAuth applications do, this is still inconsistent with:
- The
createApplicationmethod (lines 139-163) which uses serializers- The
updateApplicationmethod (lines 257-269) which uses serializers♻️ Recommended refactor to use the serializer
async createM2MApplication( name: string, organizationId: string, description?: string | null, scopes?: string[] | null, ): Promise<ConnectApplication> { - const body: Record<string, unknown> = { - application_type: 'm2m', - name: name, - organization_id: organizationId, - }; - if (description !== undefined) body.description = description; - if (scopes !== undefined) body.scopes = scopes; - const { data } = await this.workos.post<ConnectApplicationResponse>( + const payload: CreateM2MApplication = { + applicationType: 'm2m', + name, + organizationId, + ...(description !== undefined && { description }), + ...(scopes !== undefined && { scopes }), + }; + const { data } = await this.workos.post<ConnectApplicationResponse, CreateM2MApplicationResponse>( '/connect/applications', - body, + serializeCreateM2MApplication(payload), ); return deserializeConnectApplication(data); }src/connect/serializers.spec.ts (1)
43-43: ⚡ Quick winType safety bypassed with
as anycast.The
as anycast defeats TypeScript's type checking. While this may be intentional in generated code to handle minor fixture/interface mismatches, it means type errors in serializers won't be caught by these tests.If oagen controls the generation, consider ensuring fixtures exactly match their corresponding interfaces to avoid needing
as any.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a6438bd4-4c7e-48e1-b5b9-0e0b42b27e73
📒 Files selected for processing (49)
.oagen-manifest.jsonsrc/connect/connect.spec.tssrc/connect/connect.tssrc/connect/fixtures/application-credentials-list-item.jsonsrc/connect/fixtures/connect-application.jsonsrc/connect/fixtures/create-application-secret.jsonsrc/connect/fixtures/create-m2m-application.jsonsrc/connect/fixtures/create-oauth-application.jsonsrc/connect/fixtures/external-auth-complete-response.jsonsrc/connect/fixtures/list-connect-application.jsonsrc/connect/fixtures/new-connect-application-secret.jsonsrc/connect/fixtures/redirect-uri-input.jsonsrc/connect/fixtures/update-oauth-application.jsonsrc/connect/fixtures/user-consent-option-choice.jsonsrc/connect/fixtures/user-consent-option.jsonsrc/connect/fixtures/user-management-login-request.jsonsrc/connect/fixtures/user-object.jsonsrc/connect/interfaces/application-credentials-list-item.interface.tssrc/connect/interfaces/connect-application.interface.tssrc/connect/interfaces/create-application-secret.interface.tssrc/connect/interfaces/create-m2m-application.interface.tssrc/connect/interfaces/create-oauth-application.interface.tssrc/connect/interfaces/external-auth-complete-response.interface.tssrc/connect/interfaces/index.tssrc/connect/interfaces/list-applications-options.interface.tssrc/connect/interfaces/new-connect-application-secret.interface.tssrc/connect/interfaces/redirect-uri-input.interface.tssrc/connect/interfaces/update-oauth-application.interface.tssrc/connect/interfaces/user-consent-option-choice.interface.tssrc/connect/interfaces/user-consent-option.interface.tssrc/connect/interfaces/user-management-login-request.interface.tssrc/connect/interfaces/user-object.interface.tssrc/connect/serializers.spec.tssrc/connect/serializers/application-credentials-list-item.serializer.tssrc/connect/serializers/connect-application.serializer.tssrc/connect/serializers/create-application-secret.serializer.tssrc/connect/serializers/create-m2m-application.serializer.tssrc/connect/serializers/create-oauth-application.serializer.tssrc/connect/serializers/external-auth-complete-response.serializer.tssrc/connect/serializers/index.tssrc/connect/serializers/new-connect-application-secret.serializer.tssrc/connect/serializers/redirect-uri-input.serializer.tssrc/connect/serializers/update-oauth-application.serializer.tssrc/connect/serializers/user-consent-option-choice.serializer.tssrc/connect/serializers/user-consent-option.serializer.tssrc/connect/serializers/user-management-login-request.serializer.tssrc/connect/serializers/user-object.serializer.tssrc/index.tssrc/workos.ts
| /** A hint showing the last few characters of the secret value. */ | ||
| secretHint: string; | ||
| /** The timestamp when the client secret was last used, or null if never used. */ | ||
| lastUsedAt: string | null; |
There was a problem hiding this comment.
Timestamp type inconsistency in internal interface.
lastUsedAt is typed as string | null while createdAt and updatedAt are typed as Date. The internal interface should use Date | null for all timestamp fields after deserialization from the wire format. This inconsistency will cause type errors and break code that expects uniform Date handling across timestamp properties.
🐛 Proposed fix
- lastUsedAt: string | null;
+ lastUsedAt: Date | null;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| lastUsedAt: string | null; | |
| lastUsedAt: Date | null; |
There was a problem hiding this comment.
will work on getting this fixed upstream in the spec
There was a problem hiding this comment.
@gjtorikian Thanks for the update! Since the fix will happen upstream in the spec, would you like me to open a GitHub issue to track the lastUsedAt: string | null → Date | null correction in ApplicationCredentialsListItem? That way it won't get lost before the next oagen regeneration.
| /** A hint showing the last few characters of the secret value. */ | ||
| secretHint: string; | ||
| /** The timestamp when the client secret was last used, or null if never used. */ | ||
| lastUsedAt: string | null; |
There was a problem hiding this comment.
Timestamp type inconsistency in internal interface.
lastUsedAt is typed as string | null while createdAt and updatedAt are typed as Date. The internal interface should use Date | null for all timestamp fields after deserialization. This matches the same issue in application-credentials-list-item.interface.ts and will cause type errors when serializers convert timestamps.
🐛 Proposed fix
- lastUsedAt: string | null;
+ lastUsedAt: Date | null;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| lastUsedAt: string | null; | |
| lastUsedAt: Date | null; |
There was a problem hiding this comment.
will work on getting this fixed upstream in the spec
There was a problem hiding this comment.
@gjtorikian, understood! Since this is an auto-generated file, fixing it at the spec level makes sense. Would you like me to open a GitHub issue to track this fix? It would cover the lastUsedAt: string | null → Date | null correction in both new-connect-application-secret.interface.ts and the same inconsistency in application-credentials-list-item.interface.ts.
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
| /** An ISO 8601 timestamp. */ | ||
| updatedAt: Date; | ||
| /** The type of the application. */ | ||
| applicationType?: 'm2m'; |
There was a problem hiding this comment.
The CreateM2MApplication and CreateOAuthApplication types look good, but this ConnectApplication type is a bit weird. I'd expect applicationType to be a discriminator between the two as the form a kind of union, but here it only lists m2m.
Seems like this modeling isn't quite right. Is there a way to get the generator to give us something better?
| * @throws {UnprocessableEntityException} 422 | ||
| */ | ||
| async createApplication( | ||
| payload: CreateOAuthApplication | CreateM2MApplication, |
There was a problem hiding this comment.
Nice, this type of union of the different applications is more what I was imagining. 💯
mthadley
left a comment
mthadley
left a comment
There was a problem hiding this comment.
@gjtorikian Overall, looks solid.
The only thing that looks off and that could be usability problem is this, so requesting changes until it can be addressed.
2 participants
Summary
Connectresource client onWorkOSfor managing OAuth and M2M applications, application secrets, redirect URIs, user consent options, and the external auth login/consent flow.src/connect/from the latest oagen spec.src/connect/interfacesfrom the package barrel so consumers can import the new types.Test plan
npm testpasses (unit + serializer specs)npm run buildsucceeds with the new exportsworkos.connectis reachable on the client and methods type-check from a consumerSummary by CodeRabbit
New Features
Tests