-
Notifications
You must be signed in to change notification settings - Fork 583
fix: nullable validation + tests #3753
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Skipped Deployments
|
📝 WalkthroughWalkthroughThis change updates OpenAPI schema files and Go code to switch from using the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API_Server
participant Validator
Client->>API_Server: Send request with OpenAPI 3.1.0 schema
API_Server->>Validator: Validate request body
Validator-->>API_Server: Return first validation error (no nullable error filtering)
API_Server-->>Client: Respond with error details (if any)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–25 minutes Suggested labels
Suggested reviewers
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Thank you for following the naming conventions for pull request titles! 🙏 |
Graphite Automations"Post a GIF when PR approved" took an action on this PR • (08/08/25)1 gif was posted to this PR based on Andreas Thomas's automation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
go/pkg/db/bulk_key_insert_ratelimit.sql.go
(1 hunks)go/pkg/db/key_insert_ratelimit.sql_generated.go
(2 hunks)go/pkg/db/querier_generated.go
(1 hunks)go/pkg/zen/validation/validator.go
(2 hunks)go/pkg/zen/validation/validator_integration_test.go
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.go
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.go
: Follow comprehensive documentation guidelines for Go code as described in go/GO_DOCUMENTATION_GUIDELINES.md
Every public function/type in Go code must be documented
Prefer interfaces for testability in Go code
Use AIDEV-* comments for complex/important code in Go services
Files:
go/pkg/db/bulk_key_insert_ratelimit.sql.go
go/pkg/db/key_insert_ratelimit.sql_generated.go
go/pkg/db/querier_generated.go
go/pkg/zen/validation/validator_integration_test.go
go/pkg/zen/validation/validator.go
**/*.{env,js,ts,go}
📄 CodeRabbit Inference Engine (CLAUDE.md)
All environment variables must follow the format: UNKEY_<SERVICE_NAME>_VARNAME
Files:
go/pkg/db/bulk_key_insert_ratelimit.sql.go
go/pkg/db/key_insert_ratelimit.sql_generated.go
go/pkg/db/querier_generated.go
go/pkg/zen/validation/validator_integration_test.go
go/pkg/zen/validation/validator.go
**/*_test.go
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*_test.go
: Use table-driven tests in Go
Organize Go integration tests with real dependencies
Organize Go tests by HTTP status codes
Files:
go/pkg/zen/validation/validator_integration_test.go
🧠 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: chronark
PR: unkeyed/unkey#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: chronark
PR: unkeyed/unkey#3617
File: go/apps/api/openapi/openapi.yaml:3309-3312
Timestamp: 2025-07-16T17:51:57.297Z
Learning: In the Unkey API OpenAPI schema, the permissions query regex for the verifyKey endpoint intentionally allows all whitespace characters (including tabs and newlines) via `\s`. Do not flag this as an error in future reviews.
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.
Learnt from: Flo4604
PR: unkeyed/unkey#3647
File: go/apps/api/openapi/openapi-generated.yaml:3569-3575
Timestamp: 2025-07-22T18:09:41.800Z
Learning: In the Unkey codebase, using non-standard HTTP status code 529 for internal-only endpoints is acceptable and should not be flagged as an issue in future reviews.
Learnt from: Flo4604
PR: unkeyed/unkey#3631
File: go/pkg/db/bulk_keyring_insert.sql.go:23-25
Timestamp: 2025-07-17T14:24:20.403Z
Learning: In go/pkg/db/bulk_keyring_insert.sql.go and similar bulk insert generated files, hardcoded zero values for fields like size_approx and size_last_updated_at are intentional and reflect the original SQL query structure, not missing parameters.
Learnt from: chronark
PR: unkeyed/unkey#2146
File: apps/dashboard/lib/trpc/routers/api/setDefaultPrefix.ts:80-80
Timestamp: 2024-10-04T17:27:08.666Z
Learning: Ensure that audit log descriptions accurately reflect the action being performed, such as updating the `defaultPrefix`, and avoid incorrect references like 'name' when not applicable.
Learnt from: CR
PR: unkeyed/unkey#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T07:44:39.438Z
Learning: Applies to **/migrations/**/* : Use Drizzle migrations for database changes, not manual SQL
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.
Learnt from: chronark
PR: unkeyed/unkey#3161
File: go/pkg/clickhouse/schema/databases/002_ratelimits/006_ratelimits_per_day_v1.sql:1-13
Timestamp: 2025-04-22T14:43:11.724Z
Learning: In the unkey project, the SQL files in clickhouse/schema/databases represent the current production schema and shouldn't be modified directly in PRs. Schema changes require dedicated migration scripts.
Learnt from: chronark
PR: unkeyed/unkey#3161
File: go/pkg/clickhouse/schema/databases/001_verifications/002_raw_key_verifications_v1.sql:31-33
Timestamp: 2025-04-22T14:40:51.459Z
Learning: The ClickHouse table schemas in the codebase mirror the production environment and cannot be modified directly in PRs without careful migration planning.
📚 Learning: 2025-07-17T14:24:20.403Z
Learnt from: Flo4604
PR: unkeyed/unkey#3631
File: go/pkg/db/bulk_keyring_insert.sql.go:23-25
Timestamp: 2025-07-17T14:24:20.403Z
Learning: In go/pkg/db/bulk_keyring_insert.sql.go and similar bulk insert generated files, hardcoded zero values for fields like size_approx and size_last_updated_at are intentional and reflect the original SQL query structure, not missing parameters.
Applied to files:
go/pkg/db/bulk_key_insert_ratelimit.sql.go
go/pkg/db/key_insert_ratelimit.sql_generated.go
go/pkg/db/querier_generated.go
📚 Learning: 2024-10-20T07:05:55.471Z
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/pkg/db/bulk_key_insert_ratelimit.sql.go
go/pkg/db/key_insert_ratelimit.sql_generated.go
go/pkg/db/querier_generated.go
📚 Learning: 2025-07-02T11:51:58.572Z
Learnt from: chronark
PR: unkeyed/unkey#3420
File: go/pkg/hydra/store/gorm/gorm.go:486-498
Timestamp: 2025-07-02T11:51:58.572Z
Learning: The Hydra package (go/pkg/hydra) is planned to be migrated from GORM to sqlc for database operations, which explains why raw SQL queries are acceptable in the current implementation.
Applied to files:
go/pkg/db/bulk_key_insert_ratelimit.sql.go
go/pkg/db/key_insert_ratelimit.sql_generated.go
📚 Learning: 2025-07-16T15:38:53.491Z
Learnt from: Flo4604
PR: unkeyed/unkey#3606
File: go/pkg/db/replica.go:8-11
Timestamp: 2025-07-16T15:38:53.491Z
Learning: For debugging database replica usage in go/pkg/db/replica.go, it's acceptable to mark QueryRowContext operations as "success" even though SQL errors only surface during row.Scan() calls. The timing metrics are the primary concern for debugging replica performance patterns.
Applied to files:
go/pkg/db/bulk_key_insert_ratelimit.sql.go
go/pkg/db/key_insert_ratelimit.sql_generated.go
📚 Learning: 2025-04-22T14:43:11.724Z
Learnt from: chronark
PR: unkeyed/unkey#3161
File: go/pkg/clickhouse/schema/databases/002_ratelimits/006_ratelimits_per_day_v1.sql:1-13
Timestamp: 2025-04-22T14:43:11.724Z
Learning: In the unkey project, the SQL files in clickhouse/schema/databases represent the current production schema and shouldn't be modified directly in PRs. Schema changes require dedicated migration scripts.
Applied to files:
go/pkg/db/key_insert_ratelimit.sql_generated.go
📚 Learning: 2025-08-04T07:44:39.438Z
Learnt from: CR
PR: unkeyed/unkey#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T07:44:39.438Z
Learning: Applies to **/*_test.go : Organize Go integration tests with real dependencies
Applied to files:
go/pkg/zen/validation/validator_integration_test.go
📚 Learning: 2025-08-04T07:44:39.438Z
Learnt from: CR
PR: unkeyed/unkey#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T07:44:39.438Z
Learning: Applies to **/*_test.go : Use table-driven tests in Go
Applied to files:
go/pkg/zen/validation/validator_integration_test.go
📚 Learning: 2025-08-04T07:44:39.438Z
Learnt from: CR
PR: unkeyed/unkey#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T07:44:39.438Z
Learning: Applies to **/*_test.go : Organize Go tests by HTTP status codes
Applied to files:
go/pkg/zen/validation/validator_integration_test.go
📚 Learning: 2025-08-04T07:44:39.438Z
Learnt from: CR
PR: unkeyed/unkey#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T07:44:39.438Z
Learning: Applies to **/*.go : Prefer interfaces for testability in Go code
Applied to files:
go/pkg/zen/validation/validator_integration_test.go
📚 Learning: 2025-07-15T14:25:05.608Z
Learnt from: chronark
PR: unkeyed/unkey#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.
Applied to files:
go/pkg/zen/validation/validator.go
🧬 Code Graph Analysis (1)
go/pkg/zen/validation/validator_integration_test.go (1)
go/pkg/zen/validation/validator.go (1)
Validator
(27-29)
🔇 Additional comments (8)
go/pkg/db/bulk_key_insert_ratelimit.sql.go (1)
14-15
: Confirm removal of backticks for duration/auto_applyI ran a repo-wide search and found no remaining occurrences of backticked
duration
orauto_apply
in ON DUPLICATE KEY UPDATE.
- No matches for
`duration` = VALUES(`duration`)
or`auto_apply` = VALUES(`auto_apply`)
- Please ensure the original SQL (e.g.
key_insert_ratelimit.sql
used by sqlc or your generator template) likewise uses unquoted identifiers to prevent future drift.go/pkg/db/key_insert_ratelimit.sql_generated.go (2)
34-35
: SQL constant aligns with intended quoting: good change.Removing backticks for duration/auto_apply matches insert columns and avoids unnecessary quoting. No functional impact.
73-74
: Comment block kept in sync with SQL: good.Docs now reflect the unquoted identifiers; this reduces confusion.
go/pkg/db/querier_generated.go (1)
873-874
: Interface docs updated for consistency with SQL.The unquoted duration/auto_apply in the example now match actual statements.
go/pkg/zen/validation/validator.go (3)
85-104
: Good: separate non-schema errors from schema errorsTreating entries with no
SchemaValidationErrors
as non-filterable (e.g., security/method/path) is correct and aligns with the intent to only filter nullable schema errors.
109-121
: Granular inclusion of only non-nullable schema errors is correctIterating nested schema failures and appending just the non-nullable ones achieves the desired behavior from the PR objective (avoid false positives on nested nullables).
129-133
: Correct: pass validation when all errors are nullableThis early return is the right behavior for the reported bug.
go/pkg/zen/validation/validator_integration_test.go (1)
77-263
: Strong, table-driven integration tests covering nested nullable scenariosGood breadth: required vs optional, nested objects, deep nullables, mixed constraint errors, and ensuring nullable-related errors don’t fail validation. The table-driven pattern and use of real OpenAPI docs match our testing guidelines.
Also applies to: 265-409, 410-613
rabbit made some good points will fix them |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (1)
go/pkg/zen/validation/validator.go (1)
135-138
: Fallback toReason
whenMessage
is emptyThe earlier review note still applies:
firstError.Message
may be empty for some validators, producing a blankdetail
in the response.
UsefirstError.Message
if non-empty, otherwise fall back tofirstError.Reason
.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
go/pkg/zen/validation/validator.go
(3 hunks)go/pkg/zen/validation/validator_integration_test.go
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.go
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.go
: Follow comprehensive documentation guidelines for Go code as described in go/GO_DOCUMENTATION_GUIDELINES.md
Every public function/type in Go code must be documented
Prefer interfaces for testability in Go code
Use AIDEV-* comments for complex/important code in Go services
Files:
go/pkg/zen/validation/validator.go
go/pkg/zen/validation/validator_integration_test.go
**/*.{env,js,ts,go}
📄 CodeRabbit Inference Engine (CLAUDE.md)
All environment variables must follow the format: UNKEY_<SERVICE_NAME>_VARNAME
Files:
go/pkg/zen/validation/validator.go
go/pkg/zen/validation/validator_integration_test.go
**/*_test.go
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*_test.go
: Use table-driven tests in Go
Organize Go integration tests with real dependencies
Organize Go tests by HTTP status codes
Files:
go/pkg/zen/validation/validator_integration_test.go
🧠 Learnings (10)
📓 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: Flo4604
PR: unkeyed/unkey#3647
File: go/apps/api/openapi/openapi-generated.yaml:3569-3575
Timestamp: 2025-07-22T18:09:41.800Z
Learning: In the Unkey codebase, using non-standard HTTP status code 529 for internal-only endpoints is acceptable and should not be flagged as an issue in future reviews.
Learnt from: chronark
PR: unkeyed/unkey#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: chronark
PR: unkeyed/unkey#3617
File: go/apps/api/openapi/openapi.yaml:3309-3312
Timestamp: 2025-07-16T17:51:57.297Z
Learning: In the Unkey API OpenAPI schema, the permissions query regex for the verifyKey endpoint intentionally allows all whitespace characters (including tabs and newlines) via `\s`. Do not flag this as an error in future reviews.
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.
Learnt from: Flo4604
PR: unkeyed/unkey#3631
File: go/pkg/db/bulk_keyring_insert.sql.go:23-25
Timestamp: 2025-07-17T14:24:20.403Z
Learning: In go/pkg/db/bulk_keyring_insert.sql.go and similar bulk insert generated files, hardcoded zero values for fields like size_approx and size_last_updated_at are intentional and reflect the original SQL query structure, not missing parameters.
Learnt from: chronark
PR: unkeyed/unkey#2146
File: apps/dashboard/lib/trpc/routers/api/setDefaultPrefix.ts:80-80
Timestamp: 2024-10-04T17:27:08.666Z
Learning: Ensure that audit log descriptions accurately reflect the action being performed, such as updating the `defaultPrefix`, and avoid incorrect references like 'name' when not applicable.
Learnt from: CR
PR: unkeyed/unkey#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T07:44:39.438Z
Learning: Applies to **/migrations/**/* : Use Drizzle migrations for database changes, not manual SQL
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.
Learnt from: chronark
PR: unkeyed/unkey#3161
File: go/pkg/clickhouse/schema/databases/002_ratelimits/006_ratelimits_per_day_v1.sql:1-13
Timestamp: 2025-04-22T14:43:11.724Z
Learning: In the unkey project, the SQL files in clickhouse/schema/databases represent the current production schema and shouldn't be modified directly in PRs. Schema changes require dedicated migration scripts.
Learnt from: chronark
PR: unkeyed/unkey#3161
File: go/pkg/clickhouse/schema/databases/001_verifications/002_raw_key_verifications_v1.sql:31-33
Timestamp: 2025-04-22T14:40:51.459Z
Learning: The ClickHouse table schemas in the codebase mirror the production environment and cannot be modified directly in PRs without careful migration planning.
📚 Learning: 2025-07-15T14:25:05.608Z
Learnt from: chronark
PR: unkeyed/unkey#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.
Applied to files:
go/pkg/zen/validation/validator.go
📚 Learning: 2025-07-03T05:58:10.699Z
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/pkg/zen/validation/validator.go
📚 Learning: 2025-07-21T18:05:58.236Z
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-21T18:05:58.236Z
Learning: Applies to go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,env,conf,html,css,scss,xml,c,h,cpp,java,rb,rs,php,pl,sql} : Make sure to add relevant anchor comments whenever a file or piece of code is too complex, very important, confusing, or could have a bug.
Applied to files:
go/pkg/zen/validation/validator.go
📚 Learning: 2025-08-04T07:44:39.438Z
Learnt from: CR
PR: unkeyed/unkey#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T07:44:39.438Z
Learning: Applies to **/*_test.go : Organize Go tests by HTTP status codes
Applied to files:
go/pkg/zen/validation/validator.go
go/pkg/zen/validation/validator_integration_test.go
📚 Learning: 2025-08-04T07:44:39.438Z
Learnt from: CR
PR: unkeyed/unkey#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T07:44:39.438Z
Learning: Applies to **/*_test.go : Organize Go integration tests with real dependencies
Applied to files:
go/pkg/zen/validation/validator_integration_test.go
📚 Learning: 2025-08-04T07:44:39.438Z
Learnt from: CR
PR: unkeyed/unkey#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T07:44:39.438Z
Learning: Applies to **/*_test.go : Use table-driven tests in Go
Applied to files:
go/pkg/zen/validation/validator_integration_test.go
📚 Learning: 2025-08-04T07:44:39.438Z
Learnt from: CR
PR: unkeyed/unkey#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T07:44:39.438Z
Learning: Applies to **/*.go : Prefer interfaces for testability in Go code
Applied to files:
go/pkg/zen/validation/validator_integration_test.go
📚 Learning: 2025-07-16T17:51:57.297Z
Learnt from: chronark
PR: unkeyed/unkey#3617
File: go/apps/api/openapi/openapi.yaml:3309-3312
Timestamp: 2025-07-16T17:51:57.297Z
Learning: In the Unkey API OpenAPI schema, the permissions query regex for the verifyKey endpoint intentionally allows all whitespace characters (including tabs and newlines) via `\s`. Do not flag this as an error in future reviews.
Applied to files:
go/pkg/zen/validation/validator_integration_test.go
📚 Learning: 2025-07-21T18:05:58.236Z
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-21T18:05:58.236Z
Learning: Applies to go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,env,conf,html,css,scss,xml,c,h,cpp,java,rb,rs,php,pl,sql} : Use `AIDEV-NOTE:`, `AIDEV-TODO:`, `AIDEV-BUSINESS_RULE:`, or `AIDEV-QUESTION:` (all-caps prefix) as anchor comments aimed at AI and developers.
Applied to files:
go/pkg/zen/validation/validator_integration_test.go
🧬 Code Graph Analysis (1)
go/pkg/zen/validation/validator_integration_test.go (1)
go/pkg/zen/validation/validator.go (1)
Validator
(28-30)
🪛 golangci-lint (2.2.2)
go/pkg/zen/validation/validator_integration_test.go
250-250: "POST" can be replaced by http.MethodPost
(usestdlibvars)
400-400: "POST" can be replaced by http.MethodPost
(usestdlibvars)
583-583: "POST" can be replaced by http.MethodPost
(usestdlibvars)
782-782: "POST" can be replaced by http.MethodPost
(usestdlibvars)
⏰ 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). (2)
- GitHub Check: Test Go API Local / Test
- GitHub Check: Test API / API Test Local
…o fix/nullable_validator
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
🔭 Outside diff range comments (4)
go/pkg/zen/validation/validator.go (1)
57-63
: Method no longer satisfies declared interface – compilation will fail
OpenAPIValidator.Validate
expects(r *http.Request)
, butValidator.Validate
now takes an extracontext.Context
. Either:
- Drop the first parameter and read the context from
r.Context()
, or- Update the interface and every call-site.
Leaving it as-is breaks builds wherever
Validator
is used via the interface.go/apps/api/openapi/spec/paths/v2/keys/updateKey/V2KeysUpdateKeyRequestBody.yaml (3)
87-95
: Schema-doc mismatch: ratelimits described as nullable but type is array-onlyDescription says “setting null removes all rate limits,” but the schema doesn’t permit null. Align schema to accept null.
Apply:
- ratelimits: - type: array + ratelimits: + type: + - array + - "null" maxItems: 50 # Reasonable limit for rate limit configurations per key items: "$ref": "../../../../common/RatelimitRequest.yaml" description: | Defines time-based rate limits that protect against abuse by controlling request frequency. Omitting this field preserves existing rate limits, while setting null removes all rate limits. Unlike credits which track total usage, rate limits reset automatically after each window expires. Multiple rate limits can control different operation types with separate thresholds and windows.
137-225
: Optional: add an example with credits: null to lock in behaviorGiven the PR’s goal, an explicit example helps prevent regressions and clarifies client usage.
Append:
removeExpiration: summary: Make temporary key permanent description: Remove expiration from a key that was initially created with time limits value: keyId: key_temp_789 expires: null meta: status: permanent convertedDate: "2024-01-15T10:30:00Z" originalExpiry: "2024-02-15T10:30:00Z" + + unlimitedCredits: + summary: Enable unlimited usage by clearing credits + description: Remove credit limits by setting credits to null + value: + keyId: key_unlimited_123 + credits: null
80-86
: Allowcredits
to be null in the UpdateKey request schemaKeyCreditsData.yaml defines the schema as an object only (no
null
type), socredits: null
will be rejected despite the description. Wrap the$ref
in ananyOf
to permit null.
- Update go/apps/api/openapi/spec/paths/v2/keys/updateKey/V2KeysUpdateKeyRequestBody.yaml around lines 80–86
- credits: - "$ref": "../../../../common/KeyCreditsData.yaml" - description: | + credits: + anyOf: + - "$ref": "../../../../common/KeyCreditsData.yaml" + - type: "null" + description: | Controls usage-based limits for this key through credit consumption. Omitting this field preserves current credit settings, while setting null enables unlimited usage. Cannot configure refill settings when credits is null, and refillDay requires monthly interval. Essential for implementing usage-based pricing and subscription quotas.
♻️ Duplicate comments (1)
go/pkg/zen/validation/validator.go (1)
81-83
: Repeat: fall back toReason
whenMessage
is empty
Previous review noted thatfirstError.Message
can be blank. The code still assigns it directly toDetail
, risking empty error messages. Re-apply the earlier fix.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
go/go.sum
is excluded by!**/*.sum
📒 Files selected for processing (11)
go/apps/api/openapi/config.yaml
(1 hunks)go/apps/api/openapi/gen.go
(1 hunks)go/apps/api/openapi/openapi-generated.yaml
(6 hunks)go/apps/api/openapi/openapi-split.yaml
(1 hunks)go/apps/api/openapi/overlay.yaml
(1 hunks)go/apps/api/openapi/spec/common/KeyCreditsData.yaml
(1 hunks)go/apps/api/openapi/spec/paths/v2/keys/updateCredits/V2KeysUpdateCreditsRequestBody.yaml
(1 hunks)go/apps/api/openapi/spec/paths/v2/keys/updateKey/V2KeysUpdateKeyRequestBody.yaml
(3 hunks)go/demo_api/main.go
(2 hunks)go/go.mod
(3 hunks)go/pkg/zen/validation/validator.go
(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.go
: Follow comprehensive documentation guidelines for Go code as described in go/GO_DOCUMENTATION_GUIDELINES.md
Every public function/type in Go code must be documented
Prefer interfaces for testability in Go code
Use AIDEV-* comments for complex/important code in Go services
Files:
go/demo_api/main.go
go/apps/api/openapi/gen.go
go/pkg/zen/validation/validator.go
**/*.{env,js,ts,go}
📄 CodeRabbit Inference Engine (CLAUDE.md)
All environment variables must follow the format: UNKEY_<SERVICE_NAME>_VARNAME
Files:
go/demo_api/main.go
go/apps/api/openapi/gen.go
go/pkg/zen/validation/validator.go
🧠 Learnings (15)
📓 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: Flo4604
PR: unkeyed/unkey#3647
File: go/apps/api/openapi/openapi-generated.yaml:3569-3575
Timestamp: 2025-07-22T18:09:41.800Z
Learning: In the Unkey codebase, using non-standard HTTP status code 529 for internal-only endpoints is acceptable and should not be flagged as an issue in future reviews.
Learnt from: chronark
PR: unkeyed/unkey#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: chronark
PR: unkeyed/unkey#3617
File: go/apps/api/openapi/openapi.yaml:3309-3312
Timestamp: 2025-07-16T17:51:57.297Z
Learning: In the Unkey API OpenAPI schema, the permissions query regex for the verifyKey endpoint intentionally allows all whitespace characters (including tabs and newlines) via `\s`. Do not flag this as an error in future reviews.
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.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-21T18:05:58.236Z
Learning: Applies to go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,env,conf,html,css,scss,xml,c,h,cpp,java,rb,rs,php,pl,sql} : Update relevant anchors when modifying associated code.
📚 Learning: 2025-07-03T05:58:10.699Z
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-split.yaml
go/apps/api/openapi/spec/common/KeyCreditsData.yaml
go/apps/api/openapi/spec/paths/v2/keys/updateCredits/V2KeysUpdateCreditsRequestBody.yaml
go/demo_api/main.go
go/apps/api/openapi/spec/paths/v2/keys/updateKey/V2KeysUpdateKeyRequestBody.yaml
go/pkg/zen/validation/validator.go
go/apps/api/openapi/overlay.yaml
go/apps/api/openapi/openapi-generated.yaml
📚 Learning: 2025-03-19T09:25:59.751Z
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-split.yaml
go/apps/api/openapi/spec/paths/v2/keys/updateKey/V2KeysUpdateKeyRequestBody.yaml
go/pkg/zen/validation/validator.go
go/apps/api/openapi/openapi-generated.yaml
📚 Learning: 2025-07-15T14:25:05.608Z
Learnt from: chronark
PR: unkeyed/unkey#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.
Applied to files:
go/apps/api/openapi/openapi-split.yaml
go/pkg/zen/validation/validator.go
📚 Learning: 2025-07-21T18:05:58.236Z
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-21T18:05:58.236Z
Learning: Applies to go/deploy/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : When a service's `*.go` code changes significantly, increase the patch-level version number.
Applied to files:
go/demo_api/main.go
go/go.mod
go/apps/api/openapi/gen.go
📚 Learning: 2025-08-04T07:44:39.438Z
Learnt from: CR
PR: unkeyed/unkey#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T07:44:39.438Z
Learning: Applies to **/*_test.go : Organize Go integration tests with real dependencies
Applied to files:
go/go.mod
📚 Learning: 2025-07-15T14:59:30.212Z
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/go.mod
📚 Learning: 2025-07-16T10:06:35.397Z
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/go.mod
📚 Learning: 2024-11-29T15:15:47.308Z
Learnt from: chronark
PR: unkeyed/unkey#2693
File: apps/api/src/routes/v1_keys_updateKey.ts:350-368
Timestamp: 2024-11-29T15:15:47.308Z
Learning: In `apps/api/src/routes/v1_keys_updateKey.ts`, the code intentionally handles `externalId` and `ownerId` separately for clarity. The `ownerId` field will be removed in the future, simplifying the code.
Applied to files:
go/apps/api/openapi/spec/paths/v2/keys/updateKey/V2KeysUpdateKeyRequestBody.yaml
📚 Learning: 2025-07-16T17:51:57.297Z
Learnt from: chronark
PR: unkeyed/unkey#3617
File: go/apps/api/openapi/openapi.yaml:3309-3312
Timestamp: 2025-07-16T17:51:57.297Z
Learning: In the Unkey API OpenAPI schema, the permissions query regex for the verifyKey endpoint intentionally allows all whitespace characters (including tabs and newlines) via `\s`. Do not flag this as an error in future reviews.
Applied to files:
go/apps/api/openapi/spec/paths/v2/keys/updateKey/V2KeysUpdateKeyRequestBody.yaml
📚 Learning: 2025-07-15T14:47:20.490Z
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/openapi/spec/paths/v2/keys/updateKey/V2KeysUpdateKeyRequestBody.yaml
go/apps/api/openapi/openapi-generated.yaml
📚 Learning: 2025-08-04T07:44:39.438Z
Learnt from: CR
PR: unkeyed/unkey#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T07:44:39.438Z
Learning: Applies to **/*.go : Use AIDEV-* comments for complex/important code in Go services
Applied to files:
go/apps/api/openapi/gen.go
📚 Learning: 2025-07-21T18:05:58.236Z
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-21T18:05:58.236Z
Learning: Applies to go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,env,conf,html,css,scss,xml,c,h,cpp,java,rb,rs,php,pl,sql} : Make sure to add relevant anchor comments whenever a file or piece of code is too complex, very important, confusing, or could have a bug.
Applied to files:
go/pkg/zen/validation/validator.go
📚 Learning: 2025-06-18T12:28:10.449Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3321
File: apps/dashboard/lib/trpc/routers/authorization/roles/keys/schema-with-helpers.ts:5-8
Timestamp: 2025-06-18T12:28:10.449Z
Learning: In the unkey dashboard application, API validation for pagination limits is controlled at the UI level rather than requiring additional server-side validation, as the APIs are internal and protected by UI logic.
Applied to files:
go/apps/api/openapi/openapi-generated.yaml
📚 Learning: 2025-01-24T08:55:18.790Z
Learnt from: chronark
PR: unkeyed/unkey#2832
File: go/cmd/api/main.go:88-88
Timestamp: 2025-01-24T08:55:18.790Z
Learning: The metrics middleware in the API v2 implementation is intentionally initialized with nil configuration during development, with the understanding that proper configuration will be added before production deployment.
Applied to files:
go/apps/api/openapi/openapi-generated.yaml
⏰ 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 API / API Test Local
- GitHub Check: Build / Build
- GitHub Check: Test Packages / Test
- GitHub Check: autofix
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (12)
go/go.mod (1)
19-20
: Verify dependency bumps for breaking changes
oapi-codegen/v2
→ v2.5.0 andspeakeasy-api/openapi-overlay
→ v0.10.2 are minor-version jumps that occasionally introduce code-gen or overlay schema changes. Please rungo mod tidy && go test ./...
to confirm nothing else needs updating.Also applies to: 130-131, 168-168
go/apps/api/openapi/openapi-split.yaml (1)
1-1
: Spec version updated to 3.1.0 – looks good
No issues; Unkey already relies on 3.1 features.go/apps/api/openapi/gen.go (1)
3-3
: Header shows oapi-codegen v2.5.0 — OKVersion bump is consistent with the PR’s OAS 3.1/nullability work. No functional issues from this header change alone.
go/apps/api/openapi/spec/common/KeyCreditsData.yaml (1)
5-7
: 3.1-compliant nullability for remaining looks goodSwitch to
type: [integer, "null"]
is correct and addresses the unlimited case without tripping integer constraints. Keepingrequired: ["remaining"]
ensures explicit presence in responses.go/demo_api/main.go (1)
46-46
: Consistent with OpenAPI 3.1 nullabilityThe embedded spec correctly uses
type: [string, null]
for nullable strings. No issues spotted.Also applies to: 293-293
go/apps/api/openapi/spec/paths/v2/keys/updateKey/V2KeysUpdateKeyRequestBody.yaml (4)
15-17
: Union type for nullability (OAS 3.1) looks correct for nameSwitching to type: [string, "null"] is the right approach and interoperates well with min/max length constraints.
26-28
: Good: externalId now explicitly allows nullUnion type aligns with OAS 3.1 and keeps pattern/length constraints intact for string instances.
39-41
: Good: meta nullable via object|nullThis fits JSON Schema 2020-12 semantics; additionalProperties/maxProperties apply only when object.
64-66
: Good: expires as integer|null (int64) with boundsThis enables clearing expirations by sending null while preserving numeric validation otherwise.
go/apps/api/openapi/openapi-generated.yaml (3)
1029-1031
: Correct 3.1 nullable modeling for integersSwitching to type: [integer, "null"] is the right fix to allow null while preserving integer constraints. This should eliminate false positives in validators for nested fields that can be unset.
1102-1104
: Nullable object for meta is appropriateAllowing meta to be either object or null aligns with partial update semantics and should resolve prior validation complaints on nested fields.
2147-2149
: remaining as [integer, "null"] aligns with “null = unlimited” contractThis matches the documented behavior and will avoid spurious validation errors when representing unlimited credits.
What does this PR do?
Replaces bad nullable handling with using openapi 3.1 spec for the validator to be happy...
Uses an openapi-overlay file to downgrade 3.1 to 3.0.0 spec so the openapi codegen is happy.
Type of change
How should this be tested?
Checklist
Required
pnpm build
pnpm fmt
console.logs
git pull origin main
Appreciated
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores