Conversation
|
📝 WalkthroughWalkthroughAdded guarded nil/type checks and defensive JSON decoding for byte-encoded fields across API handlers and services; prevented state mutation on failed Gets; added nil-session/request guards in Bearer; and added/updated tests covering these error paths. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant KeysService as Keys Service
Client->>API: Request (rawKey)
API->>KeysService: Get(ctx, store, rawKey)
alt rawKey empty
KeysService-->>API: error ("rawKey is empty")
else Get error
KeysService-->>API: error (early return, no state mutation)
else success
KeysService->>KeysService: inspect Roles/Permissions/Ratelimits
alt field is []byte & non-nil
KeysService->>KeysService: json.Unmarshal (may error)
alt unmarshal error
KeysService-->>API: wrapped internal fault
else success
KeysService-->>API: Key with parsed slices
end
else field not []byte or nil
KeysService-->>API: Key with defaulted empty slices
end
end
API-->>Client: Response or error
sequenceDiagram
participant Caller
participant Auth as zen.Bearer
Caller->>Auth: Bearer(session)
alt session nil
Auth-->>Caller: error ("nil session")
else request nil
Auth-->>Caller: error ("nil request")
else session & request ok
Auth-->>Caller: token, nil
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
✨ 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! 🙏 |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Skipped Deployments
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🔭 Outside diff range comments (1)
go/apps/api/routes/v2_ratelimit_get_override/handler.go (1)
97-106: Wrap non-NotFound errors for consistency with v2_ratelimit_limitHere, non-NotFound errors are returned raw. In v2_ratelimit_limit the same path wraps errors with an internal code and a public message. Align to avoid leaking low-level error strings and to keep API responses consistent.
- if err != nil { - if db.IsNotFound(err) { - return fault.New("namespace was deleted", - fault.Code(codes.Data.RatelimitNamespace.NotFound.URN()), - fault.Public("This namespace does not exist."), - ) - } - - return err - } + if err != nil { + if db.IsNotFound(err) { + return fault.New("namespace was deleted", + fault.Code(codes.Data.RatelimitNamespace.NotFound.URN()), + fault.Public("This namespace does not exist."), + ) + } + return fault.Wrap(err, + fault.Code(codes.App.Internal.UnexpectedError.URN()), + fault.Public("An unexpected error occurred while fetching the namespace."), + ) + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (12)
go/apps/api/routes/v2_keys_add_roles/handler.go(1 hunks)go/apps/api/routes/v2_keys_remove_roles/handler.go(1 hunks)go/apps/api/routes/v2_keys_set_roles/handler.go(1 hunks)go/apps/api/routes/v2_permissions_get_role/handler.go(1 hunks)go/apps/api/routes/v2_permissions_list_roles/handler.go(1 hunks)go/apps/api/routes/v2_ratelimit_get_override/handler.go(1 hunks)go/apps/api/routes/v2_ratelimit_limit/handler.go(1 hunks)go/internal/services/keys/create_test.go(1 hunks)go/internal/services/keys/get.go(2 hunks)go/internal/services/keys/get_test.go(1 hunks)go/pkg/zen/auth.go(1 hunks)go/pkg/zen/auth_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/internal/services/keys/create_test.gogo/internal/services/keys/get_test.gogo/pkg/zen/auth.gogo/apps/api/routes/v2_keys_remove_roles/handler.gogo/apps/api/routes/v2_keys_set_roles/handler.gogo/apps/api/routes/v2_permissions_list_roles/handler.gogo/apps/api/routes/v2_ratelimit_get_override/handler.gogo/internal/services/keys/get.gogo/apps/api/routes/v2_permissions_get_role/handler.gogo/pkg/zen/auth_test.gogo/apps/api/routes/v2_keys_add_roles/handler.gogo/apps/api/routes/v2_ratelimit_limit/handler.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/internal/services/keys/create_test.gogo/internal/services/keys/get_test.gogo/pkg/zen/auth_test.go
**/*.{env,js,ts,go}
📄 CodeRabbit Inference Engine (CLAUDE.md)
All environment variables must follow the format: UNKEY_<SERVICE_NAME>_VARNAME
Files:
go/internal/services/keys/create_test.gogo/internal/services/keys/get_test.gogo/pkg/zen/auth.gogo/apps/api/routes/v2_keys_remove_roles/handler.gogo/apps/api/routes/v2_keys_set_roles/handler.gogo/apps/api/routes/v2_permissions_list_roles/handler.gogo/apps/api/routes/v2_ratelimit_get_override/handler.gogo/internal/services/keys/get.gogo/apps/api/routes/v2_permissions_get_role/handler.gogo/pkg/zen/auth_test.gogo/apps/api/routes/v2_keys_add_roles/handler.gogo/apps/api/routes/v2_ratelimit_limit/handler.go
🧠 Learnings (1)
📓 Common learnings
Learnt from: imeyer
PR: unkeyed/unkey#3755
File: .github/workflows/job_detect_changes.yaml:111-114
Timestamp: 2025-08-08T16:10:00.198Z
Learning: Repo: unkeyed/unkey — When imeyer writes “make me an issue” on a PR thread, automatically open a thorough GitHub issue (Summary, Impact, Where, Repro, Observed vs Expected, Acceptance Criteria, Validation Plan, Out of Scope, References), include backlinks to the PR and comment, and assign to imeyer.
Learnt from: imeyer
PR: unkeyed/unkey#3755
File: .github/actions/setup-node/action.yaml:0-0
Timestamp: 2025-08-08T15:10:46.393Z
Learning: Repo: unkeyed/unkey — Preference: If imeyer comments “issue” on a PR thread, automatically open a thorough GitHub issue (Summary, Impact, Where, Observed vs Expected, Acceptance Criteria, Validation Plan, Out of Scope, References), include backlinks to the PR and comment, and assign to imeyer.
🧬 Code Graph Analysis (3)
go/pkg/zen/auth.go (5)
go/pkg/urn/urn.go (2)
New(141-165)URN(12-19)go/pkg/codes/codes.go (1)
Code(70-79)go/pkg/fault/wrap.go (3)
Code(119-133)Internal(75-89)Public(97-111)go/pkg/codes/unkey_auth.go (1)
Auth(44-57)go/pkg/codes/constants_gen.go (1)
URN(5-5)
go/internal/services/keys/get.go (1)
go/pkg/fault/wrap.go (2)
Wrap(25-67)Internal(75-89)
go/pkg/zen/auth_test.go (3)
go/pkg/zen/auth.go (1)
Bearer(23-53)go/pkg/system_errors/errors.go (1)
Error(36-40)go/pkg/zen/session.go (1)
Session(28-42)
🪛 golangci-lint (2.2.2)
go/apps/api/routes/v2_keys_remove_roles/handler.go
222-222: the given struct should be annotated with the json tag
(musttag)
go/apps/api/routes/v2_keys_set_roles/handler.go
284-284: the given struct should be annotated with the json tag
(musttag)
go/apps/api/routes/v2_permissions_list_roles/handler.go
108-108: the given struct should be annotated with the json tag
(musttag)
go/apps/api/routes/v2_permissions_get_role/handler.go
98-98: the given struct should be annotated with the json tag
(musttag)
go/apps/api/routes/v2_keys_add_roles/handler.go
252-252: the given struct should be annotated with the json tag
(musttag)
⏰ 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). (4)
- GitHub Check: Build / Build
- GitHub Check: Test API / API Test Local
- GitHub Check: Test Go API Local / Test
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (6)
go/internal/services/keys/create_test.go (1)
163-163: for-range-10 is supported under Go 1.22+ (you’re on Go 1.24.4)Your
go/go.moddeclares Go 1.24.4 and your CI workflows useactions/setup-go@v5withgo-version-file: ./go/go.mod, sofor range 10 { … }will compile successfully. No fallback needed.go/apps/api/routes/v2_ratelimit_limit/handler.go (1)
98-102: Guarded decode prevents panics; behavior looks goodSafe type-assert + nil-check before json.Unmarshal is correct here. Propagating unmarshal errors via SWR return integrates with the outer fault wrapping.
go/apps/api/routes/v2_ratelimit_get_override/handler.go (1)
80-84: Guarded decode prevents panics; OK to proceedType-check + nil guard before json.Unmarshal is correct; returning the error allows the caller to handle it. See next comment for consistent wrapping at the call site.
go/apps/api/routes/v2_keys_set_roles/handler.go (1)
283-285: Safe decode pattern acknowledged; add musttag nolint and consider centralizing
- Guarding and defaulting to empty list is fine here.
- Add a targeted nolint for musttag on this unmarshal.
- _ = json.Unmarshal(permBytes, &rolePermissions) // Ignore error, default to empty array + _ = json.Unmarshal(permBytes, &rolePermissions) //nolint:musttag // Ignore error, default to empty arrayOptional: a shared decode helper would remove repetition across handlers.
Likely an incorrect or invalid review comment.
go/pkg/zen/auth.go (1)
24-32: Solid nil-guards to prevent panicsEarly returns for nil session/request are correct and align with Missing auth semantics. Good defensive fix.
go/internal/services/keys/get.go (1)
35-37: Correct early-return to avoid nil derefReturning immediately on Get error prevents accessing fields on a nil key. This removes a panic vector. LGTM.
imeyer
force-pushed
the
push-pknprvqtxvsr
branch
from
57f0cd6 to
bc45615
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
go/apps/api/routes/v2_ratelimit_get_override/handler.go (1)
97-106: Wrap non-not-found errors consistently to avoid leaking internalsHere errors after SWR are returned raw. In v2_ratelimit_limit, similar errors are wrapped with an internal code and public message. Align behavior and avoid exposing json.Unmarshal/db errors to clients.
Apply:
- if err != nil { - if db.IsNotFound(err) { - return fault.New("namespace was deleted", - fault.Code(codes.Data.RatelimitNamespace.NotFound.URN()), - fault.Public("This namespace does not exist."), - ) - } - - return err - } + if err != nil { + if db.IsNotFound(err) { + return fault.New("namespace was deleted", + fault.Code(codes.Data.RatelimitNamespace.NotFound.URN()), + fault.Public("This namespace does not exist."), + ) + } + return fault.Wrap(err, + fault.Code(codes.App.Internal.UnexpectedError.URN()), + fault.Public("An unexpected error occurred while fetching the namespace."), + ) + }
♻️ Duplicate comments (12)
go/apps/api/routes/v2_permissions_get_role/handler.go (1)
97-99: Guarded permissions decode LGTM; add a localized nolint for musttagThe safe []byte assertion is good. golangci-lint may flag musttag when unmarshalling into db.Permission. Add a local nolint to avoid touching DB structs.
- _ = json.Unmarshal(permBytes, &rolePermissions) // Ignore error, default to empty array + _ = json.Unmarshal(permBytes, &rolePermissions) //nolint:musttag // Ignore error, default to empty arraygo/apps/api/routes/v2_keys_remove_roles/handler.go (1)
221-223: Guarded permissions decode LGTM; silence musttag locallyAdd a localized nolint to avoid musttag warnings when unmarshalling into db.Permission.
- _ = json.Unmarshal(permBytes, &rolePermissions) // Ignore error, default to empty array + _ = json.Unmarshal(permBytes, &rolePermissions) //nolint:musttag // Ignore error, default to empty arraygo/apps/api/routes/v2_keys_add_roles/handler.go (1)
251-253: Guarded permissions decode LGTM; add nolint for musttag and consider DRY helperSilence musttag where json tags can’t be added to DB structs.
- _ = json.Unmarshal(permBytes, &rolePermissions) // Ignore error, default to empty array + _ = json.Unmarshal(permBytes, &rolePermissions) //nolint:musttag // Ignore error, default to empty arrayOptional: extract a small decodePermissions helper to remove repetition across handlers.
go/apps/api/routes/v2_permissions_list_roles/handler.go (1)
107-109: Guarded decode is correct; add localized nolint for musttagAdd a local nolint to prevent musttag warnings during Unmarshal into db.Permission.
- _ = json.Unmarshal(permBytes, &rolePermissions) // Ignore error, default to empty array + _ = json.Unmarshal(permBytes, &rolePermissions) //nolint:musttag // Ignore error, default to empty arrayConsider a small shared helper to DRY this pattern.
go/pkg/zen/auth.go (1)
24-32: Add AIDEV-SAFETY comment to document intent and semanticsDocument the defensive guard and deliberate use of codes.Auth.Authentication.Missing to normalize absent session/request as “missing” auth.
func Bearer(s *Session) (string, error) { + // AIDEV-SAFETY: Defensive guard to avoid panics and normalize to a consistent auth error path. + // We classify a nil session/request as "missing" authentication inputs (not token format validation). if s == nil { return "", fault.New("nil session", fault.Code(codes.Auth.Authentication.Missing.URN()), fault.Internal("session is nil"), fault.Public("Invalid session.")) } if s.r == nil { return "", fault.New("nil request", fault.Code(codes.Auth.Authentication.Missing.URN()), fault.Internal("session request is nil"), fault.Public("Invalid request.")) }go/pkg/zen/auth_test.go (2)
114-120: Parallelize and assert error code in TestBearer_NilSessionAdd t.Parallel() and verify the auth error code to harden the test.
func TestBearer_NilSession(t *testing.T) { - // Test with nil session - should not panic + t.Parallel() + // Test with nil session - should not panic token, err := Bearer(nil) require.Error(t, err) require.Empty(t, token) require.Contains(t, err.Error(), "nil session") + code, ok := fault.GetCode(err) + require.True(t, ok) + require.Equal(t, codes.Auth.Authentication.Missing.URN(), code) }
122-130: Mirror improvements in TestBearer_NilRequestRun in parallel and assert the Missing auth code.
func TestBearer_NilRequest(t *testing.T) { - // Test with session that has nil request - should not panic + t.Parallel() + // Test with session that has nil request - should not panic sess := &Session{} // r field is nil token, err := Bearer(sess) require.Error(t, err) require.Empty(t, token) require.Contains(t, err.Error(), "nil request") + code, ok := fault.GetCode(err) + require.True(t, ok) + require.Equal(t, codes.Auth.Authentication.Missing.URN(), code) }go/internal/services/keys/get.go (2)
121-152: Add AIDEV-SAFETY comment and attach public-safe messages on unmarshal failuresKeep the defensive defaults, but when failing explicit unmarshals, include a public-safe message for consistent API errors. Also document the intent.
- // Safely handle roles field + // AIDEV-SAFETY: Default to empty slices on missing/non-bytes values to avoid panics. + // Only hard-fail on explicit unmarshal errors and attach a public-safe message. + // Safely handle roles field rolesBytes, ok := key.Roles.([]byte) if !ok || rolesBytes == nil { roles = []string{} // Default to empty array if nil or wrong type } else { err = json.Unmarshal(rolesBytes, &roles) if err != nil { - return nil, emptyLog, fault.Wrap(err, fault.Internal("failed to unmarshal roles")) + return nil, emptyLog, fault.Wrap(err, + fault.Internal("failed to unmarshal roles"), + fault.Public("Key metadata is invalid."), + ) } } // Safely handle permissions field permissionsBytes, ok := key.Permissions.([]byte) if !ok || permissionsBytes == nil { permissions = []string{} // Default to empty array if nil or wrong type } else { err = json.Unmarshal(permissionsBytes, &permissions) if err != nil { - return nil, emptyLog, fault.Wrap(err, fault.Internal("failed to unmarshal permissions")) + return nil, emptyLog, fault.Wrap(err, + fault.Internal("failed to unmarshal permissions"), + fault.Public("Key metadata is invalid."), + ) } } // Safely handle ratelimits field ratelimitsBytes, ok := key.Ratelimits.([]byte) if !ok || ratelimitsBytes == nil { ratelimitArr = []db.KeyFindForVerificationRatelimit{} // Default to empty array if nil or wrong type } else { err = json.Unmarshal(ratelimitsBytes, &ratelimitArr) if err != nil { - return nil, emptyLog, fault.Wrap(err, fault.Internal("failed to unmarshal ratelimits")) + return nil, emptyLog, fault.Wrap(err, + fault.Internal("failed to unmarshal ratelimits"), + fault.Public("Key metadata is invalid."), + ) } }
121-152: Optional: deduplicate guarded unmarshal with a tiny helperReduce repetition and centralize the “empty on non-bytes/nil” policy.
Helper (place near the top of the file):
// AIDEV-SAFETY: Defensive JSON decoding that defaults to empty slices on invalid inputs. func unmarshalOrEmpty[T any](v any) ([]T, error) { b, ok := v.([]byte) if !ok || b == nil { return []T{}, nil } var out []T if err := json.Unmarshal(b, &out); err != nil { return nil, err } return out, nil }Then replace the three blocks:
- // Safely handle roles field - rolesBytes, ok := key.Roles.([]byte) - if !ok || rolesBytes == nil { - roles = []string{} // Default to empty array if nil or wrong type - } else { - err = json.Unmarshal(rolesBytes, &roles) - if err != nil { - return nil, emptyLog, fault.Wrap(err, fault.Internal("failed to unmarshal roles")) - } - } + roles, err = unmarshalOrEmpty[string](key.Roles) + if err != nil { + return nil, emptyLog, fault.Wrap(err, + fault.Internal("failed to unmarshal roles"), + fault.Public("Key metadata is invalid."), + ) + } - // Safely handle permissions field - permissionsBytes, ok := key.Permissions.([]byte) - if !ok || permissionsBytes == nil { - permissions = []string{} // Default to empty array if nil or wrong type - } else { - err = json.Unmarshal(permissionsBytes, &permissions) - if err != nil { - return nil, emptyLog, fault.Wrap(err, fault.Internal("failed to unmarshal permissions")) - } - } + permissions, err = unmarshalOrEmpty[string](key.Permissions) + if err != nil { + return nil, emptyLog, fault.Wrap(err, + fault.Internal("failed to unmarshal permissions"), + fault.Public("Key metadata is invalid."), + ) + } - // Safely handle ratelimits field - ratelimitsBytes, ok := key.Ratelimits.([]byte) - if !ok || ratelimitsBytes == nil { - ratelimitArr = []db.KeyFindForVerificationRatelimit{} // Default to empty array if nil or wrong type - } else { - err = json.Unmarshal(ratelimitsBytes, &ratelimitArr) - if err != nil { - return nil, emptyLog, fault.Wrap(err, fault.Internal("failed to unmarshal ratelimits")) - } - } + ratelimitArr, err = unmarshalOrEmpty[db.KeyFindForVerificationRatelimit](key.Ratelimits) + if err != nil { + return nil, emptyLog, fault.Wrap(err, + fault.Internal("failed to unmarshal ratelimits"), + fault.Public("Key metadata is invalid."), + ) + }go/internal/services/keys/get_test.go (3)
10-26: Assert Missing auth error code for nil session pathVerify the specific auth code to lock in error classification.
key, log, err := s.GetRootKey(ctx, nil) require.Error(t, err) require.Nil(t, key) require.NotNil(t, log) + code, ok := fault.GetCode(err) + require.True(t, ok) + require.Equal(t, codes.Auth.Authentication.Missing.URN(), code)
28-44: Misnamed test: it calls Get, not GetRootKey — rename to reflect intentAlso deduplicate with the next test of the same behavior.
-func TestGetRootKey_WithEmptyRawKey_ReturnsError(t *testing.T) { +func TestGet_WithEmptyRawKey_ReturnsError(t *testing.T) {
46-59: Remove duplicate test blockThis duplicates the previous test’s assertions for s.Get(ctx, nil, "").
-func TestGet_WithEmptyRawKey_ReturnsError(t *testing.T) { - t.Parallel() - - // Test the assert.NotEmpty validation path directly in Get function - s := &service{} - ctx := context.Background() - - key, log, err := s.Get(ctx, nil, "") - - require.Error(t, err) - require.Nil(t, key) - require.NotNil(t, log) - require.Contains(t, err.Error(), "rawKey is empty") -}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (12)
go/apps/api/routes/v2_keys_add_roles/handler.go(1 hunks)go/apps/api/routes/v2_keys_remove_roles/handler.go(1 hunks)go/apps/api/routes/v2_keys_set_roles/handler.go(1 hunks)go/apps/api/routes/v2_permissions_get_role/handler.go(1 hunks)go/apps/api/routes/v2_permissions_list_roles/handler.go(1 hunks)go/apps/api/routes/v2_ratelimit_get_override/handler.go(1 hunks)go/apps/api/routes/v2_ratelimit_limit/handler.go(1 hunks)go/internal/services/keys/create_test.go(1 hunks)go/internal/services/keys/get.go(2 hunks)go/internal/services/keys/get_test.go(1 hunks)go/pkg/zen/auth.go(1 hunks)go/pkg/zen/auth_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/auth_test.gogo/apps/api/routes/v2_keys_remove_roles/handler.gogo/apps/api/routes/v2_permissions_get_role/handler.gogo/internal/services/keys/get.gogo/pkg/zen/auth.gogo/internal/services/keys/get_test.gogo/apps/api/routes/v2_keys_set_roles/handler.gogo/apps/api/routes/v2_ratelimit_get_override/handler.gogo/apps/api/routes/v2_permissions_list_roles/handler.gogo/internal/services/keys/create_test.gogo/apps/api/routes/v2_keys_add_roles/handler.gogo/apps/api/routes/v2_ratelimit_limit/handler.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/auth_test.gogo/internal/services/keys/get_test.gogo/internal/services/keys/create_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/auth_test.gogo/apps/api/routes/v2_keys_remove_roles/handler.gogo/apps/api/routes/v2_permissions_get_role/handler.gogo/internal/services/keys/get.gogo/pkg/zen/auth.gogo/internal/services/keys/get_test.gogo/apps/api/routes/v2_keys_set_roles/handler.gogo/apps/api/routes/v2_ratelimit_get_override/handler.gogo/apps/api/routes/v2_permissions_list_roles/handler.gogo/internal/services/keys/create_test.gogo/apps/api/routes/v2_keys_add_roles/handler.gogo/apps/api/routes/v2_ratelimit_limit/handler.go
🧠 Learnings (1)
📓 Common learnings
Learnt from: imeyer
PR: unkeyed/unkey#3755
File: .github/workflows/job_detect_changes.yaml:111-114
Timestamp: 2025-08-08T16:10:00.198Z
Learning: Repo: unkeyed/unkey — When imeyer writes “make me an issue” on a PR thread, automatically open a thorough GitHub issue (Summary, Impact, Where, Repro, Observed vs Expected, Acceptance Criteria, Validation Plan, Out of Scope, References), include backlinks to the PR and comment, and assign to imeyer.
Learnt from: imeyer
PR: unkeyed/unkey#3755
File: .github/actions/setup-node/action.yaml:0-0
Timestamp: 2025-08-08T15:10:46.393Z
Learning: Repo: unkeyed/unkey — Preference: If imeyer comments “issue” on a PR thread, automatically open a thorough GitHub issue (Summary, Impact, Where, Observed vs Expected, Acceptance Criteria, Validation Plan, Out of Scope, References), include backlinks to the PR and comment, and assign to imeyer.
Learnt from: imeyer
PR: unkeyed/unkey#3755
File: .github/workflows/runbook-freshness-check.yaml:157-173
Timestamp: 2025-08-08T14:59:52.245Z
Learning: Repo unkeyed/unkey: When a CI/workflow fix is deferred, imeyer prefers a thorough GitHub issue be opened with sections (Summary, Impact, Where, Repro, Observed vs Expected, Acceptance Criteria, Validation Plan, Out of Scope, References) and assigned to imeyer, including backlinks to the originating PR and comment.
🧬 Code Graph Analysis (3)
go/pkg/zen/auth_test.go (3)
go/pkg/zen/auth.go (1)
Bearer(23-53)go/pkg/system_errors/errors.go (1)
Error(36-40)go/pkg/zen/session.go (1)
Session(28-42)
go/internal/services/keys/get.go (1)
go/pkg/fault/wrap.go (2)
Wrap(25-67)Internal(75-89)
go/pkg/zen/auth.go (4)
go/pkg/zen/server.go (1)
New(70-121)go/pkg/codes/codes.go (1)
Code(70-79)go/pkg/fault/wrap.go (3)
Code(119-133)Internal(75-89)Public(97-111)go/pkg/codes/unkey_auth.go (1)
Auth(44-57)
🪛 golangci-lint (2.2.2)
go/apps/api/routes/v2_keys_remove_roles/handler.go
222-222: the given struct should be annotated with the json tag
(musttag)
go/apps/api/routes/v2_permissions_get_role/handler.go
98-98: the given struct should be annotated with the json tag
(musttag)
go/apps/api/routes/v2_keys_set_roles/handler.go
284-284: the given struct should be annotated with the json tag
(musttag)
go/apps/api/routes/v2_permissions_list_roles/handler.go
108-108: the given struct should be annotated with the json tag
(musttag)
go/apps/api/routes/v2_keys_add_roles/handler.go
252-252: the given struct should be annotated with the json tag
(musttag)
⏰ 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). (4)
- GitHub Check: Test API / API Test Local
- GitHub Check: Test Go API Local / Test
- GitHub Check: Build / Build
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
go/internal/services/keys/create_test.go (1)
163-163: Confirm Go toolchain supports range-over-int (Go 1.22+)The loop
for range 10 {…}at go/internal/services/keys/create_test.go:163 requires Go 1.22 or newer. Since there’s no go.mod in the repo, please verify that your CI and all developer environments use Go ≥1.22. If you must remain compatible with earlier versions, revert to a classic index loop:- for range 10 { + for i := 0; i < 10; i++ {go/apps/api/routes/v2_ratelimit_get_override/handler.go (1)
80-84: Safe Overrides decode LGTMThe []byte assertion + nil-check prevents panics; returning JSON errors is fine since the caller can wrap. See follow-up on consistent error wrapping below.
go/internal/services/keys/get.go (1)
35-37: Early return prevents session mutation on error — LGTMThis avoids mutating sess.WorkspaceID when Get fails.
imeyer
force-pushed
the
push-pknprvqtxvsr
branch
from
bc45615 to
f808a3e
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (11)
go/apps/api/routes/v2_ratelimit_limit/handler.go (1)
98-102: Guarded Overrides decode is correct; consider DRYing via a tiny helper (repeat)Safe type-assert + nil-check avoids panics; returning the unmarshal error is right. To remove repetition across ratelimit handlers, extract a small helper.
Apply within this range:
- overrides := make([]db.FindRatelimitNamespaceLimitOverride, 0) - if overrideBytes, ok := response.Overrides.([]byte); ok && overrideBytes != nil { - err = json.Unmarshal(overrideBytes, &overrides) - if err != nil { - return result, err - } - } + overrides := make([]db.FindRatelimitNamespaceLimitOverride, 0) + if err := decodeutil.Overrides(response.Overrides, &overrides); err != nil { + return result, err + }And outside this hunk, add an import and a tiny helper (example):
// import "github.com/unkeyed/unkey/go/internal/decodeutil"Helper example (place under go/internal/decodeutil/json.go):
package decodeutil import "encoding/json" func Overrides(raw any, out *[]db.FindRatelimitNamespaceLimitOverride) error { if b, ok := raw.([]byte); ok && b != nil { return json.Unmarshal(b, out) } *out = (*out)[:0] return nil }go/apps/api/routes/v2_permissions_get_role/handler.go (1)
97-99: Silence musttag on unmarshal and keep the guarded decodeThe guarded []byte assertion is good. golangci-lint musttag complains when unmarshalling into db.Permission; add a localized nolint.
- if permBytes, ok := role.Permissions.([]byte); ok && permBytes != nil { - _ = json.Unmarshal(permBytes, &rolePermissions) // Ignore error, default to empty array - } + if permBytes, ok := role.Permissions.([]byte); ok && permBytes != nil { + _ = json.Unmarshal(permBytes, &rolePermissions) //nolint:musttag // Ignore error, default to empty array + }Optional: centralize this decode in a small helper to remove duplication across handlers.
go/apps/api/routes/v2_keys_add_roles/handler.go (1)
251-253: Add nolint for musttag on permissions decode; helper recommendedSafe guard avoids panics. Silence musttag locally and consider DRY.
- if permBytes, ok := role.Permissions.([]byte); ok && permBytes != nil { - _ = json.Unmarshal(permBytes, &rolePermissions) // Ignore error, default to empty array - } + if permBytes, ok := role.Permissions.([]byte); ok && permBytes != nil { + _ = json.Unmarshal(permBytes, &rolePermissions) //nolint:musttag // Ignore error, default to empty array + }go/apps/api/routes/v2_keys_remove_roles/handler.go (1)
221-223: Localize musttag suppression on JSON unmarshal; decode pattern is fineKeep the guard, add nolint for linter, and consider a shared helper.
- if permBytes, ok := role.Permissions.([]byte); ok && permBytes != nil { - _ = json.Unmarshal(permBytes, &rolePermissions) // Ignore error, default to empty array - } + if permBytes, ok := role.Permissions.([]byte); ok && permBytes != nil { + _ = json.Unmarshal(permBytes, &rolePermissions) //nolint:musttag // Ignore error, default to empty array + }go/apps/api/routes/v2_permissions_list_roles/handler.go (1)
107-109: Silence musttag locally; decode pattern is robustKeep the guard. Add a localized nolint and optionally move into a shared helper to remove duplication.
- if permBytes, ok := role.Permissions.([]byte); ok && permBytes != nil { - _ = json.Unmarshal(permBytes, &rolePermissions) // Ignore error, default to empty array - } + if permBytes, ok := role.Permissions.([]byte); ok && permBytes != nil { + _ = json.Unmarshal(permBytes, &rolePermissions) //nolint:musttag // Ignore error, default to empty array + }go/pkg/zen/auth.go (1)
24-32: Add an AIDEV-SAFETY comment to document intent and error semanticsDocument these guards as defensive and clarify Missing vs Malformed classification for consistency.
func Bearer(s *Session) (string, error) { + // AIDEV-SAFETY: Defensive guard to avoid panics and normalize auth errors. + // We classify nil session as "missing" inputs and nil request as "malformed" session state. if s == nil { return "", fault.New("nil session", fault.Code(codes.Auth.Authentication.Missing.URN()), fault.Internal("session is nil"), fault.Public("Invalid session.")) } if s.r == nil { return "", fault.New("nil request", fault.Code(codes.Auth.Authentication.Malformed.URN()), fault.Internal("session request is nil"), fault.Public("Invalid request.")) }go/apps/api/routes/v2_keys_set_roles/handler.go (2)
283-292: Good defensive parse + AIDEV-SAFETY logging addedThe guarded assertion, least-privilege default, and debug log on JSON parse failure look correct and align with prior guidance. Nice.
283-292: Nit: also guard on length to skip empty []byte silentlyEmpty slices can still trigger a JSON EOF and noisy logs. Prefer checking length too.
- if permBytes, ok := role.Permissions.([]byte); ok && permBytes != nil { + if permBytes, ok := role.Permissions.([]byte); ok && len(permBytes) > 0 {go/internal/services/keys/get.go (2)
121-152: Optional: dedupe guarded unmarshal with a small helperThe three guarded blocks are identical; consider a generic helper to reduce repetition and centralize policy.
// e.g., in this package or a small internal helper func unmarshalOrEmpty[T any](v any) ([]T, error) { b, ok := v.([]byte) if !ok || len(b) == 0 { return []T{}, nil } var out []T if err := json.Unmarshal(b, &out); err != nil { return nil, err } return out, nil } // Usage: roles, err = unmarshalOrEmpty[string](key.Roles) permissions, err = unmarshalOrEmpty[string](key.Permissions) ratelimitArr, err = unmarshalOrEmpty[db.KeyFindForVerificationRatelimit](key.Ratelimits)
121-152: Add AIDEV-SAFETY intent and public-safe messages for unmarshal failuresPer repo guidelines (AIDEV-* comments) and consistency with other faults, attach a public-safe message and document defensive intent.
- // Safely handle roles field + // AIDEV-SAFETY: Default to empty slices on missing/non-bytes values to avoid panics and + // treat as least-privilege. Only hard-fail on explicit unmarshal errors. + // Safely handle roles field @@ - if err != nil { - return nil, emptyLog, fault.Wrap(err, fault.Internal("failed to unmarshal roles")) + if err != nil { + return nil, emptyLog, fault.Wrap(err, + fault.Internal("failed to unmarshal roles"), + fault.Public("Key metadata is invalid."), + ) } @@ - if err != nil { - return nil, emptyLog, fault.Wrap(err, fault.Internal("failed to unmarshal permissions")) + if err != nil { + return nil, emptyLog, fault.Wrap(err, + fault.Internal("failed to unmarshal permissions"), + fault.Public("Key metadata is invalid."), + ) } @@ - if err != nil { - return nil, emptyLog, fault.Wrap(err, fault.Internal("failed to unmarshal ratelimits")) + if err != nil { + return nil, emptyLog, fault.Wrap(err, + fault.Internal("failed to unmarshal ratelimits"), + fault.Public("Key metadata is invalid."), + ) }go/internal/services/keys/get_test.go (1)
35-66: Duplicate tests exercising the same Get(ctx, nil, "") path; dedupe/renameBoth tests call Get with empty rawKey and assert the same conditions; one is misnamed. Keep a single test.
-func TestGetRootKey_WithEmptyRawKey_ReturnsError(t *testing.T) { +func TestGet_WithEmptyRawKey_ReturnsError(t *testing.T) { @@ } -func TestGet_WithEmptyRawKey_ReturnsError(t *testing.T) { - t.Parallel() - - // Test the assert.NotEmpty validation path directly in Get function - s := &service{} - ctx := context.Background() - - key, log, err := s.Get(ctx, nil, "") - - require.Error(t, err) - require.Nil(t, key) - require.NotNil(t, log) - require.Contains(t, err.Error(), "rawKey is empty") -}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (12)
go/apps/api/routes/v2_keys_add_roles/handler.go(1 hunks)go/apps/api/routes/v2_keys_remove_roles/handler.go(1 hunks)go/apps/api/routes/v2_keys_set_roles/handler.go(1 hunks)go/apps/api/routes/v2_permissions_get_role/handler.go(1 hunks)go/apps/api/routes/v2_permissions_list_roles/handler.go(1 hunks)go/apps/api/routes/v2_ratelimit_get_override/handler.go(1 hunks)go/apps/api/routes/v2_ratelimit_limit/handler.go(1 hunks)go/internal/services/keys/create_test.go(1 hunks)go/internal/services/keys/get.go(2 hunks)go/internal/services/keys/get_test.go(1 hunks)go/pkg/zen/auth.go(1 hunks)go/pkg/zen/auth_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/apps/api/routes/v2_keys_add_roles/handler.gogo/apps/api/routes/v2_keys_remove_roles/handler.gogo/apps/api/routes/v2_ratelimit_limit/handler.gogo/internal/services/keys/create_test.gogo/internal/services/keys/get.gogo/apps/api/routes/v2_permissions_get_role/handler.gogo/pkg/zen/auth_test.gogo/apps/api/routes/v2_keys_set_roles/handler.gogo/internal/services/keys/get_test.gogo/apps/api/routes/v2_ratelimit_get_override/handler.gogo/pkg/zen/auth.gogo/apps/api/routes/v2_permissions_list_roles/handler.go
**/*.{env,js,ts,go}
📄 CodeRabbit Inference Engine (CLAUDE.md)
All environment variables must follow the format: UNKEY_<SERVICE_NAME>_VARNAME
Files:
go/apps/api/routes/v2_keys_add_roles/handler.gogo/apps/api/routes/v2_keys_remove_roles/handler.gogo/apps/api/routes/v2_ratelimit_limit/handler.gogo/internal/services/keys/create_test.gogo/internal/services/keys/get.gogo/apps/api/routes/v2_permissions_get_role/handler.gogo/pkg/zen/auth_test.gogo/apps/api/routes/v2_keys_set_roles/handler.gogo/internal/services/keys/get_test.gogo/apps/api/routes/v2_ratelimit_get_override/handler.gogo/pkg/zen/auth.gogo/apps/api/routes/v2_permissions_list_roles/handler.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/internal/services/keys/create_test.gogo/pkg/zen/auth_test.gogo/internal/services/keys/get_test.go
🧠 Learnings (5)
📓 Common learnings
Learnt from: imeyer
PR: unkeyed/unkey#3755
File: .github/actions/setup-node/action.yaml:0-0
Timestamp: 2025-08-08T15:10:46.393Z
Learning: Repo: unkeyed/unkey — Preference: If imeyer comments “issue” on a PR thread, automatically open a thorough GitHub issue (Summary, Impact, Where, Observed vs Expected, Acceptance Criteria, Validation Plan, Out of Scope, References), include backlinks to the PR and comment, and assign to imeyer.
Learnt from: imeyer
PR: unkeyed/unkey#3755
File: .github/workflows/job_detect_changes.yaml:111-114
Timestamp: 2025-08-08T16:10:00.198Z
Learning: Repo: unkeyed/unkey — When imeyer writes “make me an issue” on a PR thread, automatically open a thorough GitHub issue (Summary, Impact, Where, Repro, Observed vs Expected, Acceptance Criteria, Validation Plan, Out of Scope, References), include backlinks to the PR and comment, and assign to imeyer.
Learnt from: imeyer
PR: unkeyed/unkey#3755
File: .github/workflows/runbook-freshness-check.yaml:157-173
Timestamp: 2025-08-08T14:59:52.245Z
Learning: Repo unkeyed/unkey: When a CI/workflow fix is deferred, imeyer prefers a thorough GitHub issue be opened with sections (Summary, Impact, Where, Repro, Observed vs Expected, Acceptance Criteria, Validation Plan, Out of Scope, References) and assigned to imeyer, including backlinks to the originating PR and comment.
📚 Learning: 2025-08-08T16:10:00.198Z
Learnt from: imeyer
PR: unkeyed/unkey#3755
File: .github/workflows/job_detect_changes.yaml:111-114
Timestamp: 2025-08-08T16:10:00.198Z
Learning: Repo: unkeyed/unkey — When imeyer writes “make me an issue” on a PR thread, automatically open a thorough GitHub issue (Summary, Impact, Where, Repro, Observed vs Expected, Acceptance Criteria, Validation Plan, Out of Scope, References), include backlinks to the PR and comment, and assign to imeyer.
Applied to files:
go/pkg/zen/auth_test.go
📚 Learning: 2025-08-08T14:59:52.245Z
Learnt from: imeyer
PR: unkeyed/unkey#3755
File: .github/workflows/runbook-freshness-check.yaml:157-173
Timestamp: 2025-08-08T14:59:52.245Z
Learning: Repo unkeyed/unkey: When a CI/workflow fix is deferred, imeyer prefers a thorough GitHub issue be opened with sections (Summary, Impact, Where, Repro, Observed vs Expected, Acceptance Criteria, Validation Plan, Out of Scope, References) and assigned to imeyer, including backlinks to the originating PR and comment.
Applied to files:
go/pkg/zen/auth_test.go
📚 Learning: 2025-08-08T15:10:46.393Z
Learnt from: imeyer
PR: unkeyed/unkey#3755
File: .github/actions/setup-node/action.yaml:0-0
Timestamp: 2025-08-08T15:10:46.393Z
Learning: Repo: unkeyed/unkey — Preference: If imeyer comments “issue” on a PR thread, automatically open a thorough GitHub issue (Summary, Impact, Where, Observed vs Expected, Acceptance Criteria, Validation Plan, Out of Scope, References), include backlinks to the PR and comment, and assign to imeyer.
Applied to files:
go/pkg/zen/auth_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/internal/services/keys/get_test.go
🧬 Code Graph Analysis (4)
go/internal/services/keys/get.go (1)
go/pkg/fault/wrap.go (2)
Wrap(25-67)Internal(75-89)
go/pkg/zen/auth_test.go (3)
go/pkg/zen/auth.go (1)
Bearer(23-53)go/pkg/fault/code.go (1)
GetCode(21-35)go/pkg/zen/session.go (1)
Session(28-42)
go/internal/services/keys/get_test.go (3)
go/pkg/system_errors/errors.go (1)
Error(36-40)go/pkg/fault/code.go (1)
GetCode(21-35)go/pkg/codes/unkey_auth.go (1)
Auth(44-57)
go/pkg/zen/auth.go (6)
go/pkg/zen/server.go (1)
New(70-121)go/pkg/fault/wrapped.go (1)
New(52-66)go/pkg/codes/codes.go (1)
Code(70-79)go/pkg/fault/wrap.go (3)
Code(119-133)Internal(75-89)Public(97-111)go/pkg/codes/unkey_auth.go (1)
Auth(44-57)go/pkg/codes/constants_gen.go (1)
URN(5-5)
🪛 golangci-lint (2.2.2)
go/apps/api/routes/v2_keys_add_roles/handler.go
252-252: the given struct should be annotated with the json tag
(musttag)
go/apps/api/routes/v2_keys_remove_roles/handler.go
222-222: the given struct should be annotated with the json tag
(musttag)
go/apps/api/routes/v2_permissions_get_role/handler.go
98-98: the given struct should be annotated with the json tag
(musttag)
go/apps/api/routes/v2_keys_set_roles/handler.go
286-286: the given struct should be annotated with the json tag
(musttag)
go/apps/api/routes/v2_permissions_list_roles/handler.go
108-108: the given struct should be annotated with the json tag
(musttag)
⏰ 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). (5)
- GitHub Check: Test Go API Local / Test
- GitHub Check: Test API / API Test Local
- GitHub Check: Build / Build
- GitHub Check: Analyze (actions)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (6)
go/pkg/zen/auth_test.go (2)
114-126: LGTM: NilSession test asserts code and runs in parallelCovers error message and URN (Missing); matches implementation.
128-142: LGTM: NilRequest test asserts code and runs in parallelAsserts Malformed URN; consistent with auth.go.
go/apps/api/routes/v2_keys_set_roles/handler.go (1)
283-292: Exclude generated DB models from musttag lint – JSON tags not required
ThePermissionstruct ingo/pkg/db/models_generated.go(and its related DB model structs) is auto-generated for database operations and only carriesdb:"…"tags. It isn’t marshalled/unmarshalled viaencoding/json. The OpenAPI types ingo/apps/api/openapi/gen.goalready have the properjsontags.Action items:
- Exempt
*_generated.gofiles from themusttaglinter in your.golangci.yml(or disablemusttagfor generated code).No code changes needed here.
Likely an incorrect or invalid review comment.
go/internal/services/keys/get.go (1)
35-38: Early return prevents state mutation on Get errorGood fix. Avoids touching sess.WorkspaceID when Get fails. This eliminates the prior panic/mutation risk.
go/internal/services/keys/create_test.go (1)
163-163: Go 1.24.4 supportsfor range 10– no change neededThe module’s
go.modspecifiesgo 1.24.4, which is ≥ 1.22, so thefor range 10 {…}syntax is valid and CI will pass.go/internal/services/keys/get_test.go (1)
29-33: LGTM: Asserting Missing auth code strengthens the testGood addition to verify the fault code for nil session.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
go/apps/api/routes/v2_ratelimit_get_override/handler.go(3 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/apps/api/routes/v2_ratelimit_get_override/handler.go
**/*.{env,js,ts,go}
📄 CodeRabbit Inference Engine (CLAUDE.md)
All environment variables must follow the format: UNKEY_<SERVICE_NAME>_VARNAME
Files:
go/apps/api/routes/v2_ratelimit_get_override/handler.go
🧠 Learnings (1)
📓 Common learnings
Learnt from: imeyer
PR: unkeyed/unkey#3755
File: .github/actions/setup-node/action.yaml:0-0
Timestamp: 2025-08-08T15:10:46.393Z
Learning: Repo: unkeyed/unkey — Preference: If imeyer comments “issue” on a PR thread, automatically open a thorough GitHub issue (Summary, Impact, Where, Observed vs Expected, Acceptance Criteria, Validation Plan, Out of Scope, References), include backlinks to the PR and comment, and assign to imeyer.
Learnt from: imeyer
PR: unkeyed/unkey#3755
File: .github/workflows/job_detect_changes.yaml:111-114
Timestamp: 2025-08-08T16:10:00.198Z
Learning: Repo: unkeyed/unkey — When imeyer writes “make me an issue” on a PR thread, automatically open a thorough GitHub issue (Summary, Impact, Where, Repro, Observed vs Expected, Acceptance Criteria, Validation Plan, Out of Scope, References), include backlinks to the PR and comment, and assign to imeyer.
Learnt from: imeyer
PR: unkeyed/unkey#3755
File: .github/workflows/runbook-freshness-check.yaml:157-173
Timestamp: 2025-08-08T14:59:52.245Z
Learning: Repo unkeyed/unkey: When a CI/workflow fix is deferred, imeyer prefers a thorough GitHub issue be opened with sections (Summary, Impact, Where, Repro, Observed vs Expected, Acceptance Criteria, Validation Plan, Out of Scope, References) and assigned to imeyer, including backlinks to the originating PR and comment.
🧬 Code Graph Analysis (1)
go/apps/api/routes/v2_ratelimit_get_override/handler.go (5)
go/pkg/fault/wrap.go (4)
Wrap(25-67)Code(119-133)Internal(75-89)Public(97-111)go/pkg/codes/codes.go (1)
Code(70-79)go/pkg/codes/unkey_application.go (1)
App(53-71)go/pkg/codes/constants_gen.go (1)
URN(5-5)go/pkg/urn/urn.go (1)
URN(12-19)
⏰ 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). (4)
- GitHub Check: Test API / API Test Local
- GitHub Check: Build / Build
- GitHub Check: Test Go API Local / Test
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
go/apps/api/routes/v2_ratelimit_get_override/handler.go (1)
115-117: Resolved: unexpected SWR errors now wrapped consistentlyThis matches the earlier guidance: NotFound is handled explicitly; all other SWR errors are wrapped with an internal code and a safe public message. This restores consistent fault semantics across routes.
|
👍 Review approved for @imeyer |
|
|
Graphite Automations"Post a GIF when PR approved" took an action on this PR • (08/11/25)1 gif was posted to this PR based on Andreas Thomas's automation. |
3 participants
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
How should this be tested?
Checklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated
Summary by CodeRabbit
Bug Fixes
Tests