feat!: standardize authorization list endpoint pagination#1553
Merged
gjtorikian merged 9 commits intomainfrom Apr 17, 2026
Merged
feat!: standardize authorization list endpoint pagination#1553gjtorikian merged 9 commits intomainfrom
gjtorikian merged 9 commits intomainfrom
Conversation
Contributor
Author
|
@greptile review plz |
swaroopAkkineniWorkos
changed the title
Contributor
Author
|
@greptile re-review plz |
Contributor
Greptile SummaryThis PR standardizes authorization list endpoints to use the shared The implementation is correct: path parameters are properly extracted into endpoint URLs, and only pagination options flow through to query strings. Test coverage is thorough across all refactored methods. Confidence Score: 5/5Safe to merge — the pagination bug fix is correctly implemented and all findings are minor style suggestions. No P0 or P1 issues found. The core fix (defaulting to desc order via fetchAndDeserialize/setDefaultOptions) is correct, path params are properly isolated to endpoint URLs, and the new listEffectivePermissions methods follow the established pattern correctly. All remaining observations are P2. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant Caller
participant Authorization
participant fetchAndDeserialize
participant setDefaultOptions
participant WorkOS API
Caller->>Authorization: listEffectivePermissions(options)
Authorization->>Authorization: extract path params → endpoint URL
Authorization->>Authorization: serializeListEffectivePermissionsOptions(options) → pagination-only Record
Authorization->>fetchAndDeserialize: (workos, endpoint, deserializePermission, serializedOptions)
fetchAndDeserialize->>setDefaultOptions: options (may lack order)
setDefaultOptions-->>fetchAndDeserialize: { ...options, order: options.order || 'desc' }
fetchAndDeserialize->>WorkOS API: GET endpoint?order=desc&...
WorkOS API-->>fetchAndDeserialize: ListResponse<PermissionResponse>
fetchAndDeserialize-->>Authorization: List<Permission>
Authorization->>Caller: AutoPaginatable<Permission>
Note over Caller,WorkOS API: autoPagination() calls apiCall({...options, limit:100, after:cursor}), captured endpoint closure reuses same URL
Reviews (6): Last reviewed commit: "prettier" | Re-trigger Greptile |
atainter
approved these changes
Apr 15, 2026
gjtorikian
changed the title
gjtorikian
changed the title
…ce tests Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughRefactors authorization list APIs to return AutoPaginatable, adds two effective-permissions list endpoints and their serializers/options interfaces, removes several list-specific interfaces, adds a fixture for effective permissions, and updates tests to use a unified Changes
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/authorization/authorization.ts`:
- Around line 444-529: The helpers listResourcesForMembership,
listMembershipsForResource, and listMembershipsForResourceByExternalId currently
pass the full options bag (including path identifiers) into their serializer
functions (serializeListResourcesForMembershipOptions,
serializeListMembershipsForResourceOptions); update each function to destructure
and remove path params first—organizationMembershipId, resourceId,
organizationId, resourceTypeSlug, externalId—and then call the corresponding
serializer with only the remaining query options so path identifiers cannot be
encoded into the query string.
- Around line 239-257: The AutoPaginatable change made list responses
non-enumerable and breaks callers expecting a plain { object, data, listMetadata
} shape; update listPermissions (and other authz list methods) to return a plain
enumerable object instead of exposing AutoPaginatable internals: either (A)
convert the AutoPaginatable instance into a plain object before returning (e.g.,
construct and return { object: 'list', data: autoPaginatable.list, listMetadata:
autoPaginatable.listMetadata, ...options }), or (B) modify AutoPaginatable in
src/common/utils/pagination.ts to expose enumerable data and listMetadata (or
implement a toJSON()/toPlainObject() that returns the original shape) and call
that from listPermissions; reference the listPermissions function and the
AutoPaginatable class in pagination.ts when making the change so the returned
value preserves the old enumerable shape.
🪄 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: f9a3fcec-848f-4477-94a1-949c6865ea50
📒 Files selected for processing (6)
src/authorization/authorization.spec.tssrc/authorization/authorization.tssrc/authorization/interfaces/authorization-resource.interface.tssrc/authorization/interfaces/permission.interface.tssrc/authorization/interfaces/role-assignment.interface.tssrc/user-management/interfaces/organization-membership.interface.ts
💤 Files with no reviewable changes (4)
- src/authorization/interfaces/authorization-resource.interface.ts
- src/authorization/interfaces/permission.interface.ts
- src/authorization/interfaces/role-assignment.interface.ts
- src/user-management/interfaces/organization-membership.interface.ts
Comment on lines
239
to
257
| async listPermissions( | ||
| options?: ListPermissionsOptions, | ||
| ): Promise<PermissionList> { | ||
| const { data } = await this.workos.get<PermissionListResponse>( | ||
| '/authorization/permissions', | ||
| { query: options }, | ||
| ): Promise<AutoPaginatable<Permission>> { | ||
| return new AutoPaginatable( | ||
| await fetchAndDeserialize<PermissionResponse, Permission>( | ||
| this.workos, | ||
| '/authorization/permissions', | ||
| deserializePermission, | ||
| options, | ||
| ), | ||
| (params) => | ||
| fetchAndDeserialize<PermissionResponse, Permission>( | ||
| this.workos, | ||
| '/authorization/permissions', | ||
| deserializePermission, | ||
| params, | ||
| ), | ||
| options, | ||
| ); |
There was a problem hiding this comment.
AutoPaginatable changes the runtime shape of these list responses.
This no longer returns a plain { object, data, listMetadata } object. In src/common/utils/pagination.ts:1-63, AutoPaginatable stores list, apiCall, and options as own instance fields, while data and listMetadata are prototype getters. So spreads, Object.keys(), and JSON serialization will behave differently for listPermissions() and the other converted authz list methods, which is a broader breaking change than the pagination fix itself. Please preserve the old enumerable shape or hide the internal fields before shipping this conversion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/authorization/authorization.ts` around lines 239 - 257, The
AutoPaginatable change made list responses non-enumerable and breaks callers
expecting a plain { object, data, listMetadata } shape; update listPermissions
(and other authz list methods) to return a plain enumerable object instead of
exposing AutoPaginatable internals: either (A) convert the AutoPaginatable
instance into a plain object before returning (e.g., construct and return {
object: 'list', data: autoPaginatable.list, listMetadata:
autoPaginatable.listMetadata, ...options }), or (B) modify AutoPaginatable in
src/common/utils/pagination.ts to expose enumerable data and listMetadata (or
implement a toJSON()/toPlainObject() that returns the original shape) and call
that from listPermissions; reference the listPermissions function and the
AutoPaginatable class in pagination.ts when making the change so the returned
value preserves the old enumerable shape.
Comment on lines
444
to
+529
| async listResourcesForMembership( | ||
| options: ListResourcesForMembershipOptions, | ||
| ): Promise<AuthorizationResourceList> { | ||
| ): Promise<AutoPaginatable<AuthorizationResource>> { | ||
| const { organizationMembershipId } = options; | ||
| const { data } = await this.workos.get<AuthorizationResourceListResponse>( | ||
| `/authorization/organization_memberships/${organizationMembershipId}/resources`, | ||
| { | ||
| query: serializeListResourcesForMembershipOptions(options), | ||
| }, | ||
| const endpoint = `/authorization/organization_memberships/${organizationMembershipId}/resources`; | ||
| const serializedOptions = | ||
| serializeListResourcesForMembershipOptions(options); | ||
| return new AutoPaginatable( | ||
| await fetchAndDeserialize< | ||
| AuthorizationResourceResponse, | ||
| AuthorizationResource | ||
| >( | ||
| this.workos, | ||
| endpoint, | ||
| deserializeAuthorizationResource, | ||
| serializedOptions, | ||
| ), | ||
| (params) => | ||
| fetchAndDeserialize< | ||
| AuthorizationResourceResponse, | ||
| AuthorizationResource | ||
| >(this.workos, endpoint, deserializeAuthorizationResource, params), | ||
| serializedOptions, | ||
| ); | ||
| return { | ||
| object: 'list', | ||
| data: data.data.map(deserializeAuthorizationResource), | ||
| listMetadata: { | ||
| before: data.list_metadata.before, | ||
| after: data.list_metadata.after, | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| async listMembershipsForResource( | ||
| options: ListMembershipsForResourceOptions, | ||
| ): Promise<AuthorizationOrganizationMembershipList> { | ||
| ): Promise<AutoPaginatable<AuthorizationOrganizationMembership>> { | ||
| const { resourceId } = options; | ||
| const { data } = | ||
| await this.workos.get<AuthorizationOrganizationMembershipListResponse>( | ||
| `/authorization/resources/${resourceId}/organization_memberships`, | ||
| { | ||
| query: serializeListMembershipsForResourceOptions(options), | ||
| }, | ||
| ); | ||
| return { | ||
| object: 'list', | ||
| data: data.data.map(deserializeAuthorizationOrganizationMembership), | ||
| listMetadata: { | ||
| before: data.list_metadata.before, | ||
| after: data.list_metadata.after, | ||
| }, | ||
| }; | ||
| const endpoint = `/authorization/resources/${resourceId}/organization_memberships`; | ||
| const serializedOptions = | ||
| serializeListMembershipsForResourceOptions(options); | ||
| return new AutoPaginatable( | ||
| await fetchAndDeserialize< | ||
| AuthorizationOrganizationMembershipResponse, | ||
| AuthorizationOrganizationMembership | ||
| >( | ||
| this.workos, | ||
| endpoint, | ||
| deserializeAuthorizationOrganizationMembership, | ||
| serializedOptions, | ||
| ), | ||
| (params) => | ||
| fetchAndDeserialize< | ||
| AuthorizationOrganizationMembershipResponse, | ||
| AuthorizationOrganizationMembership | ||
| >( | ||
| this.workos, | ||
| endpoint, | ||
| deserializeAuthorizationOrganizationMembership, | ||
| params, | ||
| ), | ||
| serializedOptions, | ||
| ); | ||
| } | ||
|
|
||
| async listMembershipsForResourceByExternalId( | ||
| options: ListMembershipsForResourceByExternalIdOptions, | ||
| ): Promise<AuthorizationOrganizationMembershipList> { | ||
| ): Promise<AutoPaginatable<AuthorizationOrganizationMembership>> { | ||
| const { organizationId, resourceTypeSlug, externalId } = options; | ||
| const { data } = | ||
| await this.workos.get<AuthorizationOrganizationMembershipListResponse>( | ||
| `/authorization/organizations/${organizationId}/resources/${resourceTypeSlug}/${externalId}/organization_memberships`, | ||
| { | ||
| query: serializeListMembershipsForResourceOptions(options), | ||
| }, | ||
| ); | ||
| return { | ||
| object: 'list', | ||
| data: data.data.map(deserializeAuthorizationOrganizationMembership), | ||
| listMetadata: { | ||
| before: data.list_metadata.before, | ||
| after: data.list_metadata.after, | ||
| }, | ||
| }; | ||
| const endpoint = `/authorization/organizations/${organizationId}/resources/${resourceTypeSlug}/${externalId}/organization_memberships`; | ||
| const serializedOptions = | ||
| serializeListMembershipsForResourceOptions(options); | ||
| return new AutoPaginatable( | ||
| await fetchAndDeserialize< | ||
| AuthorizationOrganizationMembershipResponse, | ||
| AuthorizationOrganizationMembership | ||
| >( | ||
| this.workos, | ||
| endpoint, | ||
| deserializeAuthorizationOrganizationMembership, | ||
| serializedOptions, | ||
| ), | ||
| (params) => | ||
| fetchAndDeserialize< | ||
| AuthorizationOrganizationMembershipResponse, | ||
| AuthorizationOrganizationMembership | ||
| >( | ||
| this.workos, | ||
| endpoint, | ||
| deserializeAuthorizationOrganizationMembership, | ||
| params, | ||
| ), | ||
| serializedOptions, | ||
| ); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Strip path identifiers before serializing query options.
These helpers encode organizationMembershipId, resourceId, organizationId, resourceTypeSlug, and externalId into the URL, but still pass the full options bag to the serializers. That makes correctness depend on each serializer silently ignoring non-query fields. Destructure the path params first and serialize only the remaining query options so a later serializer change cannot start leaking duplicate query params.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/authorization/authorization.ts` around lines 444 - 529, The helpers
listResourcesForMembership, listMembershipsForResource, and
listMembershipsForResourceByExternalId currently pass the full options bag
(including path identifiers) into their serializer functions
(serializeListResourcesForMembershipOptions,
serializeListMembershipsForResourceOptions); update each function to destructure
and remove path params first—organizationMembershipId, resourceId,
organizationId, resourceTypeSlug, externalId—and then call the corresponding
serializer with only the remaining query options so path identifiers cannot be
encoded into the query string.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/authorization/authorization.ts`:
- Around line 559-581: The function listEffectivePermissionsByExternalId has a
long single-line destructuring that breaks Prettier rules; split the
destructuring across multiple lines (or extract the incoming options to a named
variable before destructuring) to satisfy line-length formatting and re-run
Prettier. Update the const { organizationMembershipId, organizationId,
resourceTypeSlug, externalId } = options; in
listEffectivePermissionsByExternalId so each property sits on its own line (or
perform const opts = options; const organizationId = opts.organizationId; etc.),
then ensure endpoint, serializedOptions (from
serializeListEffectivePermissionsOptions) and the AutoPaginatable call remain
unchanged functionally and pass lint/format checks.
In
`@src/authorization/interfaces/list-effective-permissions-by-external-id-options.interface.ts`:
- Around line 1-9: Prettier formatting errors are present in the
ListEffectivePermissionsByExternalIdOptions interface; run the formatter
(prettier --write) on this file or reformat so import and interface declarations
meet project style (ensure import spacing, line breaks, and indentation around
the export interface ListEffectivePermissionsByExternalIdOptions and its members
organizationMembershipId, organizationId, resourceTypeSlug, externalId follow
Prettier rules).
🪄 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: a8517858-93dc-412c-9e1d-c7081dc3ea6d
📒 Files selected for processing (8)
src/authorization/authorization.spec.tssrc/authorization/authorization.tssrc/authorization/fixtures/list-effective-permissions.jsonsrc/authorization/interfaces/index.tssrc/authorization/interfaces/list-effective-permissions-by-external-id-options.interface.tssrc/authorization/interfaces/list-effective-permissions-options.interface.tssrc/authorization/serializers/index.tssrc/authorization/serializers/list-effective-permissions-options.serializer.ts
atainter
changed the title
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/authorization/authorization.ts (2)
535-556: 🧹 Nitpick | 🔵 TrivialStrip path identifiers before serializing effective-permissions query options.
Both helpers embed the identifiers in
endpoint, then still pass the fulloptionsbag intoserializeListEffectivePermissionsOptions(...). That currently works only because the serializer ignores non-pagination fields; a later serializer change could start leakingorganizationMembershipId,organizationId,resourceTypeSlug, orexternalIdinto the query string. Destructure...queryOptionsfirst and serialize only those.♻️ Proposed fix
async listEffectivePermissions( options: ListEffectivePermissionsOptions, ): Promise<AutoPaginatable<Permission>> { - const { organizationMembershipId, resourceId } = options; + const { organizationMembershipId, resourceId, ...queryOptions } = options; const endpoint = `/authorization/resources/${resourceId}/organization_memberships/${organizationMembershipId}/permissions`; - const serializedOptions = serializeListEffectivePermissionsOptions(options); + const serializedOptions = + serializeListEffectivePermissionsOptions(queryOptions); return new AutoPaginatable( await fetchAndDeserialize<PermissionResponse, Permission>( this.workos, endpoint, deserializePermission, @@ async listEffectivePermissionsByExternalId( options: ListEffectivePermissionsByExternalIdOptions, ): Promise<AutoPaginatable<Permission>> { const { organizationMembershipId, organizationId, resourceTypeSlug, externalId, + ...queryOptions } = options; const endpoint = `/authorization/organizations/${organizationId}/resources/${resourceTypeSlug}/${externalId}/organization_memberships/${organizationMembershipId}/permissions`; - const serializedOptions = serializeListEffectivePermissionsOptions(options); + const serializedOptions = + serializeListEffectivePermissionsOptions(queryOptions); return new AutoPaginatable( await fetchAndDeserialize<PermissionResponse, Permission>( this.workos, endpoint, deserializePermission,Also applies to: 559-585
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/authorization/authorization.ts` around lines 535 - 556, The serializer is being passed the full options bag including path identifiers which may leak into the query string; in listEffectivePermissions, destructure options to remove organizationMembershipId and resourceId (e.g., const { organizationMembershipId, resourceId, ...queryOptions } = options) and call serializeListEffectivePermissionsOptions(queryOptions) so only query parameters are serialized; apply the same pattern to the other similar method referenced (the block around lines 559-585) and keep endpoint construction using the extracted identifiers (endpoint = `/authorization/resources/${resourceId}/organization_memberships/${organizationMembershipId}/permissions`).
242-260:⚠️ Potential issue | 🟠 MajorReturning
AutoPaginatablehere is a breaking runtime shape change.
AutoPaginatableis a class instance, not the previous plain list object. Insrc/common/utils/pagination.ts,list/optionsare own fields whiledataandlistMetadataare prototype getters, so spreads,Object.keys(), andJSON.stringify()no longer preserve the old{ object, data, listMetadata }shape for this and the other converted authz list methods.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/authorization/authorization.ts` around lines 242 - 260, The method listPermissions currently returns a new AutoPaginatable instance (in function listPermissions), which changes the runtime shape; instead return the original plain list object shape produced by fetchAndDeserialize (so consumers still get { object, data, listMetadata, ... } and spreads/JSON.stringify behave the same). Replace the new AutoPaginatable(...) return with the raw result from fetchAndDeserialize<PermissionResponse, Permission>(this.workos, '/authorization/permissions', deserializePermission, options) (and ensure subsequent paginated caller wrapper uses that raw result or provides a .toPlainObject() helper), and apply the same pattern to the other converted authorization list methods to preserve the previous plain-object API shape.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/authorization/authorization.ts`:
- Around line 535-556: The serializer is being passed the full options bag
including path identifiers which may leak into the query string; in
listEffectivePermissions, destructure options to remove organizationMembershipId
and resourceId (e.g., const { organizationMembershipId, resourceId,
...queryOptions } = options) and call
serializeListEffectivePermissionsOptions(queryOptions) so only query parameters
are serialized; apply the same pattern to the other similar method referenced
(the block around lines 559-585) and keep endpoint construction using the
extracted identifiers (endpoint =
`/authorization/resources/${resourceId}/organization_memberships/${organizationMembershipId}/permissions`).
- Around line 242-260: The method listPermissions currently returns a new
AutoPaginatable instance (in function listPermissions), which changes the
runtime shape; instead return the original plain list object shape produced by
fetchAndDeserialize (so consumers still get { object, data, listMetadata, ... }
and spreads/JSON.stringify behave the same). Replace the new
AutoPaginatable(...) return with the raw result from
fetchAndDeserialize<PermissionResponse, Permission>(this.workos,
'/authorization/permissions', deserializePermission, options) (and ensure
subsequent paginated caller wrapper uses that raw result or provides a
.toPlainObject() helper), and apply the same pattern to the other converted
authorization list methods to preserve the previous plain-object API shape.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5116ea1c-f78a-49a6-bd7d-b252f61337ee
📒 Files selected for processing (2)
src/authorization/authorization.tssrc/authorization/interfaces/list-effective-permissions-by-external-id-options.interface.ts
cmatheson
approved these changes
Apr 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Authorization endpoints weren’t using the shared pagination types, which automatically set order='desc'. Because of this, the authorization methods defaulted to PaginationOrder=normal, causing the next page to return a before cursor instead of after.
Also adds listEffectivePermissions methods.
Summary by CodeRabbit