Skip to content

fix: api list keys zod errors#3702

Merged
chronark merged 7 commits intomainfrom
fix/v2-zod-errors
Jul 31, 2025
Merged

fix: api list keys zod errors#3702
chronark merged 7 commits intomainfrom
fix/v2-zod-errors

Conversation

@Flo4604
Copy link
Member

@Flo4604 Flo4604 commented Jul 31, 2025

What does this PR do?

Fixes # (issue)

If there is not an issue for this, please create one first. This is used to tracking purposes and also helps use understand why this PR exists

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Chore (refactoring code, technical debt, workflow improvements)
  • Enhancement (small improvements)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How should this be tested?

  • Test A
  • Test B

Checklist

Required

  • Filled out the "How to test" section in this PR
  • Read Contributing Guide
  • Self-reviewed my own code
  • Commented on my code in hard-to-understand areas
  • Ran pnpm build
  • Ran pnpm fmt
  • Checked for warnings, there are none
  • Removed all console.logs
  • Merged the latest changes from main onto my branch with git pull origin main
  • My changes don't cause any responsiveness issues

Appreciated

  • If a UI change was made: Added a screen recording or screenshots to this PR
  • Updated the Unkey Docs if changes were necessary

Summary by CodeRabbit

  • Bug Fixes

    • Corrected casing of "Role" schema references in the OpenAPI specification to ensure consistency and proper schema resolution.
    • Updated references to schema filenames in API response specifications to use the correct capitalization.
  • Refactor

    • Removed duplicate struct definitions and consolidated the "Role" entity definition for clarity and maintainability.
    • Modified handling of optional rate limit and permissions data to use pointer fields with omitempty, allowing omission when empty and improving API response clarity.
    • Updated API handlers and tests to correctly handle optional rate limit and permissions data as pointers, ensuring accurate data representation.
    • Adjusted initialization of response fields to assign pointers only when data is present, preventing empty slices from overwriting nil values.
  • Style

    • Removed unnecessary blank lines in benchmark scripts for improved code readability.

@changeset-bot
Copy link

changeset-bot bot commented Jul 31, 2025

⚠️ No Changeset found

Latest commit: 4b89eba

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Jul 31, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
dashboard ⬜️ Ignored (Inspect) Visit Preview Jul 31, 2025 0:14am
engineering ⬜️ Ignored (Inspect) Visit Preview Jul 31, 2025 0:14am

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jul 31, 2025

📝 Walkthrough

Walkthrough

This change modifies the Identity struct to use a pointer for the Ratelimits slice with an omitempty JSON tag, making it optional in serialization. It consolidates duplicate Role struct definitions into a single location earlier in the file. Corresponding OpenAPI schema references to the Role schema were corrected for casing consistency, and the ratelimits and permissions properties were made optional in the OpenAPI Identity and Role schemas respectively. Code handling response struct initialization was updated to initialize slices as pointers and assign them conditionally only when non-empty. Test code was adjusted to correctly handle the pointer to the Ratelimits and Permissions slices. Minor formatting cleanup was also applied.

Changes

Cohort / File(s) Change Summary
Identity struct and Role struct adjustments
go/apps/api/openapi/gen.go
Changed Identity.Ratelimits from slice to pointer to slice with omitempty; removed duplicate Role struct definition and kept one earlier in file; changed Role.Permissions to pointer to slice with omitempty.
OpenAPI schema updates for Identity and Role
go/apps/api/openapi/openapi-generated.yaml, go/apps/api/openapi/spec/common/Identity.yaml, go/apps/api/openapi/spec/common/Role.yaml
Removed ratelimits from required properties in Identity schema; removed permissions from required properties in Role schema; updated all role schema references to Role for casing consistency.
OpenAPI schema file path casing corrections
go/apps/api/openapi/spec/paths/v2/permissions/getRole/V2PermissionsGetRoleResponseData.yaml, go/apps/api/openapi/spec/paths/v2/permissions/listRoles/V2PermissionsListRolesResponseData.yaml
Changed schema references from role.yaml to Role.yaml.
Response struct initialization with pointer slices
go/apps/api/routes/v2_apis_list_keys/handler.go, go/apps/api/routes/v2_keys_get_key/handler.go, go/apps/api/routes/v2_keys_verify_key/handler.go, go/apps/api/routes/v2_keys_whoami/handler.go, go/apps/api/routes/v2_identities_list_identities/handler.go, go/apps/api/routes/v2_identities_update_identity/handler.go
Initialized Permissions, Roles, and Ratelimits fields with pointer to slices only if non-empty; replaced direct slice assignments with conditional pointer assignments.
Role permissions handling refactor
go/apps/api/routes/v2_keys_add_roles/handler.go, go/apps/api/routes/v2_keys_remove_roles/handler.go, go/apps/api/routes/v2_keys_set_roles/handler.go, go/apps/api/routes/v2_permissions_get_role/handler.go, go/apps/api/routes/v2_permissions_list_roles/handler.go
Replaced direct appending to Permissions slices with accumulation in local slices and conditional assignment of pointers to these slices, using helper function ptr.P.
Test code updates for pointer dereferencing
go/apps/api/routes/v2_identities_list_identities/200_test.go, go/apps/api/routes/v2_identities_update_identity/200_test.go, go/apps/api/routes/v2_permissions_get_role/200_test.go, go/apps/api/routes/v2_permissions_list_roles/200_test.go
Updated tests to dereference Ratelimits and Permissions pointers before accessing length and elements; changed nil and empty checks accordingly.
Formatting cleanup
go/benchmarks/keyverify.js
Removed two extraneous blank lines; no functional changes.

Sequence Diagram(s)

sequenceDiagram
    participant Handler as API Handler
    participant Response as Response Struct

    Handler->>Response: Initialize Permissions, Roles, Ratelimits as nil or empty pointer slices
    Handler->>Handler: Populate permissions, roles, ratelimits slices
    alt If permissions slice not empty
        Handler->>Response: Assign pointer to Permissions slice
    end
    alt If roles slice not empty
        Handler->>Response: Assign pointer to Roles slice
    end
    alt If ratelimits slice not empty
        Handler->>Response: Assign pointer to Ratelimits slice
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~15 minutes

Suggested labels

Core Team

Suggested reviewers

  • perkinsjr
  • imeyer
  • mcstepp
  • MichaelUnkey
  • ogzhanolguncu
  • chronark

Note

⚡️ Unit Test Generation is now available in beta!

Learn more here, or try it out under "Finishing Touches" below.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4bcd0d0 and 4b89eba.

📒 Files selected for processing (11)
  • go/apps/api/openapi/gen.go (2 hunks)
  • go/apps/api/openapi/openapi-generated.yaml (2 hunks)
  • go/apps/api/openapi/spec/common/Role.yaml (0 hunks)
  • go/apps/api/routes/v2_identities_list_identities/200_test.go (2 hunks)
  • go/apps/api/routes/v2_keys_add_roles/handler.go (4 hunks)
  • go/apps/api/routes/v2_keys_remove_roles/handler.go (3 hunks)
  • go/apps/api/routes/v2_keys_set_roles/handler.go (3 hunks)
  • go/apps/api/routes/v2_permissions_get_role/200_test.go (2 hunks)
  • go/apps/api/routes/v2_permissions_get_role/handler.go (3 hunks)
  • go/apps/api/routes/v2_permissions_list_roles/200_test.go (1 hunks)
  • go/apps/api/routes/v2_permissions_list_roles/handler.go (2 hunks)
💤 Files with no reviewable changes (1)
  • go/apps/api/openapi/spec/common/Role.yaml
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: Flo4604
PR: unkeyed/unkey#3421
File: go/apps/api/openapi/openapi.yaml:196-200
Timestamp: 2025-07-03T05:58:10.699Z
Learning: In the Unkey codebase, OpenAPI 3.1 is used, which allows sibling keys (such as `description`) alongside `$ref` in schema objects. Do not flag this as an error in future reviews.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3324
File: apps/dashboard/app/(app)/authorization/roles/components/table/components/actions/keys-table-action.popover.constants.tsx:17-18
Timestamp: 2025-06-19T11:48:05.070Z
Learning: In the authorization roles refactor, the RoleBasic type uses `roleId` as the property name for the role identifier, not `id`. This is consistent throughout the codebase in apps/dashboard/lib/trpc/routers/authorization/roles/query.ts.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3661
File: go/apps/api/routes/v2_identities_update_identity/handler.go:115-119
Timestamp: 2025-07-28T11:47:43.144Z
Learning: The v2 update identity endpoint (go/apps/api/routes/v2_identities_update_identity/handler.go) intentionally uses `ExternalId` field instead of the unified `Identity` field used in other v2 identity endpoints. This is because the update endpoint needs to both find by externalId and potentially update the externalId value, making the specific field name more appropriate than the generic `Identity` field.
📚 Learning: in the authorization roles refactor, the rolebasic type uses `roleid` as the property name for the r...
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3324
File: apps/dashboard/app/(app)/authorization/roles/components/table/components/actions/keys-table-action.popover.constants.tsx:17-18
Timestamp: 2025-06-19T11:48:05.070Z
Learning: In the authorization roles refactor, the RoleBasic type uses `roleId` as the property name for the role identifier, not `id`. This is consistent throughout the codebase in apps/dashboard/lib/trpc/routers/authorization/roles/query.ts.

Applied to files:

  • go/apps/api/routes/v2_permissions_list_roles/200_test.go
  • go/apps/api/routes/v2_permissions_get_role/200_test.go
  • go/apps/api/openapi/gen.go
  • go/apps/api/openapi/openapi-generated.yaml
📚 Learning: in the unkey codebase, role and permission names are validated at the openapi schema layer with stri...
Learnt from: chronark
PR: unkeyed/unkey#3560
File: go/apps/api/routes/v2_keys_create_key/handler.go:468-581
Timestamp: 2025-07-15T14:47:20.490Z
Learning: In the Unkey codebase, role and permission names are validated at the OpenAPI schema layer with strict regex patterns: role names must match "^[a-zA-Z][a-zA-Z0-9_-]*$" (start with letter, followed by letters/numbers/underscores/hyphens) and permission names must match "^[a-zA-Z0-9_]+$" (letters, numbers, underscores only). This validation occurs during zen.BindBody call before handlers run, preventing malicious or improperly formatted names from reaching auto-creation logic.

Applied to files:

  • go/apps/api/routes/v2_permissions_list_roles/200_test.go
  • go/apps/api/routes/v2_permissions_list_roles/handler.go
  • go/apps/api/routes/v2_keys_set_roles/handler.go
  • go/apps/api/routes/v2_keys_remove_roles/handler.go
  • go/apps/api/routes/v2_permissions_get_role/handler.go
  • go/apps/api/routes/v2_keys_add_roles/handler.go
  • go/apps/api/routes/v2_permissions_get_role/200_test.go
  • go/apps/api/openapi/gen.go
  • go/apps/api/openapi/openapi-generated.yaml
📚 Learning: go/deploy/metald cannot currently import helpers from go/pkg/db because it is not yet part of the ma...
Learnt from: chronark
PR: unkeyed/unkey#3560
File: go/deploy/metald/internal/database/repository.go:0-0
Timestamp: 2025-07-15T14:59:30.212Z
Learning: go/deploy/metald cannot currently import helpers from go/pkg/db because it is not yet part of the main Go module; avoid suggesting such imports until the modules are unified.

Applied to files:

  • go/apps/api/routes/v2_keys_set_roles/handler.go
  • go/apps/api/routes/v2_keys_remove_roles/handler.go
  • go/apps/api/routes/v2_permissions_get_role/handler.go
  • go/apps/api/routes/v2_keys_add_roles/handler.go
📚 Learning: in go packages, variables defined in one file within a package (like `latencybuckets` and `constlabe...
Learnt from: Flo4604
PR: unkeyed/unkey#3606
File: go/pkg/prometheus/metrics/database.go:29-30
Timestamp: 2025-07-16T10:06:35.397Z
Learning: In Go packages, variables defined in one file within a package (like `latencyBuckets` and `constLabels` in go/pkg/prometheus/metrics/http.go) are accessible from other files in the same package without requiring imports. This is a common pattern for sharing configuration across multiple files within a package.

Applied to files:

  • go/apps/api/routes/v2_keys_set_roles/handler.go
  • go/apps/api/routes/v2_keys_remove_roles/handler.go
  • go/apps/api/routes/v2_permissions_get_role/handler.go
  • go/apps/api/routes/v2_keys_add_roles/handler.go
📚 Learning: the v2 update identity endpoint (go/apps/api/routes/v2_identities_update_identity/handler.go) intent...
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3661
File: go/apps/api/routes/v2_identities_update_identity/handler.go:115-119
Timestamp: 2025-07-28T11:47:43.144Z
Learning: The v2 update identity endpoint (go/apps/api/routes/v2_identities_update_identity/handler.go) intentionally uses `ExternalId` field instead of the unified `Identity` field used in other v2 identity endpoints. This is because the update endpoint needs to both find by externalId and potentially update the externalId value, making the specific field name more appropriate than the generic `Identity` field.

Applied to files:

  • go/apps/api/routes/v2_identities_list_identities/200_test.go
  • go/apps/api/openapi/gen.go
  • go/apps/api/openapi/openapi-generated.yaml
📚 Learning: in the rate limit test files (e.g., `apps/api/src/routes/v1_ratelimit_getoverride.happy.test.ts`), u...
Learnt from: chronark
PR: unkeyed/unkey#2126
File: apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts:36-36
Timestamp: 2024-11-13T19:06:36.786Z
Learning: In the rate limit test files (e.g., `apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts`), URL parameters like `namespaceId` and `identifier` do not need to be URL-encoded in the test code because the values used are always considered safe within the test environment.

Applied to files:

  • go/apps/api/routes/v2_identities_list_identities/200_test.go
📚 Learning: in `apps/api/src/pkg/keys/service.ts`, `ratelimitasync` is a table relation, not a column selection....
Learnt from: chronark
PR: unkeyed/unkey#2294
File: apps/api/src/pkg/keys/service.ts:268-271
Timestamp: 2024-10-20T07:05:55.471Z
Learning: In `apps/api/src/pkg/keys/service.ts`, `ratelimitAsync` is a table relation, not a column selection. When querying, ensure that table relations are included appropriately, not as columns.

Applied to files:

  • go/apps/api/routes/v2_identities_list_identities/200_test.go
📚 Learning: in `authorization/roles/[roleid]/update-role.tsx`, the tag `role-${role.id}` is revalidated after up...
Learnt from: AkshayBandi027
PR: unkeyed/unkey#2215
File: apps/dashboard/app/(app)/@breadcrumb/authorization/roles/[roleId]/page.tsx:28-29
Timestamp: 2024-10-08T15:33:04.290Z
Learning: In `authorization/roles/[roleId]/update-role.tsx`, the tag `role-${role.id}` is revalidated after updating a role to ensure that the caching mechanism is properly handled for roles.

Applied to files:

  • go/apps/api/openapi/gen.go
  • go/apps/api/openapi/openapi-generated.yaml
📚 Learning: in the unkey codebase, input validation for api endpoints is primarily handled through openapi schem...
Learnt from: Flo4604
PR: unkeyed/unkey#2955
File: go/apps/api/routes/v2_identities_create_identity/handler.go:162-202
Timestamp: 2025-03-19T09:25:59.751Z
Learning: In the Unkey codebase, input validation for API endpoints is primarily handled through OpenAPI schema validation, which occurs before requests reach the handler code. For example, in the identities.createIdentity endpoint, minimum values for ratelimit duration and limit are defined in the OpenAPI schema rather than duplicating these checks in the handler.

Applied to files:

  • go/apps/api/openapi/openapi-generated.yaml
📚 Learning: in the unkey codebase, openapi 3.1 is used, which allows sibling keys (such as `description`) alongs...
Learnt from: Flo4604
PR: unkeyed/unkey#3421
File: go/apps/api/openapi/openapi.yaml:196-200
Timestamp: 2025-07-03T05:58:10.699Z
Learning: In the Unkey codebase, OpenAPI 3.1 is used, which allows sibling keys (such as `description`) alongside `$ref` in schema objects. Do not flag this as an error in future reviews.

Applied to files:

  • go/apps/api/openapi/openapi-generated.yaml
🧬 Code Graph Analysis (4)
go/apps/api/routes/v2_permissions_list_roles/handler.go (2)
go/apps/api/openapi/gen.go (1)
  • Permission (292-312)
go/pkg/ptr/pointer.go (1)
  • P (49-51)
go/apps/api/routes/v2_keys_remove_roles/handler.go (3)
go/apps/api/openapi/gen.go (1)
  • Permission (292-312)
go/pkg/db/models_generated.go (1)
  • Permission (746-754)
go/pkg/ptr/pointer.go (1)
  • P (49-51)
go/apps/api/routes/v2_permissions_get_role/handler.go (2)
go/apps/api/openapi/gen.go (1)
  • Permission (292-312)
go/pkg/ptr/pointer.go (1)
  • P (49-51)
go/apps/api/routes/v2_keys_add_roles/handler.go (3)
go/apps/api/openapi/gen.go (1)
  • Permission (292-312)
go/pkg/db/models_generated.go (1)
  • Permission (746-754)
go/pkg/ptr/pointer.go (1)
  • P (49-51)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: Test Go API Local / Test
  • GitHub Check: Test Agent Local / test_agent_local
  • GitHub Check: Test API / API Test Local
  • GitHub Check: Build / Build
  • GitHub Check: Test Packages / Test
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (15)
go/apps/api/routes/v2_permissions_list_roles/200_test.go (1)

136-138: LGTM - Correct pointer handling in tests

The test correctly handles the updated Permissions field as a pointer to slice by dereferencing it for length checks and iteration. This aligns with the API schema changes.

go/apps/api/routes/v2_permissions_list_roles/handler.go (1)

108-127: Verify empty-array vs. omitted-field behavior for permissions

The handler in go/apps/api/routes/v2_permissions_list_roles/handler.go (lines 108–127) only assigns roleResponse.Permissions = ptr.P(perms) when len(perms) > 0, which causes the field to be omitted (nil) in JSON when there are no permissions. Your PR description, however, states that empty arrays should be returned.

Please confirm the intended OpenAPI schema for the permissions property on the Role response:

  • If the schema requires an empty array when no permissions are present, remove the if len(perms) > 0 guard so that you always do roleResponse.Permissions = ptr.P(perms).
  • If omitting the field when empty is correct (and the schema does not require it), the current implementation may remain.

• File: go/apps/api/routes/v2_permissions_list_roles/handler.go
• Lines: 108–127

go/apps/api/routes/v2_keys_remove_roles/handler.go (1)

19-19: LGTM - Required import added

The ptr package import is correctly added to support the pointer assignment pattern.

go/apps/api/routes/v2_permissions_get_role/handler.go (1)

14-14: LGTM - Required import added

The ptr package import is correctly added for the pointer assignment functionality.

go/apps/api/routes/v2_keys_add_roles/handler.go (1)

20-20: LGTM - Required import added

The ptr package import is correctly added to support pointer operations.

go/apps/api/routes/v2_permissions_get_role/200_test.go (3)

109-109: Add nil check before dereferencing Permissions pointer.

The test correctly handles the pointer semantics, but dereferencing *role.Permissions without a nil check could cause a panic if the field is nil.

Add a nil check for safer pointer handling:

+		require.NotNil(t, role.Permissions)
 		require.Len(t, *role.Permissions, 2)

113-113: Pointer dereference is safe here due to previous length check.

The dereference in the range loop is safe since we've already verified the slice has length 2 in the previous assertion.


163-163: LGTM! Correct handling of nil pointer for empty permissions.

The test correctly checks for nil instead of an empty slice, which aligns with the new pointer semantics where roles without permissions have a nil Permissions field.

go/apps/api/routes/v2_keys_set_roles/handler.go (2)

20-20: LGTM! Import added for ptr package.

The ptr package import is correctly added to support the pointer assignment pattern used later in the handler.


292-310: Excellent implementation of pointer slice pattern.

The code correctly implements the pattern for optional slice fields:

  1. Builds permissions in a local slice (perms)
  2. Only assigns as a pointer when non-empty using ptr.P(perms)
  3. This ensures proper JSON serialization with omitempty behavior

This approach maintains consistency with the broader refactor to use pointer slices for optional fields like permissions and ratelimits.

go/apps/api/routes/v2_identities_list_identities/200_test.go (1)

363-363: LGTM! Appropriate nil check for Ratelimits field.

The test correctly checks that Ratelimits is not nil, which aligns with the new pointer semantics for optional slice fields.

go/apps/api/openapi/openapi-generated.yaml (2)

2826-2829: Same fix in array items – LGTM

Array items now points at the correctly-cased Role schema. No further changes required here.


2811-2814: Schema ref casing consistency verified
I ran rg --fixed-strings '"#/components/schemas/role"' go/apps/api/openapi and confirmed there are no lowercase #/components/schemas/role references left—everything now uses #/components/schemas/Role. LGTM!

go/apps/api/openapi/gen.go (2)

149-149: LGTM! Improved JSON serialization for optional ratelimits.

Converting Ratelimits to a pointer slice with omitempty correctly makes this field optional in JSON responses. When nil, the field will be omitted entirely rather than serialized as an empty array, which should resolve the Zod validation errors mentioned in the PR objectives.


417-442: LGTM! Proper optional serialization for permissions.

The Role struct now correctly uses *[]Permission with omitempty for the Permissions field. This addresses the previous review concern about nil slices serializing as null instead of being omitted. Now when permissions are nil/empty, the field will be excluded from JSON responses entirely, which should resolve Zod validation issues and provide cleaner API responses.

This change aligns well with the PR objective of fixing Zod validation errors in the API list keys functionality.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/v2-zod-errors

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 31, 2025

Thank you for following the naming conventions for pull request titles! 🙏

Copy link
Collaborator

@chronark chronark left a comment

Choose a reason for hiding this comment

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

so we return empty arrays now? that's the idea right? The pr had no description unfortunately

@Flo4604
Copy link
Member Author

Flo4604 commented Jul 31, 2025

my bad, but yes we are just returning empty arrays now

Copy link
Collaborator

well if we do that, we should not make them optional anymore

Copy link
Contributor

@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: 4

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dbe030b and 29d605f.

📒 Files selected for processing (11)
  • go/apps/api/openapi/gen.go (2 hunks)
  • go/apps/api/openapi/openapi-generated.yaml (2 hunks)
  • go/apps/api/openapi/spec/common/Identity.yaml (0 hunks)
  • go/apps/api/routes/v2_apis_list_keys/handler.go (4 hunks)
  • go/apps/api/routes/v2_identities_list_identities/200_test.go (1 hunks)
  • go/apps/api/routes/v2_identities_list_identities/handler.go (1 hunks)
  • go/apps/api/routes/v2_identities_update_identity/200_test.go (5 hunks)
  • go/apps/api/routes/v2_identities_update_identity/handler.go (2 hunks)
  • go/apps/api/routes/v2_keys_get_key/handler.go (1 hunks)
  • go/apps/api/routes/v2_keys_verify_key/handler.go (4 hunks)
  • go/apps/api/routes/v2_keys_whoami/handler.go (1 hunks)
💤 Files with no reviewable changes (1)
  • go/apps/api/openapi/spec/common/Identity.yaml
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: Flo4604
PR: unkeyed/unkey#3421
File: go/apps/api/openapi/openapi.yaml:196-200
Timestamp: 2025-07-03T05:58:10.699Z
Learning: In the Unkey codebase, OpenAPI 3.1 is used, which allows sibling keys (such as `description`) alongside `$ref` in schema objects. Do not flag this as an error in future reviews.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3661
File: go/apps/api/routes/v2_identities_update_identity/handler.go:115-119
Timestamp: 2025-07-28T11:47:43.144Z
Learning: The v2 update identity endpoint (go/apps/api/routes/v2_identities_update_identity/handler.go) intentionally uses `ExternalId` field instead of the unified `Identity` field used in other v2 identity endpoints. This is because the update endpoint needs to both find by externalId and potentially update the externalId value, making the specific field name more appropriate than the generic `Identity` field.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3324
File: apps/dashboard/app/(app)/authorization/roles/components/table/components/actions/keys-table-action.popover.constants.tsx:17-18
Timestamp: 2025-06-19T11:48:05.070Z
Learning: In the authorization roles refactor, the RoleBasic type uses `roleId` as the property name for the role identifier, not `id`. This is consistent throughout the codebase in apps/dashboard/lib/trpc/routers/authorization/roles/query.ts.
go/apps/api/routes/v2_identities_list_identities/handler.go (2)

Learnt from: ogzhanolguncu
PR: #3661
File: go/apps/api/routes/v2_identities_update_identity/handler.go:115-119
Timestamp: 2025-07-28T11:47:43.144Z
Learning: The v2 update identity endpoint (go/apps/api/routes/v2_identities_update_identity/handler.go) intentionally uses ExternalId field instead of the unified Identity field used in other v2 identity endpoints. This is because the update endpoint needs to both find by externalId and potentially update the externalId value, making the specific field name more appropriate than the generic Identity field.

Learnt from: chronark
PR: #2544
File: apps/api/src/pkg/env.ts:4-6
Timestamp: 2024-10-23T12:05:31.121Z
Learning: The cloudflareRatelimiter type definition in apps/api/src/pkg/env.ts should not have its interface changed; it should keep the limit method returning Promise<{ success: boolean }> without additional error properties.

go/apps/api/routes/v2_identities_list_identities/200_test.go (3)

Learnt from: ogzhanolguncu
PR: #3661
File: go/apps/api/routes/v2_identities_update_identity/handler.go:115-119
Timestamp: 2025-07-28T11:47:43.144Z
Learning: The v2 update identity endpoint (go/apps/api/routes/v2_identities_update_identity/handler.go) intentionally uses ExternalId field instead of the unified Identity field used in other v2 identity endpoints. This is because the update endpoint needs to both find by externalId and potentially update the externalId value, making the specific field name more appropriate than the generic Identity field.

Learnt from: chronark
PR: #2126
File: apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts:36-36
Timestamp: 2024-11-13T19:06:36.786Z
Learning: In the rate limit test files (e.g., apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts), URL parameters like namespaceId and identifier do not need to be URL-encoded in the test code because the values used are always considered safe within the test environment.

Learnt from: chronark
PR: #2294
File: apps/api/src/pkg/keys/service.ts:268-271
Timestamp: 2024-10-20T07:05:55.471Z
Learning: In apps/api/src/pkg/keys/service.ts, ratelimitAsync is a table relation, not a column selection. When querying, ensure that table relations are included appropriately, not as columns.

go/apps/api/routes/v2_keys_whoami/handler.go (3)

Learnt from: Flo4604
PR: #2955
File: go/apps/api/routes/v2_identities_create_identity/handler.go:162-202
Timestamp: 2025-03-19T09:25:59.751Z
Learning: In the Unkey codebase, input validation for API endpoints is primarily handled through OpenAPI schema validation, which occurs before requests reach the handler code. For example, in the identities.createIdentity endpoint, minimum values for ratelimit duration and limit are defined in the OpenAPI schema rather than duplicating these checks in the handler.

Learnt from: chronark
PR: #2294
File: apps/api/src/pkg/keys/service.ts:268-271
Timestamp: 2024-10-20T07:05:55.471Z
Learning: In apps/api/src/pkg/keys/service.ts, ratelimitAsync is a table relation, not a column selection. When querying, ensure that table relations are included appropriately, not as columns.

Learnt from: ogzhanolguncu
PR: #3661
File: go/apps/api/routes/v2_identities_update_identity/handler.go:115-119
Timestamp: 2025-07-28T11:47:43.144Z
Learning: The v2 update identity endpoint (go/apps/api/routes/v2_identities_update_identity/handler.go) intentionally uses ExternalId field instead of the unified Identity field used in other v2 identity endpoints. This is because the update endpoint needs to both find by externalId and potentially update the externalId value, making the specific field name more appropriate than the generic Identity field.

go/apps/api/routes/v2_identities_update_identity/200_test.go (2)

Learnt from: ogzhanolguncu
PR: #3661
File: go/apps/api/routes/v2_identities_update_identity/handler.go:115-119
Timestamp: 2025-07-28T11:47:43.144Z
Learning: The v2 update identity endpoint (go/apps/api/routes/v2_identities_update_identity/handler.go) intentionally uses ExternalId field instead of the unified Identity field used in other v2 identity endpoints. This is because the update endpoint needs to both find by externalId and potentially update the externalId value, making the specific field name more appropriate than the generic Identity field.

Learnt from: chronark
PR: #2126
File: apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts:36-36
Timestamp: 2024-11-13T19:06:36.786Z
Learning: In the rate limit test files (e.g., apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts), URL parameters like namespaceId and identifier do not need to be URL-encoded in the test code because the values used are always considered safe within the test environment.

go/apps/api/routes/v2_keys_get_key/handler.go (3)

Learnt from: chronark
PR: #2294
File: apps/api/src/pkg/keys/service.ts:268-271
Timestamp: 2024-10-20T07:05:55.471Z
Learning: In apps/api/src/pkg/keys/service.ts, ratelimitAsync is a table relation, not a column selection. When querying, ensure that table relations are included appropriately, not as columns.

Learnt from: ogzhanolguncu
PR: #3661
File: go/apps/api/routes/v2_identities_update_identity/handler.go:115-119
Timestamp: 2025-07-28T11:47:43.144Z
Learning: The v2 update identity endpoint (go/apps/api/routes/v2_identities_update_identity/handler.go) intentionally uses ExternalId field instead of the unified Identity field used in other v2 identity endpoints. This is because the update endpoint needs to both find by externalId and potentially update the externalId value, making the specific field name more appropriate than the generic Identity field.

Learnt from: Flo4604
PR: #2955
File: go/apps/api/routes/v2_identities_create_identity/handler.go:162-202
Timestamp: 2025-03-19T09:25:59.751Z
Learning: In the Unkey codebase, input validation for API endpoints is primarily handled through OpenAPI schema validation, which occurs before requests reach the handler code. For example, in the identities.createIdentity endpoint, minimum values for ratelimit duration and limit are defined in the OpenAPI schema rather than duplicating these checks in the handler.

go/apps/api/routes/v2_keys_verify_key/handler.go (2)

Learnt from: ogzhanolguncu
PR: #3661
File: go/apps/api/routes/v2_identities_update_identity/handler.go:115-119
Timestamp: 2025-07-28T11:47:43.144Z
Learning: The v2 update identity endpoint (go/apps/api/routes/v2_identities_update_identity/handler.go) intentionally uses ExternalId field instead of the unified Identity field used in other v2 identity endpoints. This is because the update endpoint needs to both find by externalId and potentially update the externalId value, making the specific field name more appropriate than the generic Identity field.

Learnt from: Flo4604
PR: #2955
File: go/apps/api/routes/v2_identities_create_identity/handler.go:162-202
Timestamp: 2025-03-19T09:25:59.751Z
Learning: In the Unkey codebase, input validation for API endpoints is primarily handled through OpenAPI schema validation, which occurs before requests reach the handler code. For example, in the identities.createIdentity endpoint, minimum values for ratelimit duration and limit are defined in the OpenAPI schema rather than duplicating these checks in the handler.

go/apps/api/routes/v2_identities_update_identity/handler.go (4)

Learnt from: ogzhanolguncu
PR: #3661
File: go/apps/api/routes/v2_identities_update_identity/handler.go:115-119
Timestamp: 2025-07-28T11:47:43.144Z
Learning: The v2 update identity endpoint (go/apps/api/routes/v2_identities_update_identity/handler.go) intentionally uses ExternalId field instead of the unified Identity field used in other v2 identity endpoints. This is because the update endpoint needs to both find by externalId and potentially update the externalId value, making the specific field name more appropriate than the generic Identity field.

Learnt from: Flo4604
PR: #2955
File: go/apps/api/routes/v2_identities_create_identity/handler.go:162-202
Timestamp: 2025-03-19T09:25:59.751Z
Learning: In the Unkey codebase, input validation for API endpoints is primarily handled through OpenAPI schema validation, which occurs before requests reach the handler code. For example, in the identities.createIdentity endpoint, minimum values for ratelimit duration and limit are defined in the OpenAPI schema rather than duplicating these checks in the handler.

Learnt from: chronark
PR: #3560
File: go/deploy/metald/internal/database/repository.go:0-0
Timestamp: 2025-07-15T14:59:30.212Z
Learning: go/deploy/metald cannot currently import helpers from go/pkg/db because it is not yet part of the main Go module; avoid suggesting such imports until the modules are unified.

Learnt from: Flo4604
PR: #3606
File: go/pkg/prometheus/metrics/database.go:29-30
Timestamp: 2025-07-16T10:06:35.397Z
Learning: In Go packages, variables defined in one file within a package (like latencyBuckets and constLabels in go/pkg/prometheus/metrics/http.go) are accessible from other files in the same package without requiring imports. This is a common pattern for sharing configuration across multiple files within a package.

go/apps/api/routes/v2_apis_list_keys/handler.go (4)

Learnt from: chronark
PR: #3560
File: go/apps/api/routes/v2_keys_create_key/handler.go:468-581
Timestamp: 2025-07-15T14:47:20.490Z
Learning: In the Unkey codebase, role and permission names are validated at the OpenAPI schema layer with strict regex patterns: role names must match "^[a-zA-Z][a-zA-Z0-9_-]*$" (start with letter, followed by letters/numbers/underscores/hyphens) and permission names must match "^[a-zA-Z0-9_]+$" (letters, numbers, underscores only). This validation occurs during zen.BindBody call before handlers run, preventing malicious or improperly formatted names from reaching auto-creation logic.

Learnt from: Flo4604
PR: #2955
File: go/apps/api/routes/v2_identities_create_identity/handler.go:162-202
Timestamp: 2025-03-19T09:25:59.751Z
Learning: In the Unkey codebase, input validation for API endpoints is primarily handled through OpenAPI schema validation, which occurs before requests reach the handler code. For example, in the identities.createIdentity endpoint, minimum values for ratelimit duration and limit are defined in the OpenAPI schema rather than duplicating these checks in the handler.

Learnt from: chronark
PR: #3560
File: go/apps/api/routes/v2_keys_create_key/handler.go:353-466
Timestamp: 2025-07-15T14:25:05.608Z
Learning: In the Unkey codebase, input validation for API endpoints is handled at the OpenAPI schema layer, which validates request fields like permission slugs (pattern: "^[a-zA-Z0-9_]+$", length: 1-100 characters) before requests reach the handler code. This validation occurs during the zen.BindBody call in handlers.

Learnt from: ogzhanolguncu
PR: #3661
File: go/apps/api/routes/v2_identities_update_identity/handler.go:115-119
Timestamp: 2025-07-28T11:47:43.144Z
Learning: The v2 update identity endpoint (go/apps/api/routes/v2_identities_update_identity/handler.go) intentionally uses ExternalId field instead of the unified Identity field used in other v2 identity endpoints. This is because the update endpoint needs to both find by externalId and potentially update the externalId value, making the specific field name more appropriate than the generic Identity field.

go/apps/api/openapi/gen.go (3)

Learnt from: ogzhanolguncu
PR: #3661
File: go/apps/api/routes/v2_identities_update_identity/handler.go:115-119
Timestamp: 2025-07-28T11:47:43.144Z
Learning: The v2 update identity endpoint (go/apps/api/routes/v2_identities_update_identity/handler.go) intentionally uses ExternalId field instead of the unified Identity field used in other v2 identity endpoints. This is because the update endpoint needs to both find by externalId and potentially update the externalId value, making the specific field name more appropriate than the generic Identity field.

Learnt from: ogzhanolguncu
PR: #3324
File: apps/dashboard/app/(app)/authorization/roles/components/table/components/actions/keys-table-action.popover.constants.tsx:17-18
Timestamp: 2025-06-19T11:48:05.070Z
Learning: In the authorization roles refactor, the RoleBasic type uses roleId as the property name for the role identifier, not id. This is consistent throughout the codebase in apps/dashboard/lib/trpc/routers/authorization/roles/query.ts.

Learnt from: chronark
PR: #3560
File: go/apps/api/routes/v2_keys_create_key/handler.go:468-581
Timestamp: 2025-07-15T14:47:20.490Z
Learning: In the Unkey codebase, role and permission names are validated at the OpenAPI schema layer with strict regex patterns: role names must match "^[a-zA-Z][a-zA-Z0-9_-]*$" (start with letter, followed by letters/numbers/underscores/hyphens) and permission names must match "^[a-zA-Z0-9_]+$" (letters, numbers, underscores only). This validation occurs during zen.BindBody call before handlers run, preventing malicious or improperly formatted names from reaching auto-creation logic.

go/apps/api/openapi/openapi-generated.yaml (6)

Learnt from: ogzhanolguncu
PR: #3324
File: apps/dashboard/app/(app)/authorization/roles/components/table/components/actions/keys-table-action.popover.constants.tsx:17-18
Timestamp: 2025-06-19T11:48:05.070Z
Learning: In the authorization roles refactor, the RoleBasic type uses roleId as the property name for the role identifier, not id. This is consistent throughout the codebase in apps/dashboard/lib/trpc/routers/authorization/roles/query.ts.

Learnt from: ogzhanolguncu
PR: #3661
File: go/apps/api/routes/v2_identities_update_identity/handler.go:115-119
Timestamp: 2025-07-28T11:47:43.144Z
Learning: The v2 update identity endpoint (go/apps/api/routes/v2_identities_update_identity/handler.go) intentionally uses ExternalId field instead of the unified Identity field used in other v2 identity endpoints. This is because the update endpoint needs to both find by externalId and potentially update the externalId value, making the specific field name more appropriate than the generic Identity field.

Learnt from: Flo4604
PR: #2955
File: go/apps/api/routes/v2_identities_create_identity/handler.go:162-202
Timestamp: 2025-03-19T09:25:59.751Z
Learning: In the Unkey codebase, input validation for API endpoints is primarily handled through OpenAPI schema validation, which occurs before requests reach the handler code. For example, in the identities.createIdentity endpoint, minimum values for ratelimit duration and limit are defined in the OpenAPI schema rather than duplicating these checks in the handler.

Learnt from: chronark
PR: #3560
File: go/apps/api/routes/v2_keys_create_key/handler.go:468-581
Timestamp: 2025-07-15T14:47:20.490Z
Learning: In the Unkey codebase, role and permission names are validated at the OpenAPI schema layer with strict regex patterns: role names must match "^[a-zA-Z][a-zA-Z0-9_-]*$" (start with letter, followed by letters/numbers/underscores/hyphens) and permission names must match "^[a-zA-Z0-9_]+$" (letters, numbers, underscores only). This validation occurs during zen.BindBody call before handlers run, preventing malicious or improperly formatted names from reaching auto-creation logic.

Learnt from: Flo4604
PR: #3421
File: go/apps/api/openapi/openapi.yaml:196-200
Timestamp: 2025-07-03T05:58:10.699Z
Learning: In the Unkey codebase, OpenAPI 3.1 is used, which allows sibling keys (such as description) alongside $ref in schema objects. Do not flag this as an error in future reviews.

Learnt from: AkshayBandi027
PR: #2215
File: apps/dashboard/app/(app)/@breadcrumb/authorization/roles/[roleId]/page.tsx:28-29
Timestamp: 2024-10-08T15:33:04.290Z
Learning: In authorization/roles/[roleId]/update-role.tsx, the tag role-${role.id} is revalidated after updating a role to ensure that the caching mechanism is properly handled for roles.

🧬 Code Graph Analysis (5)
go/apps/api/routes/v2_identities_list_identities/handler.go (1)
go/pkg/ptr/pointer.go (1)
  • P (49-51)
go/apps/api/routes/v2_keys_whoami/handler.go (2)
go/apps/api/openapi/gen.go (2)
  • RatelimitResponse (400-415)
  • Identity (141-150)
go/pkg/ptr/pointer.go (1)
  • P (49-51)
go/apps/api/routes/v2_identities_update_identity/handler.go (2)
go/apps/api/openapi/gen.go (2)
  • Identity (141-150)
  • Meta (259-262)
go/pkg/ptr/pointer.go (1)
  • P (49-51)
go/apps/api/routes/v2_apis_list_keys/handler.go (2)
go/pkg/ptr/pointer.go (1)
  • P (49-51)
go/apps/api/openapi/gen.go (2)
  • RatelimitResponse (400-415)
  • Identity (141-150)
go/apps/api/openapi/gen.go (3)
apps/dashboard/lib/trpc/routers/api/keys/query-api-keys/schema.ts (1)
  • Ratelimits (32-32)
internal/ui/src/components/id.tsx (1)
  • Id (22-69)
go/pkg/db/models_generated.go (1)
  • Permission (746-754)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
  • GitHub Check: Test Go API Local / Test
  • GitHub Check: Test Agent Local / test_agent_local
  • GitHub Check: Test API / API Test Local
  • GitHub Check: Build / Build
  • GitHub Check: Test Packages / Test
  • GitHub Check: autofix
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (19)
go/apps/api/routes/v2_identities_list_identities/handler.go (1)

129-135: LGTM! Proper implementation of optional ratelimits.

The conditional assignment of Ratelimits as a pointer only when non-empty correctly implements the optional field pattern. This ensures that empty ratelimit arrays are omitted from JSON serialization rather than being included as empty arrays.

go/apps/api/routes/v2_keys_get_key/handler.go (1)

279-292: LGTM! Correct handling of pointer-based ratelimits.

The changes properly handle the new pointer semantics for Identity.Ratelimits:

  1. Uses a local slice to accumulate ratelimits safely
  2. Conditionally assigns a pointer to the slice only when non-empty
  3. Prevents potential nil pointer panics that would occur with direct appending

This implementation is consistent with the broader pattern across the codebase.

go/apps/api/openapi/openapi-generated.yaml (2)

2812-2814: Reference casing corrected – spec now resolves properly

Switching the $ref from role to Role fixes the case-sensitive lookup in OpenAPI and eliminates resolution errors that downstream code-gen and validators were hitting. Nice catch.


2828-2829: Item schema reference aligned with canonical Role

The list element now points to the correct Role schema, ensuring consistency across single-object and collection endpoints. No further action needed.

go/apps/api/routes/v2_identities_update_identity/200_test.go (5)

127-127: LGTM: Correct nil check for optional ratelimits field.

The test correctly checks for nil instead of an empty slice, which aligns with the new pointer-based approach where absent ratelimits are represented as nil rather than empty slices.


163-173: LGTM: Proper pointer dereferencing for ratelimits access.

The changes correctly dereference the Ratelimits pointer before accessing its length and elements. The pattern *res.Body.Data.Ratelimits is used consistently for both length checks and slice element access.


188-188: LGTM: Consistent pointer dereferencing in range loop.

The range loop correctly dereferences the pointer to iterate over the ratelimits slice.


209-209: LGTM: Appropriate nil check for empty ratelimits scenario.

The test correctly expects nil when all ratelimits are removed, which is consistent with the optional pointer-based field design.


264-265: LGTM: Consistent pointer handling for ratelimits verification.

Both the length check and slice assignment correctly dereference the pointer to access the underlying slice data.

go/apps/api/routes/v2_identities_update_identity/handler.go (3)

19-19: LGTM: Appropriate import for pointer utility.

The ptr package import is needed for the ptr.P helper function used to create pointers to slices.


395-403: LGTM: Improved response construction with conditional field assignment.

The refactored approach builds the response data in a structured way:

  1. Initialize identityData with base fields and Ratelimits: nil
  2. Conditionally assign the ratelimits pointer only when the slice is non-empty

This pattern ensures that empty ratelimits are represented as nil (omitted from JSON) rather than empty arrays, which is more semantically correct for optional fields.


409-409: LGTM: Clean response construction.

Using the pre-built identityData makes the response construction cleaner and more maintainable.

go/apps/api/routes/v2_apis_list_keys/handler.go (2)

427-429: LGTM: Conditional assignment for permissions.

The pattern correctly assigns the permissions pointer only when the slice contains data, ensuring empty permissions are represented as nil.


438-445: LGTM: Conditional assignment for roles.

The logic correctly processes roles and assigns the pointer only when roles exist, maintaining consistency with the optional field pattern.

go/apps/api/routes/v2_keys_verify_key/handler.go (4)

162-175: LGTM: Clean response data initialization.

The refactored approach builds the response data in a structured keyData variable with all optional fields properly initialized to nil. This makes the code more maintainable and follows the established pattern for optional fields.


177-183: LGTM: Consistent conditional assignment for permissions and roles.

The pattern correctly assigns pointers to slices only when they contain data, ensuring empty collections are represented as nil rather than empty arrays in the JSON response.


211-228: LGTM: Proper identity ratelimits handling.

The code correctly builds identity ratelimits in a local slice and conditionally assigns the pointer only when ratelimits exist. This follows the same pattern used consistently across other handlers.


265-270: LGTM: Clean final response construction.

Using the pre-built keyData makes the final response construction clean and maintainable.

go/apps/api/openapi/gen.go (1)

148-150: Pointer conversion changes runtime behaviour – audit downstream uses

Ratelimits changed from []RatelimitResponse to *[]RatelimitResponse (pointer slice + omitempty).
This is great for clean JSON (ratelimits is now omitted instead of []), but any code that previously did:

len(identity.Ratelimits)   // or range identity.Ratelimits { … }

will now panic when Ratelimits == nil.

Search all call-sites and add a nil-check (or use slices.EqualFunc style helpers) before dereferencing.

@Flo4604 Flo4604 requested a review from chronark July 31, 2025 11:29
@graphite-app
Copy link

graphite-app bot commented Jul 31, 2025

Celebrity gif. A young Keanu Reeves stands in the rain smiling. He raises up his arm and gives an enthusiastic thumbs up. (Added via Giphy)

@graphite-app
Copy link

graphite-app bot commented Jul 31, 2025

Graphite Automations

"Post a GIF when PR approved" took an action on this PR • (07/31/25)

1 gif was posted to this PR based on Andreas Thomas's automation.

"Notify author when CI fails" took an action on this PR • (07/31/25)

1 teammate was notified to this PR based on Andreas Thomas's automation.

@chronark chronark merged commit c64be47 into main Jul 31, 2025
19 checks passed
@chronark chronark deleted the fix/v2-zod-errors branch July 31, 2025 12:59
chronark added a commit that referenced this pull request Aug 7, 2025
* Dialog created

* small changes

* half functional

* small spacing changes and chevron

* Rabbit Changes

* remove useffect

* almost

* re ordered badge list collapse

* undo rabbit

* [autofix.ci] apply automated fixes

* fix scroll

* changes before merge

* chore: deprecate v1 endpoints (#3680)

* fix: openapi

* chore: deprecate v1 endpoints

* fix: vault credentials and chproxy config (#3681)

* fix: openapi

* fix: vault credentials and chproxy config

* fix: rename flag accessor too

* fix: linter issues

* fix: some more v2 api changes (#3677)

* remove namespaceID

* actually use limit and cursor

* filter out delted overrides

* fix error messages list endpoints

* fix more error messages

* ensure identity create handles like permission/role create

* fix regex for roles

* fix regex for roles

* fix list keys cursor

* fix: uppercase common files (#3683)

* name files uppercase

* name files uppercase

* [autofix.ci] apply automated fixes

* name files uppercase

* name files uppercase

---------

Co-authored-by: Andreas Thomas <dev@chronark.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

* fix: conflicting casing (#3689)

* fix the openapi spec again (#3692)

* dont trace chproxy endpoints (#3691)

* fix: log verifications to the owning workspace (#3693)

* functioning again

* style change

* more tweaks

* fix: validate s3 config (#3694)

* Fix all the fucking things

* remove close button

* fmt

* fix: speakeasy ignore directive is ignored if it's a string (#3699)

* fix: upsert permissions with slug or name colission (#3696)

* fix: upsert permissions with slug or name colission

* chore: also remove index

* fix permission test and remove unnnecessary test

---------

Co-authored-by: Flo <53355483+Flo4604@users.noreply.github.com>

* docs: migration (#3678)

* fix: openapi

* docs: migration from v1 to v2

* fix: remove binaries

* [autofix.ci] apply automated fixes

* docs: add james' feedback

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

* fix: api list keys zod errors (#3702)

* fix api zod errors

* [autofix.ci] apply automated fixes

* make array handling uniform

* make array handling uniform

* fix rabbi comment

* fix: permission array for roles

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

* ci: don't build for windows and freebsd (#3700)

* docs: errors (#3703)

* chore: move sdks to unkeyed/sdks (#3701)

* fix: omitting array vs null (#3704)

* fix omitting array vs null

* [autofix.ci] apply automated fixes

* fix flakey test

* fix flakey test

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

* fix: panics not being catched (#3706)

* fix panics not being catched

* feat: add counter to track panics

Signed-off-by: Ian Meyer <k@imeyer.io>

---------

Signed-off-by: Ian Meyer <k@imeyer.io>
Co-authored-by: Ian Meyer <k@imeyer.io>

* docs: use `CodeGroup` in hono/nextjs TS libraries (#3708)

* Update hono.mdx

* Update nextjs.mdx

* ci: remove outdated steps and flows (#3709)

* docs: update sdks (#3712)

* docs: update sdks

* Update nextjs.mdx

* [autofix.ci] apply automated fixes

* fix: rabbit feedback

* Update nextjs.mdx

* fix: root key is required

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

* dialog and confirm added

* feat(deployment beta): projects UI for Unkey Deploy (#3662)

* projects and branches

* wip

* wip

* spec differ wip

* fix some docker, add some trpc, integrate diff viewer

* change version to deployments, add feature flag, update go schema

* update versions page

* fix null condition

* delete old router, fix null assertion

* fmt

* fmt

* fmt again

* apply auth and feature flagging access to projects, remove versions

* yolo

* stable yolo

* stable yolo

* style: fmt

* fix: hardcode time, so it doesn't fail on the first of a month

* [autofix.ci] apply automated fixes

---------

Co-authored-by: chronark <dev@chronark.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

* chore: add missingd delete endpoint for v2 (#3711)

* only log key at the end (#3716)

* fix: allow wildcard and colon in permissions query (#3717)

* remove regex for permissions

* allow for asterix and colon in permissions

* fix: update identity by identity key instead of externalId and fix wrong body for permission and role (#3713)

* docs and remove externalId from keyResponse

* fix updateIdentity to take in an identity parameter instead of an externalId

* fix get role/permission

* Update go/apps/api/openapi/spec/common/Permission.yaml

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* Update openapi-generated.yaml

* fix comment

---------

Co-authored-by: Andreas Thomas <dev@chronark.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* chore: openapi references (#3723)

* feat: add paginated tRPC endpoint for projects (#3697)

* feat: add new endpoint for deploy projects

* chore: replace file path

* [autofix.ci] apply automated fixes

* feat: add missing endpoint

* fix: trpc path

* fix: add feature flag

* chore: remove optin

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

* fix: region not showing and wrong rl id (#3722)

* fix: region not showing and wrong rl id

* pass region down

* perf: bad get key performance (#3724)

* perf: make getKey 2 seperate queries so mysql chooses correct idx

* fix query

* fix query name

* docs: verify identities endpoints (#3727)

* chore: docs (#3728)

* chore: fixup migration guide

* adjust more

* adjust more

* adjust more

* rabbit comments

* Update index.mdx

* working updates

* re factor for clarity

* only update if diff than existing

* [autofix.ci] apply automated fixes

* re name create-root-key to root-key folder

---------

Signed-off-by: Ian Meyer <k@imeyer.io>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Andreas Thomas <dev@chronark.com>
Co-authored-by: Flo <53355483+Flo4604@users.noreply.github.com>
Co-authored-by: Oğuzhan Olguncu <21091016+ogzhanolguncu@users.noreply.github.com>
Co-authored-by: James Perkins <jamesperkins@hey.com>
Co-authored-by: Ian Meyer <k@imeyer.io>
Co-authored-by: JA Castro <51177379+ubinatus@users.noreply.github.com>
Co-authored-by: Meg Stepp <mcstepp@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@coderabbitai coderabbitai bot mentioned this pull request Aug 11, 2025
18 tasks
perkinsjr added a commit that referenced this pull request Aug 20, 2025
* chore: new root key dialog (#3637)

* Dialog created

* small changes

* feat: ui show permission side bar when select permission button is (#3654)

* Dialog created

* small changes

* half functional

* small spacing changes and chevron

* Rabbit Changes

* remove useffect

* feat: UI when a permission is selected it should show in the modal (#3663)

* Dialog created

* small changes

* half functional

* small spacing changes and chevron

* Rabbit Changes

* remove useffect

* almost

* re ordered badge list collapse

* undo rabbit

* [autofix.ci] apply automated fixes

* fix scroll

* button size and margin

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

* feat: UI when a user searches for a permission it should show (#3695)

* Dialog created

* small changes

* half functional

* small spacing changes and chevron

* Rabbit Changes

* remove useffect

* almost

* re ordered badge list collapse

* undo rabbit

* [autofix.ci] apply automated fixes

* fix scroll

* changes before merge

* functioning again

* style change

* more tweaks

* Fix all the fucking things

* remove close button

* fmt

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: James Perkins <jamesperkins@hey.com>

* feat: rootkey create with success dialog and confirm close (#3714)

* Dialog created

* small changes

* half functional

* small spacing changes and chevron

* Rabbit Changes

* remove useffect

* almost

* re ordered badge list collapse

* undo rabbit

* [autofix.ci] apply automated fixes

* fix scroll

* changes before merge

* functioning again

* style change

* more tweaks

* Fix all the fucking things

* remove close button

* fmt

* dialog and confirm added

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: James Perkins <jamesperkins@hey.com>

* feat: a user selects edit root key and an edit root key modal (#3731)

* Dialog created

* small changes

* half functional

* small spacing changes and chevron

* Rabbit Changes

* remove useffect

* almost

* re ordered badge list collapse

* undo rabbit

* [autofix.ci] apply automated fixes

* fix scroll

* changes before merge

* chore: deprecate v1 endpoints (#3680)

* fix: openapi

* chore: deprecate v1 endpoints

* fix: vault credentials and chproxy config (#3681)

* fix: openapi

* fix: vault credentials and chproxy config

* fix: rename flag accessor too

* fix: linter issues

* fix: some more v2 api changes (#3677)

* remove namespaceID

* actually use limit and cursor

* filter out delted overrides

* fix error messages list endpoints

* fix more error messages

* ensure identity create handles like permission/role create

* fix regex for roles

* fix regex for roles

* fix list keys cursor

* fix: uppercase common files (#3683)

* name files uppercase

* name files uppercase

* [autofix.ci] apply automated fixes

* name files uppercase

* name files uppercase

---------

Co-authored-by: Andreas Thomas <dev@chronark.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

* fix: conflicting casing (#3689)

* fix the openapi spec again (#3692)

* dont trace chproxy endpoints (#3691)

* fix: log verifications to the owning workspace (#3693)

* functioning again

* style change

* more tweaks

* fix: validate s3 config (#3694)

* Fix all the fucking things

* remove close button

* fmt

* fix: speakeasy ignore directive is ignored if it's a string (#3699)

* fix: upsert permissions with slug or name colission (#3696)

* fix: upsert permissions with slug or name colission

* chore: also remove index

* fix permission test and remove unnnecessary test

---------

Co-authored-by: Flo <53355483+Flo4604@users.noreply.github.com>

* docs: migration (#3678)

* fix: openapi

* docs: migration from v1 to v2

* fix: remove binaries

* [autofix.ci] apply automated fixes

* docs: add james' feedback

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

* fix: api list keys zod errors (#3702)

* fix api zod errors

* [autofix.ci] apply automated fixes

* make array handling uniform

* make array handling uniform

* fix rabbi comment

* fix: permission array for roles

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

* ci: don't build for windows and freebsd (#3700)

* docs: errors (#3703)

* chore: move sdks to unkeyed/sdks (#3701)

* fix: omitting array vs null (#3704)

* fix omitting array vs null

* [autofix.ci] apply automated fixes

* fix flakey test

* fix flakey test

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

* fix: panics not being catched (#3706)

* fix panics not being catched

* feat: add counter to track panics

Signed-off-by: Ian Meyer <k@imeyer.io>

---------

Signed-off-by: Ian Meyer <k@imeyer.io>
Co-authored-by: Ian Meyer <k@imeyer.io>

* docs: use `CodeGroup` in hono/nextjs TS libraries (#3708)

* Update hono.mdx

* Update nextjs.mdx

* ci: remove outdated steps and flows (#3709)

* docs: update sdks (#3712)

* docs: update sdks

* Update nextjs.mdx

* [autofix.ci] apply automated fixes

* fix: rabbit feedback

* Update nextjs.mdx

* fix: root key is required

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

* dialog and confirm added

* feat(deployment beta): projects UI for Unkey Deploy (#3662)

* projects and branches

* wip

* wip

* spec differ wip

* fix some docker, add some trpc, integrate diff viewer

* change version to deployments, add feature flag, update go schema

* update versions page

* fix null condition

* delete old router, fix null assertion

* fmt

* fmt

* fmt again

* apply auth and feature flagging access to projects, remove versions

* yolo

* stable yolo

* stable yolo

* style: fmt

* fix: hardcode time, so it doesn't fail on the first of a month

* [autofix.ci] apply automated fixes

---------

Co-authored-by: chronark <dev@chronark.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

* chore: add missingd delete endpoint for v2 (#3711)

* only log key at the end (#3716)

* fix: allow wildcard and colon in permissions query (#3717)

* remove regex for permissions

* allow for asterix and colon in permissions

* fix: update identity by identity key instead of externalId and fix wrong body for permission and role (#3713)

* docs and remove externalId from keyResponse

* fix updateIdentity to take in an identity parameter instead of an externalId

* fix get role/permission

* Update go/apps/api/openapi/spec/common/Permission.yaml

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* Update openapi-generated.yaml

* fix comment

---------

Co-authored-by: Andreas Thomas <dev@chronark.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* chore: openapi references (#3723)

* feat: add paginated tRPC endpoint for projects (#3697)

* feat: add new endpoint for deploy projects

* chore: replace file path

* [autofix.ci] apply automated fixes

* feat: add missing endpoint

* fix: trpc path

* fix: add feature flag

* chore: remove optin

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

* fix: region not showing and wrong rl id (#3722)

* fix: region not showing and wrong rl id

* pass region down

* perf: bad get key performance (#3724)

* perf: make getKey 2 seperate queries so mysql chooses correct idx

* fix query

* fix query name

* docs: verify identities endpoints (#3727)

* chore: docs (#3728)

* chore: fixup migration guide

* adjust more

* adjust more

* adjust more

* rabbit comments

* Update index.mdx

* working updates

* re factor for clarity

* only update if diff than existing

* [autofix.ci] apply automated fixes

* re name create-root-key to root-key folder

---------

Signed-off-by: Ian Meyer <k@imeyer.io>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Andreas Thomas <dev@chronark.com>
Co-authored-by: Flo <53355483+Flo4604@users.noreply.github.com>
Co-authored-by: Oğuzhan Olguncu <21091016+ogzhanolguncu@users.noreply.github.com>
Co-authored-by: James Perkins <jamesperkins@hey.com>
Co-authored-by: Ian Meyer <k@imeyer.io>
Co-authored-by: JA Castro <51177379+ubinatus@users.noreply.github.com>
Co-authored-by: Meg Stepp <mcstepp@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* fresh

* start rabbit changes

* rabbits

* missing processing

* Update apps/dashboard/lib/trpc/routers/index.ts

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* Update apps/dashboard/app/(app)/settings/root-keys/components/root-key/components/permission-badge-list.tsx

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* more rabbit

* function name

* [autofix.ci] apply automated fixes

* minor changes

* rabbit checked locally

* comment changes

* light mode fix

* removed old page files

* updated success

* slack comment changes

* cleanup

* rabbit

* more rabbits

* most consts

* change clear and details

* cleanup

* pr suggested changes

* few nits

---------

Signed-off-by: Ian Meyer <k@imeyer.io>
Co-authored-by: CodeReaper <148160799+MichaelUnkey@users.noreply.github.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Andreas Thomas <dev@chronark.com>
Co-authored-by: Flo <53355483+Flo4604@users.noreply.github.com>
Co-authored-by: Oğuzhan Olguncu <21091016+ogzhanolguncu@users.noreply.github.com>
Co-authored-by: Ian Meyer <k@imeyer.io>
Co-authored-by: JA Castro <51177379+ubinatus@users.noreply.github.com>
Co-authored-by: Meg Stepp <mcstepp@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: MichaelUnkey <michael@unkey.com>
@coderabbitai coderabbitai bot mentioned this pull request Nov 10, 2025
19 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants