feat(oagen[1/2]): Add new endpoints to the SDK via autogeneration#1535
feat(oagen[1/2]): Add new endpoints to the SDK via autogeneration#1535gjtorikian wants to merge 8 commits intomainfrom
Conversation
Adds imports and readonly properties for AdminPortal, Permissions, Radar, WebhooksEndpoints, WorkOSConnect, Applications, and ApplicationClientSecrets. Adds createPaginatedList helper used by generated paginated list methods. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds the minimal set of auto-generated interfaces and serializers needed by the new service classes: - 12 common enum/type interfaces (referenced by radar, webhooks, admin-portal models) - 2 authorization model files (referenced by permissions serializer) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
gjtorikian
changed the title
gjtorikian
changed the title
Greptile SummaryThis PR introduces five new SDK surfaces via autogeneration — AdminPortal, Permissions, Radar, WebhooksEndpoints, and WorkOSConnect (Applications + ApplicationClientSecrets) — along with deprecation stubs on the old
Confidence Score: 4/5Safe to merge after the two P1 issues in applications.ts and application-client-secrets.ts are addressed Two P1 findings: the unserialized default payload in Applications.create() would send camelCase keys to the API in production, and the list() method on ApplicationClientSecrets has a name/return-type mismatch that could confuse callers. All other new services look correct and well-tested. src/workos-connect/applications.ts (default case in create()), src/workos-connect/application-client-secrets.ts (list() return type vs. endpoint semantics) Important Files Changed
Class Diagram%%{init: {'theme': 'neutral'}}%%
classDiagram
class WorkOS {
+adminPortal: AdminPortal
+permissions: Permissions
+radar: Radar
+webhooksEndpoints: WebhooksEndpoints
+workOsConnect: WorkOSConnect
+applications: Applications
+applicationClientSecrets: ApplicationClientSecrets
}
class AdminPortal {
+generatePortalLink(payload) PortalLinkResponse
}
class Permissions {
+list(options?) AutoPaginatable
+create(payload) Permission
+find(slug) AuthorizationPermission
+update(slug, payload) AuthorizationPermission
+delete(slug) void
}
class Radar {
+assess(payload) RadarStandaloneResponse
+updateRadarAttempt(id, payload) void
+updateRadarList(type, action, payload) RadarListEntryAlreadyPresentResponse
+deleteRadarListEntry(type, action, payload) void
}
class WebhooksEndpoints {
+list(options?) AutoPaginatable
+create(payload) WebhookEndpointJson
+update(id, payload) WebhookEndpointJson
+delete(id) void
}
class WorkOSConnect {
+completeLogin(payload) ExternalAuthCompleteResponse
}
class Applications {
+list(options?) AutoPaginatable
+create(payload) ConnectApplication
+find(id) ConnectApplication
+update(id, payload) ConnectApplication
+delete(id) void
}
class ApplicationClientSecrets {
+list(id) ApplicationCredentialsListItem
+create(id, payload) NewConnectApplicationSecret
+delete(id) void
}
WorkOS --> AdminPortal
WorkOS --> Permissions
WorkOS --> Radar
WorkOS --> WebhooksEndpoints
WorkOS --> WorkOSConnect
WorkOS --> Applications
WorkOS --> ApplicationClientSecrets
Reviews (1): Last reviewed commit: "feat(node): add generated model dependen..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
|
||
| /** | ||
| * Get a Connect Application | ||
| * | ||
| * Retrieve details for a specific Connect Application by ID or client ID. | ||
| * @param id - The application ID or client ID of the Connect Application. | ||
| * @example "conn_app_01HXYZ123456789ABCDEFGHIJ" | ||
| * @returns {ConnectApplication} | ||
| * @throws {NotFoundException} 404 |
There was a problem hiding this comment.
default case sends unserialized camelCase payload to the API
When applicationType is anything other than 'oauth' or 'm2m' (e.g., missing or an unexpected value), the raw TypeScript model with camelCase field names is sent directly to the API. The API expects snake_case wire format, so this silently produces a malformed request. The test even exercises this path via workos.applications.create({} as any), which hits the default branch and sends {} — but because the mock always returns the fixture, the test doesn't catch the mismatch.
The default should throw rather than silently forward the unserialized payload:
| /** | |
| * Get a Connect Application | |
| * | |
| * Retrieve details for a specific Connect Application by ID or client ID. | |
| * @param id - The application ID or client ID of the Connect Application. | |
| * @example "conn_app_01HXYZ123456789ABCDEFGHIJ" | |
| * @returns {ConnectApplication} | |
| * @throws {NotFoundException} 404 | |
| default: | |
| throw new Error( | |
| `Unknown applicationType: ${(payload as any).applicationType}`, | |
| ); |
| await this.workos.get<ApplicationCredentialsListItemResponse>( | ||
| `/connect/applications/${id}/client_secrets`, | ||
| ); | ||
| return deserializeApplicationCredentialsListItem(data); | ||
| } | ||
|
|
||
| /** | ||
| * Create a new client secret for a Connect Application |
There was a problem hiding this comment.
list() returns a single item, not a paginated list
The method is named list and the JSDoc says "List all client secrets", but the return type is Promise<ApplicationCredentialsListItem> — a single deserialized object. The endpoint /connect/applications/${id}/client_secrets is a collection path that typically returns multiple items. If the API actually returns a list envelope (or an array), the SDK silently drops all but the deserialized single-object interpretation.
The fixture application-credentials-list-item.fixture.json is a single object, which is consistent with the current implementation, but the naming ApplicationCredentialsListItem and the list() method name both suggest this is one item in a larger collection. If the API truly returns a single item at this endpoint, the method should be renamed to get() to reflect that. If it returns a list, it should use createPaginatedList (as done in the WebhooksEndpoints, Permissions, and Applications classes).
| @@ -1,3 +1,5 @@ | |||
| // This file is auto-generated by oagen. Do not edit. | |||
There was a problem hiding this comment.
workos.ts is the primary hand-maintained class that the rest of the SDK builds on — it contains the HTTP client, auth headers, error handling, and all existing service registrations. Marking it as // This file is auto-generated by oagen. Do not edit. signals to future contributors that manual edits will be overwritten, which may be true for the autogenerated sections but is misleading for the parts of the class that are not auto-generated. The same concern applies to src/common/utils/fetch-and-deserialize.ts and src/common/utils/test-utils.ts, both of which had hand-written code before this PR and now carry the same blanket "Do not edit" comment.
If only certain sections are managed by the generator, consider a more targeted approach (e.g., fenced regions or separate generated files that are imported) rather than marking the entire file as off-limits.
| switch ((payload as any).applicationType) { | ||
| case 'oauth': | ||
| return serializeCreateOAuthApplication(payload as any); | ||
| case 'm2m': | ||
| return serializeCreateM2MApplication(payload as any); | ||
| default: | ||
| return payload; | ||
| } |
There was a problem hiding this comment.
question: This whole switch statement casting the payload to any is making nervous. Is this hiding a larger types issue?
| readonly applications = new Applications(this); | ||
| readonly applicationClientSecrets = new ApplicationClientSecrets(this); |
There was a problem hiding this comment.
question: Should these be exposed at the top-level or should it be under workos.connect?
| readonly permissions = new Permissions(this); | ||
| readonly radar = new Radar(this); | ||
| readonly webhooksEndpoints = new WebhooksEndpoints(this); | ||
| readonly workOsConnect = new WorkOSConnect(this); |
There was a problem hiding this comment.
question: Should this just be connect so it's workos.connect instead of workos.workOsConnect?
If not, I'm not a fan of the workOsConnect casing.
| readonly adminPortal = new AdminPortal(this); | ||
| readonly permissions = new Permissions(this); | ||
| readonly radar = new Radar(this); | ||
| readonly webhooksEndpoints = new WebhooksEndpoints(this); | ||
| readonly workOsConnect = new WorkOSConnect(this); | ||
| readonly applications = new Applications(this); | ||
| readonly applicationClientSecrets = new ApplicationClientSecrets(this); |
There was a problem hiding this comment.
nitpick: all these new properties should live with the other properties.
| async list(id: string): Promise<ApplicationCredentialsListItem> { | ||
| const { data } = | ||
| await this.workos.get<ApplicationCredentialsListItemResponse>( | ||
| `/connect/applications/${id}/client_secrets`, | ||
| ); | ||
| return deserializeApplicationCredentialsListItem(data); | ||
| } |
There was a problem hiding this comment.
question: this method is called list but from the looks of it only returns one item. Is this the correct name for it?
| workos, | ||
| endpoint, | ||
| deserializeFn, | ||
| params, |
There was a problem hiding this comment.
AutoPaginatable only hands this callback { limit, after } on each page, so any extended filter options (like organizationId) get dropped after page 1. Merging options in keeps the filters on every page, and putting params last lets the pagination cursor still override after/limit as pages advance.
| params, | |
| { ...options, ...params }, |
| async list( | ||
| options?: ApplicationsListOptions, | ||
| ): Promise<AutoPaginatable<ConnectApplication, ApplicationsListOptions>> { | ||
| return createPaginatedList< | ||
| ConnectApplicationResponse, | ||
| ConnectApplication, | ||
| ApplicationsListOptions | ||
| >( | ||
| this.workos, | ||
| '/connect/applications', | ||
| deserializeConnectApplication, | ||
| options, | ||
| ); | ||
| } |
There was a problem hiding this comment.
question: is this going to pass in the deserialized options? Like organizationId instead of organization_id?
| import { WorkOS } from '../workos'; | ||
| import { GeneratePortalLinkIntent } from './interfaces/generate-portal-link-intent.interface'; | ||
|
|
||
| /** @deprecated Use `AdminPortal` instead. */ |
There was a problem hiding this comment.
issue: this was added here but the deprecation message doesn't pass through to the WorkOS property (workos.portal.generateLink, for example). The @deprecated directive should be added to the property in wokros.ts, too.
| /** @deprecated Use `workos.permissions.list()` instead. */ | ||
| async listPermissions( | ||
| options?: ListPermissionsOptions, | ||
| ): Promise<PermissionList> { |
There was a problem hiding this comment.
issue: This returns PermissionList which is different from what workos.permissions.list will return (AutoPaginatable). This is technically a breaking change.
| export interface ApplicationsListOptions extends PaginationOptions { | ||
| /** Filter Connect Applications by organization ID. */ | ||
| organizationId?: string; | ||
| } |
There was a problem hiding this comment.
question: should this be externally exported? I don't think it's exposed anywhere.
2 participants
Description
This PR adds new SDK support for five new WorkOS API surfaces:
portal, which is a terrible name. I@deprecated those files in favor of this one with a better naming system. The SDK autogenerated wants to call theseAdminPortal.Permissions: full CRUD for authorization permissions. Previously this was confusingly tucked underfeat(oagen[2/2]): Update existing SDK modules with generated types, docs, and serializers #1536 reverts this.authorization; that, too, has been marked as@deprecate, considering that the SDK generator and API docs treat them separately than the other/authorizationendpoints.ApplicationsandApplicationClientSecretssub-services for managing Connect Applications and their secretsIn the cases above where the SDK autogen is choosing a name (AdminPortal, Permissions), we can, of course, override these, but I happen to agree with it.
Note: this is the first of two PRs.
Why so many changed files?
Each change above was broken into its own commit. The final commit, 2b5ca3c, introduces a set of new changes across the SDK.
Each auto-generated service class imports typed interfaces for its request/response models and enum types. These types are split into separate files by the generator (one interface per schema). The 14 dependency files break down as:
src/common/interfaces/: shared enum/literal-union types (e.g.,RadarStandaloneResponseVerdict,WebhookEndpointJsonStatus,GenerateLinkIntent) referencedby Radar, WebhooksEndpoints, and AdminPortal model interfaces
src/authorization/:UpdateOrganizationRoleinterface and serializer, imported by the Permissions serializer because both share the same wire shape for role-permission updates