feat: use new credits tables in API v1#4191
Closed
Flo4604 wants to merge 3 commits intofeat/identity-credits-schemafrom
Closed
feat: use new credits tables in API v1#4191Flo4604 wants to merge 3 commits intofeat/identity-credits-schemafrom
Flo4604 wants to merge 3 commits intofeat/identity-credits-schemafrom
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Member
Author
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
This was referenced Oct 28, 2025
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis PR introduces a credits-based remaining/refill system for API keys. It migrates remaining and refill tracking from the keys table to a new credits table, implements dual-path logic to support both legacy keys and new credits throughout the codebase, and updates API routes, usage limiters, and cache structures accordingly. Changes
Sequence DiagramsequenceDiagram
participant Client
participant API Route
participant Key Service
participant DB
participant Usage Limiter
participant Durable Object
Client->>API Route: Request with key
activate API Route
API Route->>Key Service: getData()
activate Key Service
Key Service->>DB: Fetch key with credits
DB-->>Key Service: key + credits
Key Service-->>API Route: {key, credits}
deactivate Key Service
rect rgb(200, 220, 255)
Note over API Route,API Route: Determine remaining source
alt Credits exist
API Route->>API Route: remaining = credits.remaining
else
API Route->>API Route: remaining = key.remaining
end
end
rect rgb(220, 255, 220)
Note over Client,API Route: On key verification
Client->>API Route: verifyKey(key, cost)
API Route->>Usage Limiter: limit(creditId or keyId, cost)
end
activate Usage Limiter
Usage Limiter->>Durable Object: POST /limit
activate Durable Object
alt creditId provided
Durable Object->>DB: Fetch credit record (lazy)
DB-->>Durable Object: credit
Durable Object->>Durable Object: Deduct cost from credit.remaining
Durable Object->>DB: Update credit.remaining
Durable Object-->>Usage Limiter: {valid: true/false, remaining}
else keyId provided (legacy)
Durable Object->>DB: Fetch key (lazy)
DB-->>Durable Object: key
Durable Object->>Durable Object: Deduct cost from key.remaining
Durable Object->>DB: Update key.remaining
Durable Object-->>Usage Limiter: {valid: true/false, remaining}
end
deactivate Durable Object
Usage Limiter-->>API Route: response
deactivate Usage Limiter
API Route-->>Client: Verification result with updated remaining
deactivate API Route
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring extra attention:
Possibly related PRs
Suggested labels
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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. Comment |
Flo4604
changed the title
18 tasks
Flo4604
force-pushed
the
feat/identity-credits-api-v1
branch
from
c300f90 to
aa1950a
Compare
Flo4604
force-pushed
the
feat/identity-credits-api-v1
branch
from
aa1950a to
baf6928
Compare
Flo4604
force-pushed
the
feat/identity-credits-schema
branch
from
b943287 to
ae7385a
Compare
18 tasks
Flo4604
force-pushed
the
feat/identity-credits-api-v1
branch
from
3369e55 to
8d4a3a4
Compare
Flo4604
force-pushed
the
feat/identity-credits-schema
branch
from
ae7385a to
3715c57
Compare
Flo4604
force-pushed
the
feat/identity-credits-api-v1
branch
from
0f0c40f to
775d35a
Compare
Member
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
apps/api/src/pkg/usagelimit/interface.ts (1)
3-26: Enforce identifier presence at the schema levelWith both
keyIdandcreditIdoptional, these schemas now accept payloads that contain neither identifier. Downstream code assumes at least one is present and only fails later at runtime, so we should reject such requests up front. Adding arefine(or equivalent) to require at least one identifier for both limit and revalidate requests would restore the earlier validation guarantees.-export const limitRequestSchema = z.object({ +export const limitRequestSchema = z + .object({ /** * For legacy key-based credits stored in keys.remaining */ keyId: z.string().optional(), /** * For new credits system stored in credits table. * When present, this takes precedence over keyId. */ creditId: z.string().optional(), cost: z.number(), -}); + }) + .refine((req) => Boolean(req.creditId ?? req.keyId), { + message: "Either creditId or keyId must be provided.", + }); ... -export const revalidateRequestSchema = z.object({ - keyId: z.string().optional(), - creditId: z.string().optional(), -}); +export const revalidateRequestSchema = z + .object({ + keyId: z.string().optional(), + creditId: z.string().optional(), + }) + .refine((req) => Boolean(req.creditId ?? req.keyId), { + message: "Either creditId or keyId must be provided.", + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
apps/api/src/pkg/cache/index.ts(1 hunks)apps/api/src/pkg/cache/namespaces.ts(4 hunks)apps/api/src/pkg/keys/service.ts(5 hunks)apps/api/src/pkg/usagelimit/client.ts(1 hunks)apps/api/src/pkg/usagelimit/durable_object.ts(4 hunks)apps/api/src/pkg/usagelimit/interface.ts(2 hunks)apps/api/src/routes/v1_apis_listKeys.happy.test.ts(1 hunks)apps/api/src/routes/v1_apis_listKeys.ts(6 hunks)apps/api/src/routes/v1_keys_createKey.happy.test.ts(2 hunks)apps/api/src/routes/v1_keys_createKey.ts(4 hunks)apps/api/src/routes/v1_keys_deleteKey.happy.test.ts(1 hunks)apps/api/src/routes/v1_keys_deleteKey.ts(4 hunks)apps/api/src/routes/v1_keys_getKey.happy.test.ts(1 hunks)apps/api/src/routes/v1_keys_getKey.ts(4 hunks)apps/api/src/routes/v1_keys_getVerifications.ts(2 hunks)apps/api/src/routes/v1_keys_updateKey.happy.test.ts(3 hunks)apps/api/src/routes/v1_keys_updateKey.ts(6 hunks)apps/api/src/routes/v1_keys_updateRemaining.happy.test.ts(5 hunks)apps/api/src/routes/v1_keys_updateRemaining.ts(5 hunks)apps/api/src/routes/v1_keys_verifyKey.test.ts(2 hunks)apps/api/src/routes/v1_keys_whoami.happy.test.ts(1 hunks)apps/api/src/routes/v1_keys_whoami.ts(5 hunks)apps/api/src/routes/v1_migrations_createKey.ts(5 hunks)
🧰 Additional context used
🧠 Learnings (24)
📓 Common learnings
Learnt from: Flo4604
PR: unkeyed/unkey#4190
File: go/internal/services/keys/verifier.go:51-53
Timestamp: 2025-10-30T15:10:52.710Z
Learning: PR #4190 for unkeyed/unkey is focused solely on database schema and query changes for identity-based credits. It adds IdentityCredits and KeyCredits fields to structs and queries, but does not implement the priority enforcement logic in the usagelimiter. The logic implementation is intentionally deferred to a later PR in the stack.
Learnt from: Flo4604
PR: unkeyed/unkey#3785
File: go/apps/api/routes/v2_keys_reroll_key/401_test.go:52-61
Timestamp: 2025-08-14T16:25:48.167Z
Learning: User Flo4604 requested creation of a GitHub issue to track converting all test files to use table-driven test patterns as a broader codebase improvement, following the suggestion made during review of go/apps/api/routes/v2_keys_reroll_key/401_test.go.
Learnt from: Flo4604
PR: unkeyed/unkey#3800
File: go/internal/services/usagelimiter/redis.go:176-186
Timestamp: 2025-08-19T09:46:03.702Z
Learning: Zero-cost requests in the usage limiter should not change the remaining credits but should still return the actual remaining credits for that key, not treat the key as unlimited. A key with 100 credits remaining should still report 100 credits remaining after a zero-cost request.
📚 Learning: 2025-10-30T15:10:52.710Z
Learnt from: Flo4604
PR: unkeyed/unkey#4190
File: go/internal/services/keys/verifier.go:51-53
Timestamp: 2025-10-30T15:10:52.710Z
Learning: PR #4190 for unkeyed/unkey is focused solely on database schema and query changes for identity-based credits. It adds IdentityCredits and KeyCredits fields to structs and queries, but does not implement the priority enforcement logic in the usagelimiter. The logic implementation is intentionally deferred to a later PR in the stack.
Applied to files:
apps/api/src/routes/v1_keys_updateRemaining.happy.test.tsapps/api/src/routes/v1_keys_whoami.happy.test.tsapps/api/src/pkg/usagelimit/client.tsapps/api/src/routes/v1_keys_updateKey.tsapps/api/src/routes/v1_keys_getKey.happy.test.tsapps/api/src/routes/v1_migrations_createKey.tsapps/api/src/routes/v1_keys_deleteKey.happy.test.tsapps/api/src/pkg/cache/namespaces.tsapps/api/src/pkg/keys/service.tsapps/api/src/routes/v1_apis_listKeys.happy.test.tsapps/api/src/routes/v1_keys_updateRemaining.tsapps/api/src/routes/v1_keys_createKey.happy.test.tsapps/api/src/routes/v1_keys_whoami.tsapps/api/src/pkg/usagelimit/durable_object.tsapps/api/src/routes/v1_keys_verifyKey.test.tsapps/api/src/routes/v1_keys_updateKey.happy.test.tsapps/api/src/routes/v1_apis_listKeys.tsapps/api/src/routes/v1_keys_getVerifications.tsapps/api/src/routes/v1_keys_deleteKey.tsapps/api/src/pkg/usagelimit/interface.tsapps/api/src/routes/v1_keys_getKey.tsapps/api/src/routes/v1_keys_createKey.ts
📚 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:
apps/api/src/routes/v1_keys_updateRemaining.happy.test.tsapps/api/src/pkg/usagelimit/client.tsapps/api/src/routes/v1_keys_updateKey.tsapps/api/src/routes/v1_migrations_createKey.tsapps/api/src/routes/v1_keys_updateRemaining.tsapps/api/src/routes/v1_keys_whoami.tsapps/api/src/routes/v1_keys_updateKey.happy.test.tsapps/api/src/routes/v1_apis_listKeys.tsapps/api/src/routes/v1_keys_getVerifications.tsapps/api/src/routes/v1_keys_deleteKey.tsapps/api/src/pkg/usagelimit/interface.tsapps/api/src/routes/v1_keys_getKey.tsapps/api/src/routes/v1_keys_createKey.ts
📚 Learning: 2025-08-14T16:25:48.167Z
Learnt from: Flo4604
PR: unkeyed/unkey#3785
File: go/apps/api/routes/v2_keys_reroll_key/401_test.go:52-61
Timestamp: 2025-08-14T16:25:48.167Z
Learning: User Flo4604 requested creation of a GitHub issue to track converting all test files to use table-driven test patterns as a broader codebase improvement, following the suggestion made during review of go/apps/api/routes/v2_keys_reroll_key/401_test.go.
Applied to files:
apps/api/src/routes/v1_keys_updateRemaining.happy.test.tsapps/api/src/routes/v1_keys_whoami.happy.test.tsapps/api/src/routes/v1_keys_getKey.happy.test.tsapps/api/src/routes/v1_apis_listKeys.happy.test.tsapps/api/src/routes/v1_keys_createKey.happy.test.tsapps/api/src/routes/v1_keys_verifyKey.test.ts
📚 Learning: 2025-10-21T09:45:47.560Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#4107
File: go/pkg/clickhouse/key_verifications_test.go:20-20
Timestamp: 2025-10-21T09:45:47.560Z
Learning: In ClickHouse tests (e.g., go/pkg/clickhouse/key_verifications_test.go), parallel execution with t.Parallel() is safe when tests use workspaceID-based isolation. Each test generates a unique workspaceID (via uid.New(uid.WorkspacePrefix)), and all database operations are scoped to that workspaceID, providing logical isolation even when tests share a single ClickHouse instance.
Applied to files:
apps/api/src/routes/v1_keys_updateRemaining.happy.test.ts
📚 Learning: 2025-08-19T09:46:03.702Z
Learnt from: Flo4604
PR: unkeyed/unkey#3800
File: go/internal/services/usagelimiter/redis.go:176-186
Timestamp: 2025-08-19T09:46:03.702Z
Learning: Zero-cost requests in the usage limiter should not change the remaining credits but should still return the actual remaining credits for that key, not treat the key as unlimited. A key with 100 credits remaining should still report 100 credits remaining after a zero-cost request.
Applied to files:
apps/api/src/routes/v1_keys_updateRemaining.happy.test.tsapps/api/src/pkg/usagelimit/client.tsapps/api/src/routes/v1_keys_updateKey.tsapps/api/src/pkg/keys/service.tsapps/api/src/routes/v1_keys_updateRemaining.tsapps/api/src/routes/v1_keys_verifyKey.test.tsapps/api/src/routes/v1_keys_updateKey.happy.test.tsapps/api/src/pkg/usagelimit/interface.ts
📚 Learning: 2025-08-21T15:54:45.198Z
Learnt from: chronark
PR: unkeyed/unkey#3825
File: go/internal/services/usagelimiter/limit.go:38-0
Timestamp: 2025-08-21T15:54:45.198Z
Learning: In go/internal/services/usagelimiter/limit.go, the UpdateKeyCreditsDecrement operation cannot be safely wrapped with db.WithRetry due to the lack of idempotency mechanisms in the current tech stack. Retrying this non-idempotent write operation risks double-charging users if the first attempt commits but the client sees a transient error.
Applied to files:
apps/api/src/pkg/usagelimit/client.tsapps/api/src/routes/v1_keys_updateKey.tsapps/api/src/pkg/keys/service.tsapps/api/src/routes/v1_keys_updateRemaining.ts
📚 Learning: 2025-08-20T11:41:36.718Z
Learnt from: Flo4604
PR: unkeyed/unkey#3810
File: go/apps/api/routes/v2_ratelimit_limit/handler.go:54-56
Timestamp: 2025-08-20T11:41:36.718Z
Learning: In the Unkey codebase, the X-Unkey-Metrics: disabled header is used by the v1 API when replaying ratelimit requests to the v2 API (go/apps/api/routes/v2_ratelimit_limit/handler.go) to prevent double billing/logging to ClickHouse. This is a legitimate internal service-to-service communication pattern for API migration scenarios, not intended for external client usage.
Applied to files:
apps/api/src/pkg/usagelimit/client.ts
📚 Learning: 2024-10-23T12:05:31.121Z
Learnt from: chronark
PR: unkeyed/unkey#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.
Applied to files:
apps/api/src/pkg/usagelimit/client.tsapps/api/src/pkg/usagelimit/interface.ts
📚 Learning: 2025-08-19T09:42:40.919Z
Learnt from: Flo4604
PR: unkeyed/unkey#3800
File: go/internal/services/keys/validation.go:45-52
Timestamp: 2025-08-19T09:42:40.919Z
Learning: In go/internal/services/keys/validation.go, keys with unlimited usage (RemainingRequests.Valid = false) have an early return that bypasses the usage limiter entirely. The usage limiter is only called for keys with finite remaining request counts.
Applied to files:
apps/api/src/pkg/usagelimit/client.tsapps/api/src/routes/v1_keys_updateKey.ts
📚 Learning: 2025-08-22T12:48:58.289Z
Learnt from: perkinsjr
PR: unkeyed/unkey#3775
File: apps/dashboard/lib/trpc/routers/api/keys/query-key-usage-timeseries/index.ts:1-1
Timestamp: 2025-08-22T12:48:58.289Z
Learning: The team at Unkey is planning to move from TRPC to calling their v2 API directly. They're comfortable keeping TRPC input schemas imported from app route folders as technical debt since this code will be replaced, rather than refactoring to move schemas to lib/schemas.
Applied to files:
apps/api/src/routes/v1_keys_updateKey.tsapps/api/src/routes/v1_keys_updateRemaining.ts
📚 Learning: 2025-08-22T12:49:20.668Z
Learnt from: perkinsjr
PR: unkeyed/unkey#3775
File: apps/dashboard/lib/trpc/routers/api/setDefaultBytes.ts:1-1
Timestamp: 2025-08-22T12:49:20.668Z
Learning: The team at unkeyed/unkey is comfortable with keeping TRPC schema imports from app route folders as temporary technical debt during their migration from TRPC to calling their v2 API directly, since this code will be replaced rather than maintained long-term.
Applied to files:
apps/api/src/routes/v1_keys_updateKey.tsapps/api/src/routes/v1_keys_updateRemaining.ts
📚 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:
apps/api/src/routes/v1_keys_updateKey.tsapps/api/src/pkg/keys/service.tsapps/api/src/routes/v1_keys_updateRemaining.tsapps/api/src/routes/v1_keys_whoami.tsapps/api/src/routes/v1_keys_getKey.ts
📚 Learning: 2025-08-21T12:37:40.996Z
Learnt from: Flo4604
PR: unkeyed/unkey#3821
File: apps/dashboard/lib/trpc/routers/key/updateRootKeyPermissions.ts:74-74
Timestamp: 2025-08-21T12:37:40.996Z
Learning: Root keys in Unkey have two workspace fields: `workspaceId` (always set to env().UNKEY_WORKSPACE_ID for the Unkey workspace that owns the key) and `forWorkspaceId` (set to ctx.workspace.id for the user's workspace that the key is for). When querying root keys, the system filters by forWorkspaceId to get keys for the current user's workspace, but the returned rootKey.workspaceId is always the Unkey workspace ID.
Applied to files:
apps/api/src/routes/v1_migrations_createKey.tsapps/api/src/routes/v1_keys_createKey.ts
📚 Learning: 2025-08-29T13:48:43.353Z
Learnt from: Flo4604
PR: unkeyed/unkey#3884
File: go/apps/api/routes/v2_ratelimit_delete_override/handler.go:218-228
Timestamp: 2025-08-29T13:48:43.353Z
Learning: In the unkeyed/unkey codebase, when working with ratelimit namespace caching, req.Namespace parameter is either the namespace ID or the namespace name, so cache invalidation only needs to remove cache keys for namespace.ID and namespace.Name - no need to also remove req.Namespace as a separate key.
Applied to files:
apps/api/src/pkg/cache/namespaces.ts
📚 Learning: 2025-08-22T12:50:07.024Z
Learnt from: perkinsjr
PR: unkeyed/unkey#3775
File: apps/dashboard/lib/trpc/routers/api/keys/query-overview-timeseries/index.ts:1-2
Timestamp: 2025-08-22T12:50:07.024Z
Learning: The team at Unkey is comfortable accepting TRPC schema imports from app route folders in server-side TRPC routers as technical debt, since they're planning to migrate away from TRPC to calling their v2 API directly, making these imports temporary and not worth refactoring.
Applied to files:
apps/api/src/routes/v1_keys_updateRemaining.ts
📚 Learning: 2025-04-30T15:25:33.917Z
Learnt from: mcstepp
PR: unkeyed/unkey#3210
File: apps/dashboard/app/new/page.tsx:3-3
Timestamp: 2025-04-30T15:25:33.917Z
Learning: There are two different `getAuth` functions in the Unkey codebase with different purposes:
1. `@/lib/auth/get-auth` - Base function without redirects, used in special cases on the dashboard where redirect control is needed (like `/new` page) and within tRPC context
2. `@/lib/auth` - Helper function with redirects, used in most dashboard cases (approximately 98%)
Applied to files:
apps/api/src/routes/v1_keys_updateRemaining.ts
📚 Learning: 2025-09-15T19:53:28.487Z
Learnt from: mcstepp
PR: unkeyed/unkey#3952
File: go/proto/ctrl/v1/routing.proto:0-0
Timestamp: 2025-09-15T19:53:28.487Z
Learning: In the Unkey codebase, authentication/authorization errors intentionally return NOT_FOUND error codes instead of distinct auth error codes (like FORBIDDEN or UNAUTHORIZED) for security reasons. This prevents attackers from distinguishing between "resource doesn't exist" and "you don't have permission to access this resource", avoiding information disclosure about workspace existence and access boundaries.
Applied to files:
apps/api/src/routes/v1_keys_updateRemaining.tsapps/api/src/routes/v1_apis_listKeys.ts
📚 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:
apps/api/src/routes/v1_keys_whoami.tsapps/api/src/pkg/usagelimit/durable_object.ts
📚 Learning: 2024-11-13T19:06:36.786Z
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:
apps/api/src/routes/v1_keys_verifyKey.test.ts
📚 Learning: 2025-04-22T11:48:39.670Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3156
File: apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/ratelimit-setup.tsx:36-47
Timestamp: 2025-04-22T11:48:39.670Z
Learning: The Unkey dashboard's form validation for numeric values like rate limits is handled through the Zod schema validation (with `.positive()` validators and additional checks in `superRefine`), rather than HTML input attributes like `min`.
Applied to files:
apps/api/src/pkg/usagelimit/interface.ts
📚 Learning: 2025-08-17T22:54:06.787Z
Learnt from: perkinsjr
PR: unkeyed/unkey#3798
File: apps/dashboard/app/(app)/apis/[apiId]/_overview/hooks/use-spent-credits.ts:118-121
Timestamp: 2025-08-17T22:54:06.787Z
Learning: In the Unkey dashboard, real-time data updates are prioritized over caching optimizations. The 10-second refetch interval for spent credits and similar metrics is intentional and used consistently across the codebase to provide real-time dashboard functionality.
Applied to files:
apps/api/src/routes/v1_keys_getKey.ts
📚 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:
apps/api/src/routes/v1_keys_createKey.ts
📚 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:
apps/api/src/routes/v1_keys_createKey.ts
🔇 Additional comments (5)
apps/api/src/pkg/cache/index.ts (1)
35-39: Cache buster bump keeps remote caches honest.Moving to v10 will invalidate stale Cloudflare entries once this ships, so existing deployments see the new credits-aware payload immediately. Nice catch.
apps/api/src/routes/v1_keys_deleteKey.happy.test.ts (1)
81-144: Great coverage for the new cascade.This happy-path test verifies the permanent delete pulls the matching credits row as well, proving the new cleanup logic. Thanks for wiring the watchdog in integration tests.
apps/api/src/routes/v1_keys_getVerifications.ts (1)
103-124: Cache shape update matched here too.Adding
creditsto the key fetch keeps the cached payload aligned with the new schema so downstream callers don’t see partial data. Appreciate the consistency.apps/api/src/routes/v1_keys_deleteKey.ts (1)
74-148: Cascade cleanup looks thorough.Pulling in credits, ratelimits, encrypted keys, permissions, and roles inside the permanent delete transaction avoids orphaned rows after the switch to the new tables. Solid hardening.
apps/api/src/routes/v1_keys_getKey.happy.test.ts (1)
70-189: Love the precedence matrix.These three scenarios (new credits, legacy-only, and override) spell out the intended behavior and will catch regressions while teams migrate. Nice work.
Comment on lines
+395
to
+398
| refillDay: req.refill?.interval === "daily" ? null : (req?.refill?.refillDay ?? 1), | ||
| refilledAt: req.refill?.interval ? Date.now() : null, | ||
| updatedAt: null, | ||
| }); |
Contributor
There was a problem hiding this comment.
Do not assign a monthly refill day when no refill is configured
Because the ternary falls through to req?.refill?.refillDay ?? 1, every key that sets remaining but omits refill will still write refillDay = 1. That marks the credit as having a monthly schedule even though the caller never asked for one, so the limiter/migration logic will think it should auto-refill on day 1. Please guard the default so we only set a day when refill is present.
- refillDay: req.refill?.interval === "daily" ? null : (req?.refill?.refillDay ?? 1),
+ refillDay: req.refill
+ ? req.refill.interval === "daily"
+ ? null
+ : req.refill.refillDay ?? 1
+ : null,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| refillDay: req.refill?.interval === "daily" ? null : (req?.refill?.refillDay ?? 1), | |
| refilledAt: req.refill?.interval ? Date.now() : null, | |
| updatedAt: null, | |
| }); | |
| refillDay: req.refill | |
| ? req.refill.interval === "daily" | |
| ? null | |
| : req.refill.refillDay ?? 1 | |
| : null, | |
| refilledAt: req.refill?.interval ? Date.now() : null, | |
| updatedAt: null, | |
| }); |
🤖 Prompt for AI Agents
In apps/api/src/routes/v1_keys_createKey.ts around lines 395 to 398, the current
ternary lets refillDay fall through to (req?.refill?.refillDay ?? 1) even when
req.refill is missing, causing keys with no refill to be marked as monthly;
change the assignment so refillDay is null unless req.refill is present, e.g.
guard the default with a top-level check (if req.refill then use interval ===
"daily" ? null : (req.refill.refillDay ?? 1) else null) so no refillDay is
written when no refill is configured.
Comment on lines
+183
to
+204
| if (hasOldCredits) { | ||
| await db.primary | ||
| .update(schema.keys) | ||
| .set({ | ||
| remaining: req.value, | ||
| }) | ||
| .where(eq(schema.keys.id, req.keyId)); | ||
| } else if (hasNewCredits) { | ||
| await db.primary | ||
| .update(schema.keys) | ||
| .set({ | ||
| remaining: req.value, | ||
| }) | ||
| .where(eq(schema.keys.id, req.keyId)); | ||
| } else { | ||
| await db.primary.insert(schema.credits).values({ | ||
| id: newId("credit"), | ||
| keyId: req.keyId, | ||
| remaining: req.value as number, | ||
| workspaceId: auth.authorizedWorkspaceId, | ||
| createdAt: Date.now(), | ||
| refilledAt: Date.now(), |
Contributor
There was a problem hiding this comment.
Update the credits row when handling set for new-table keys.
For keys already migrated to the credits table, we currently write to schema.keys.remaining. That column stays NULL on new keys, so the API returns 200 but the credits row never changes, leaving remaining untouched and the response stale. Please update schema.credits (and only that) when hasNewCredits is true.
Use this diff to target the correct table:
- } else if (hasNewCredits) {
- await db.primary
- .update(schema.keys)
- .set({
- remaining: req.value,
- })
- .where(eq(schema.keys.id, req.keyId));
+ } else if (hasNewCredits) {
+ await db.primary
+ .update(schema.credits)
+ .set({
+ remaining: req.value,
+ })
+ .where(eq(schema.credits.id, key.credits.id));This ensures set actually mutates the active credits balance for migrated keys.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/api/src/routes/v1_keys_updateRemaining.ts around lines 183 to 204, the
hasNewCredits branch is incorrectly updating schema.keys.remaining (which is
NULL for migrated keys) instead of updating the credits table; change that
branch to update schema.credits, setting remaining to req.value and use a WHERE
that targets the credits row (e.g., eq(schema.credits.keyId, req.keyId)),
leaving the hasOldCredits branch as-is and not touching schema.keys in the
hasNewCredits case.
Comment on lines
+489
to
+498
| id: newId("credit"), | ||
| remaining: key.remaining, | ||
| workspaceId: authorizedWorkspaceId, | ||
| createdAt: Date.now(), | ||
| keyId: key.keyId, | ||
| identityId: null, | ||
| refillAmount: key.refill?.amount ?? null, | ||
| refillDay: key.refill?.interval === "daily" ? null : (key?.refill?.refillDay ?? 1), | ||
| refilledAt: key.refill?.interval ? Date.now() : null, | ||
| updatedAt: null, |
Contributor
There was a problem hiding this comment.
Guard the refillDay default during migrations
Here too, when a migrated key lacks a refill payload, the ?? 1 default forces refillDay = 1, so we create a credit row that looks like it should refill monthly. Keys that previously had no auto-refill will suddenly be scheduled. Please mirror the conditional from the create route so we only default refillDay when refill was actually supplied.
- refillDay: key.refill?.interval === "daily" ? null : (key?.refill?.refillDay ?? 1),
+ refillDay: key.refill
+ ? key.refill.interval === "daily"
+ ? null
+ : key.refill.refillDay ?? 1
+ : null,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| id: newId("credit"), | |
| remaining: key.remaining, | |
| workspaceId: authorizedWorkspaceId, | |
| createdAt: Date.now(), | |
| keyId: key.keyId, | |
| identityId: null, | |
| refillAmount: key.refill?.amount ?? null, | |
| refillDay: key.refill?.interval === "daily" ? null : (key?.refill?.refillDay ?? 1), | |
| refilledAt: key.refill?.interval ? Date.now() : null, | |
| updatedAt: null, | |
| id: newId("credit"), | |
| remaining: key.remaining, | |
| workspaceId: authorizedWorkspaceId, | |
| createdAt: Date.now(), | |
| keyId: key.keyId, | |
| identityId: null, | |
| refillAmount: key.refill?.amount ?? null, | |
| refillDay: key.refill | |
| ? key.refill.interval === "daily" | |
| ? null | |
| : key.refill.refillDay ?? 1 | |
| : null, | |
| refilledAt: key.refill?.interval ? Date.now() : null, | |
| updatedAt: null, |
🤖 Prompt for AI Agents
In apps/api/src/routes/v1_migrations_createKey.ts around lines 489 to 498, the
migration code unconditionally uses "?? 1" for refillDay which sets refillDay =
1 for keys that have no refill payload; change the expression to only apply the
default when a refill object exists—i.e. if key.refill is present then set
refillDay = key.refill.interval === "daily" ? null : (key.refill.refillDay ??
1), otherwise set refillDay = null—so migrated keys without refill remain
non-scheduled.
62afa56 to
7017b5a
Compare
Flo4604
force-pushed
the
feat/identity-credits-api-v1
branch
from
775d35a to
c6a547e
Compare
Contributor
|
Thank you for following the naming conventions for pull request titles! 🙏 |
Flo4604
force-pushed
the
feat/identity-credits-api-v1
branch
from
7fe43e5 to
fb08a80
Compare
chronark
approved these changes
Nov 3, 2025
ogzhanolguncu
approved these changes
Nov 3, 2025
|
|
Graphite Automations"Post a GIF when PR approved" took an action on this PR • (11/03/25)1 gif was posted to this PR based on Andreas Thomas's automation. |
Flo4604
force-pushed
the
feat/identity-credits-schema
branch
from
7017b5a to
84cf2ac
Compare
Flo4604
force-pushed
the
feat/identity-credits-api-v1
branch
from
53c3ced to
139d760
Compare
- Update cache to support credit-based invalidation - Enhance usage limiter to work with both key and credit IDs - Update all v1 key endpoints to support new credits table - Add comprehensive tests for credit operations (create, update, verify) - Implement double-read strategy (credits table takes precedence over legacy remaining) - Support refill configuration in credits table
Flo4604
force-pushed
the
feat/identity-credits-api-v1
branch
from
139d760 to
ab009c0
Compare
Flo4604
force-pushed
the
feat/identity-credits-schema
branch
from
84cf2ac to
8277f35
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
This migrates the v1 api to the in the first pr mentioned table, its done in a way that will allow us to migrate after deploying all the code everywhere at some later point.
Important: The V1 API will NOT have access to identity based credits, if someone wants them they can go ahead and upgrade to v2.
permanentlyremoves the data from the other tables now, we didn't do this before for some reason.The only thing I didn't update to use this new schema is the migrate workflow which I dont think we are using anyways.
The schema HAS to be created in planetscale before deploying this.
Type of change
How should this be tested?
Make sure the v1 api still works.
Creating a new key inserts into credtis schema and works when verifying the key.
Updating Key -> remaining/refill setting it removing it etc | Works also with key(s) that got created before the new table
Updating Credits still works -> set increment / decrement | Works also with key(s) that got created before the new table
Verifying a key works with keys that have the new / old system.
pnpm vitest run -c vitest.integration.ts keys --exclude="**/*{ratelimit,multilimit}*" --bail=1Checklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated