refactor: remove orgId from getApiKeyDetails#316
Merged
sweetmantech merged 7 commits intotestfrom Mar 19, 2026
Merged
Conversation
All API keys are personal — org context is no longer derived from the key itself. Access to other accounts is determined at access-check time via shared org membership (canAccessAccount). - getApiKeyDetails: removed org membership lookup, returns only accountId - validateAuthContext: removed getApiKeyDetails call and orgId tracking - validateAccountIdOverride: passes currentAccountId to canAccessAccount - orgId in AuthContext now only set from explicit organizationId input - Updated all tests Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR removes organization ID derivation from API-key authentication across the codebase. API keys no longer resolve their organization context; Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/auth/validateAuthContext.ts (1)
85-97:⚠️ Potential issue | 🟠 MajorPass
organization_idcontext to account override validationThe override validation receives
orgId: null, which causescanAccessAccountto immediately returnfalse(per its implementation). This blocks cross-account overrides even when a validorganization_idis explicitly provided in the request body. The organization_id validation at line ~102 is unreachable due to the early 403 return.Pass the request's organization_id to the override check for consistency with other validation paths (e.g.,
validateGetTaskRunQuery.ts):Fix
// Handle account_id override from request body if (input.accountId) { const overrideResult = await validateAccountIdOverride({ currentAccountId: accountId, targetAccountId: input.accountId, - orgId: null, + orgId: input.organizationId ?? null, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/auth/validateAuthContext.ts` around lines 85 - 97, The override check is passing orgId: null which causes canAccessAccount to deny overrides; update the validateAccountIdOverride call in validateAuthContext.ts to pass the request's organization identifier (e.g., orgId: input.organization_id) instead of null so the override path can validate organization-level access consistently; adjust the validateAccountIdOverride invocation that sets currentAccountId, targetAccountId, orgId to use input.organization_id (or the correct request field name) so later organization checks are reachable.
🧹 Nitpick comments (3)
lib/auth/validateAuthContext.ts (2)
27-28: Stale comment — API keys no longer provide org contextThe comment states org context comes "from API key or request body," but with this refactor, API keys are strictly personal and never contribute org context. Consider updating to reflect the new architecture.
📝 Suggested documentation fix
- /** The organization context (from API key or request body) */ + /** The organization context (from request body only) */ orgId: string | null;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/auth/validateAuthContext.ts` around lines 27 - 28, Update the stale doc comment on the orgId field in validateAuthContext.ts to remove reference to API keys supplying org context and reflect the current architecture where org context only comes from the request body (or other request-bound sources); locate the orgId declaration/comment (the "orgId: string | null" field) and change the comment to state that orgId is derived from the request body or other request-scoped data and not from API keys.
39-39: Outdated docstring — "org keys" no longer existLine 39 references "org keys can access member accounts," but with the refactor, all keys are personal. This comment should be updated to reflect the current access model.
📝 Suggested documentation fix
- * 3. Validating account_id override access (org keys can access member accounts, personal keys can access own account) + * 3. Validating account_id override access (via org membership when organization_id is provided)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/auth/validateAuthContext.ts` at line 39, The docstring in validateAuthContext.ts incorrectly mentions "org keys can access member accounts"; update that comment to reflect the refactor where all keys are personal by removing the "org keys" wording and clearly stating the current model (e.g., "personal keys can access their own account; account_id override requires appropriate membership/permission checks"). Edit the comment near the validation step that currently reads "Validating account_id override access (org keys can access member accounts, personal keys can access own account)" so it accurately describes the new access rules used by the validateAuthContext logic.lib/auth/validateAccountIdOverride.ts (1)
13-14: Consider clarifying the "always null now" commentThe comment "null for personal keys — always null now" is accurate but slightly confusing. It would be clearer to explain the architectural reason briefly.
📝 Clearer documentation suggestion
- /** The organization ID (null for personal keys — always null now) */ + /** The organization ID from request context (null when not explicitly provided) */ orgId: string | null;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/auth/validateAccountIdOverride.ts` around lines 13 - 14, Update the inline comment on the orgId property in validateAccountIdOverride (the type/interface that declares "orgId: string | null") to replace "null for personal keys — always null now" with a concise explanation of the architecture reason (e.g., that organization-scoped API keys are no longer issued and all keys are personal, so orgId is currently unused and always null); edit the comment directly above the orgId declaration to be short, clear, and factual so future readers understand why it will always be null.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@lib/auth/validateAuthContext.ts`:
- Around line 85-97: The override check is passing orgId: null which causes
canAccessAccount to deny overrides; update the validateAccountIdOverride call in
validateAuthContext.ts to pass the request's organization identifier (e.g.,
orgId: input.organization_id) instead of null so the override path can validate
organization-level access consistently; adjust the validateAccountIdOverride
invocation that sets currentAccountId, targetAccountId, orgId to use
input.organization_id (or the correct request field name) so later organization
checks are reachable.
---
Nitpick comments:
In `@lib/auth/validateAccountIdOverride.ts`:
- Around line 13-14: Update the inline comment on the orgId property in
validateAccountIdOverride (the type/interface that declares "orgId: string |
null") to replace "null for personal keys — always null now" with a concise
explanation of the architecture reason (e.g., that organization-scoped API keys
are no longer issued and all keys are personal, so orgId is currently unused and
always null); edit the comment directly above the orgId declaration to be short,
clear, and factual so future readers understand why it will always be null.
In `@lib/auth/validateAuthContext.ts`:
- Around line 27-28: Update the stale doc comment on the orgId field in
validateAuthContext.ts to remove reference to API keys supplying org context and
reflect the current architecture where org context only comes from the request
body (or other request-bound sources); locate the orgId declaration/comment (the
"orgId: string | null" field) and change the comment to state that orgId is
derived from the request body or other request-scoped data and not from API
keys.
- Line 39: The docstring in validateAuthContext.ts incorrectly mentions "org
keys can access member accounts"; update that comment to reflect the refactor
where all keys are personal by removing the "org keys" wording and clearly
stating the current model (e.g., "personal keys can access their own account;
account_id override requires appropriate membership/permission checks"). Edit
the comment near the validation step that currently reads "Validating account_id
override access (org keys can access member accounts, personal keys can access
own account)" so it accurately describes the new access rules used by the
validateAuthContext logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7a963f42-d00c-45d6-9b43-6fa53a5bbe83
⛔ Files ignored due to path filters (2)
lib/auth/__tests__/validateAuthContext.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/keys/__tests__/getApiKeyDetails.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (3)
lib/auth/validateAccountIdOverride.tslib/auth/validateAuthContext.tslib/keys/getApiKeyDetails.ts
Missed this file in the orgId removal — was still referencing keyDetails.orgId which no longer exists. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
orgId is always null now — no need to accept it as a parameter. Hardcode null in the canAccessAccount call instead. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
sweetmantech
commented
Mar 19, 2026
|
|
||
| const hasAccess = await canAccessAccount({ | ||
| orgId: keyDetails.orgId, | ||
| orgId: null, |
Contributor
Author
There was a problem hiding this comment.
How is orgId used in canAccessAccount?
(reply to this comment with your answer)
Contributor
Author
There was a problem hiding this comment.
orgId is never non-null in this context anymore. It's passed as null to canAccessAccount which then falls through to the currentAccountId shared-org membership path. Will remove the explicit orgId: null param from canAccessAccount in a follow-up once we clean up all callers.
| targetAccountId, | ||
| }); | ||
| const hasAccess = await canAccessAccount({ | ||
| orgId: null, |
Contributor
Author
There was a problem hiding this comment.
Same question here. In what context is orgId not null when calling canAccessAccount?
Contributor
Author
There was a problem hiding this comment.
Already removed in the latest commit (3ffaab2) — orgId is no longer a param on validateAccountIdOverride. It's hardcoded as null internally when calling canAccessAccount.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/auth/validateAuthContext.ts (1)
113-116: Update theAuthContext.orgIddocs to match Line 115.Line 115 now makes
orgIdrequest-scoped only, but the interface/JSDoc above still says it comes from “API key or request body.” Tightening that contract here will make the new semantics much clearer for downstream callers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/auth/validateAuthContext.ts` around lines 113 - 116, Update the AuthContext JSDoc/interface to reflect that orgId is request-scoped and comes from the incoming request (input.organizationId) rather than the API key; modify the comment above the AuthContext declaration and any JSDoc on the AuthContext.type/interface to state "orgId: request-scoped (from request body or query) or null" and remove the assertion that it is derived from the API key. Ensure references to orgId in validateAuthContext and any exported AuthContext type/signature align with this new contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/accounts/validateOverrideAccountId.ts`:
- Around line 59-63: canAccessAccount is being called with orgId: null and an
unexpected currentAccountId, causing both a type error and a logic bug; update
the CanAccessAccountParams in lib/organizations/canAccessAccount.ts to include
an optional currentAccountId and implement the cross-organization membership
validation inside canAccessAccount (handle cases where orgId may be provided
from the caller rather than forcing falsy rejection), then change the call in
validateOverrideAccountId (the call that passes orgId: null, targetAccountId,
currentAccountId: keyDetails.accountId) to pass the real orgId (e.g.,
keyDetails.orgId) and the currentAccountId so canAccessAccount can perform the
correct access check for override requests.
---
Nitpick comments:
In `@lib/auth/validateAuthContext.ts`:
- Around line 113-116: Update the AuthContext JSDoc/interface to reflect that
orgId is request-scoped and comes from the incoming request
(input.organizationId) rather than the API key; modify the comment above the
AuthContext declaration and any JSDoc on the AuthContext.type/interface to state
"orgId: request-scoped (from request body or query) or null" and remove the
assertion that it is derived from the API key. Ensure references to orgId in
validateAuthContext and any exported AuthContext type/signature align with this
new contract.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 81e0aa5c-9f86-4897-be9b-1331b9a963fd
⛔ Files ignored due to path filters (1)
lib/accounts/__tests__/validateOverrideAccountId.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (3)
lib/accounts/validateOverrideAccountId.tslib/auth/validateAccountIdOverride.tslib/auth/validateAuthContext.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/auth/validateAccountIdOverride.ts
Removed orgId from: - getApiKeyDetails (no longer checks if account is an org) - validateAuthContext (no longer reads orgId from key details) - validateAccountIdOverride (orgId param removed) - McpAuthInfoExtra / verifyApiKey (orgId removed from extra) - All buildGet*Params input interfaces and callers - All MCP tool registrations - validateChatRequest (removed getApiKeyDetails call) orgId in AuthContext is now only set from explicit organizationId input. All access control goes through canAccessAccount with currentAccountId for shared org membership checks. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
lib/pulse/validateGetPulsesRequest.ts (1)
17-31:⚠️ Potential issue | 🟡 MinorStale JSDoc: Org key and admin key behaviors no longer apply.
The documentation at lines 21-23 describes behaviors that have been removed in this refactor:
- "For org keys: Returns orgId for filtering by org membership in database"
- "For Recoup admin key: Returns empty params to indicate ALL pulse records"
These should be updated or removed to reflect that all API keys now behave the same way (returning the authenticated account's pulses).
📝 Suggested JSDoc update
/** * Validates GET /api/pulses request. * Handles authentication via x-api-key or Authorization bearer token. * - * For personal keys: Returns accountIds with the key owner's account - * For org keys: Returns orgId for filtering by org membership in database - * For Recoup admin key: Returns empty params to indicate ALL pulse records + * Returns accountIds scoped to the authenticated account. + * If account_id query param is provided, validates access before returning that account's pulses. * * Query parameters: - * - account_id: Filter to a specific account (validated against org membership) + * - account_id: Filter to a specific account (validated via shared org membership) * - active: Filter by active status (true/false). If undefined, returns all.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/pulse/validateGetPulsesRequest.ts` around lines 17 - 31, Update the JSDoc for validateGetPulsesRequest to remove the stale descriptions about org keys and Recoup admin keys and replace them with the current behavior: all API keys return the authenticated account's pulses (i.e., the function always returns the authenticated account's accountIds). Keep the rest of the doc (query params, active handling, and return type) intact and ensure the summary and param/returns accurately reflect validateGetPulsesRequest's current authentication semantics.lib/artists/buildGetArtistsParams.ts (1)
17-28:⚠️ Potential issue | 🟡 MinorStale JSDoc: Key type distinctions no longer apply.
The documentation still references different behaviors for "personal keys", "org keys", and "Recoup admin key" (lines 20-22), but these distinctions have been removed. All keys now behave identically.
📝 Suggested JSDoc update
/** * Builds the parameters for getArtists based on auth context. * - * For personal keys: Returns the key owner's accountId - * For org keys: Returns the key owner's accountId (can override with targetAccountId) - * For Recoup admin key: Returns the key owner's accountId (can override with targetAccountId) - * + * Returns the authenticated account's artists by default. * If targetAccountId is provided, validates access and returns that account. + * If orgIdFilter is provided, filters to that organization's artists. * * `@param` input - The auth context and optional filters * `@returns` The params for getArtists or an error */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/artists/buildGetArtistsParams.ts` around lines 17 - 28, The JSDoc for buildGetArtistsParams is stale because it references distinct key types ("personal keys", "org keys", "Recoup admin key") that no longer exist; update the comment to describe the current unified behavior: all keys return the key owner's accountId by default, and if a targetAccountId is provided it will be validated and returned instead; mention the input parameter (auth context and optional filters) and that the function returns params for getArtists or an error, and remove any language about different key-type behaviors.lib/artists/validateGetArtistsRequest.ts (1)
16-30:⚠️ Potential issue | 🟡 MinorStale JSDoc: Update to reflect simplified key behavior.
Similar to other validators, the JSDoc references "org keys" and "Recoup admin key" behaviors (lines 21-22) that no longer exist.
📝 Suggested JSDoc update
/** * Validates GET /api/artists request. * Handles authentication via x-api-key or Authorization bearer token. * - * For personal keys/Bearer tokens: Returns the authenticated account's artists - * For org keys: Returns the key owner's artists (can filter by account_id) - * For Recoup admin key: Returns the key owner's artists (can filter by account_id) + * Returns the authenticated account's artists by default. * * Query parameters: - * - account_id: Filter to a specific account (org keys only) + * - account_id: Filter to a specific account (validated via shared org membership) * - org_id: Filter to artists in a specific organization (omit for personal artists)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/artists/validateGetArtistsRequest.ts` around lines 16 - 30, The JSDoc on validateGetArtistsRequest is stale: remove references to "org keys" and "Recoup admin key" and update the summary to match the current simplified key behavior (authentication via x-api-key or Authorization bearer token) stating that requests return the authenticated account's artists; also adjust the Query parameters section to reflect only supported filters (e.g., remove or clarify account_id/org_id if they are no longer applicable) so the top-of-file comment accurately documents validateGetArtistsRequest's current behavior.lib/organizations/validateGetOrganizationsRequest.ts (1)
14-26:⚠️ Potential issue | 🟡 MinorJSDoc is now stale and misleading.
The documentation describes behaviors that have been removed in this refactor:
- "For org keys: Returns organizationId for filtering by org membership"
- "For Recoup admin key: Returns empty params to indicate ALL organization records"
These cases no longer exist since API keys are now personal without org context.
📝 Proposed fix to update documentation
/** * Validates GET /api/organizations request. * Handles authentication via x-api-key or Authorization bearer token. * - * For personal keys: Returns accountId with the key owner's account - * For org keys: Returns organizationId for filtering by org membership in database - * For Recoup admin key: Returns empty params to indicate ALL organization records + * Returns accountId with the key owner's account. + * If account_id query param is provided, validates access via shared org membership. * * Query parameters: * - account_id: Filter to a specific account (validated against org membership) * * `@param` request - The NextRequest object * `@returns` A NextResponse with an error if validation fails, or GetAccountOrganizationsParams */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/organizations/validateGetOrganizationsRequest.ts` around lines 14 - 26, The JSDoc for validateGetOrganizationsRequest is stale: remove or update lines describing org keys and Recoup admin key behavior and any mention of organizationId filtering since API keys are now personal only; adjust the summary and the "For ..." sections to reflect that only personal keys (returning accountId) are supported and that no org-scoped or admin-all behavior exists, and ensure the parameter and return descriptions (GetAccountOrganizationsParams) match the current implementation in validateGetOrganizationsRequest.lib/chats/validateGetChatsRequest.ts (1)
14-27:⚠️ Potential issue | 🟡 MinorJSDoc is stale after refactor.
Same issue as
validateGetOrganizationsRequest.ts— the documentation references removed behaviors for "org keys" and "Recoup admin key" that no longer apply after this refactor.📝 Proposed fix to update documentation
/** * Validates GET /api/chats request. * Handles authentication via x-api-key or Authorization bearer token. * - * For personal keys: Returns accountIds with the key owner's account - * For org keys: Returns orgId for filtering by org membership in database - * For Recoup admin key: Returns empty params to indicate ALL chat records + * Returns account_ids scoped to the key owner's account. + * If account_id query param is provided, validates access via shared org membership. * * Query parameters: * - account_id: Filter to a specific account (validated against org membership) * - artist_account_id: Filter by artist ID * * `@param` request - The NextRequest object * `@returns` A NextResponse with an error if validation fails, or SelectRoomsParams */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/chats/validateGetChatsRequest.ts` around lines 14 - 27, Update the stale JSDoc for validateGetChatsRequest to reflect the current behavior after the refactor: remove references to "org keys" and "Recoup admin key" and any mention of returning orgId or empty params for ALL records, and instead describe the actual authentication/authorization flow and outputs produced by the validateGetChatsRequest function (e.g., which auth headers are supported, what SelectRoomsParams fields are returned like account_id and artist_account_id after validation, and when a NextResponse error is returned); mirror the cleanup performed in validateGetOrganizationsRequest.ts so the doc accurately matches the current implementation and returned values.
🧹 Nitpick comments (2)
lib/sandbox/buildGetSandboxesParams.ts (1)
4-11: Naming convention inconsistency with other param builders.This interface uses
snake_case(account_id,target_account_id) while similar interfaces inbuildGetPulsesParams.tsandbuildGetArtistsParams.tsusecamelCase(accountId,targetAccountId). Consider aligning naming conventions across these param builder interfaces for consistency.This is pre-existing and not blocking, but worth noting for future cleanup.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/sandbox/buildGetSandboxesParams.ts` around lines 4 - 11, The BuildGetSandboxesParamsInput interface uses snake_case property names (account_id, target_account_id, sandbox_id) which is inconsistent with other builders; update the interface to use camelCase names (accountId, targetAccountId, sandboxId) and adjust any callers or destructuring that reference BuildGetSandboxesParamsInput to the new property names (search for usages of BuildGetSandboxesParamsInput and fields in buildGetSandboxesParams, its consumers, and tests) so the parameter builder matches the naming convention used by buildGetPulsesParams and buildGetArtistsParams.lib/chats/buildGetChatsParams.ts (1)
4-11: Minor naming inconsistency with sibling module.The interface uses
snake_case(account_id,target_account_id,artist_id) whileBuildGetOrganizationsParamsInputusescamelCase(accountId,targetAccountId). This appears intentional to align with API parameter conventions, but consider standardizing acrossbuildGet*Paramsmodules for internal consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/chats/buildGetChatsParams.ts` around lines 4 - 11, The BuildGetChatsParamsInput interface uses snake_case field names (account_id, target_account_id, artist_id) while the sibling BuildGetOrganizationsParamsInput uses camelCase; update BuildGetChatsParamsInput to use camelCase (accountId, targetAccountId, artistId) to standardize internal naming, then adjust any callers or serialization logic that expect snake_case (e.g., in functions that call buildGetChatsParams or serialize its output) to map to the API parameter names if needed so external API compatibility is preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@lib/artists/buildGetArtistsParams.ts`:
- Around line 17-28: The JSDoc for buildGetArtistsParams is stale because it
references distinct key types ("personal keys", "org keys", "Recoup admin key")
that no longer exist; update the comment to describe the current unified
behavior: all keys return the key owner's accountId by default, and if a
targetAccountId is provided it will be validated and returned instead; mention
the input parameter (auth context and optional filters) and that the function
returns params for getArtists or an error, and remove any language about
different key-type behaviors.
In `@lib/artists/validateGetArtistsRequest.ts`:
- Around line 16-30: The JSDoc on validateGetArtistsRequest is stale: remove
references to "org keys" and "Recoup admin key" and update the summary to match
the current simplified key behavior (authentication via x-api-key or
Authorization bearer token) stating that requests return the authenticated
account's artists; also adjust the Query parameters section to reflect only
supported filters (e.g., remove or clarify account_id/org_id if they are no
longer applicable) so the top-of-file comment accurately documents
validateGetArtistsRequest's current behavior.
In `@lib/chats/validateGetChatsRequest.ts`:
- Around line 14-27: Update the stale JSDoc for validateGetChatsRequest to
reflect the current behavior after the refactor: remove references to "org keys"
and "Recoup admin key" and any mention of returning orgId or empty params for
ALL records, and instead describe the actual authentication/authorization flow
and outputs produced by the validateGetChatsRequest function (e.g., which auth
headers are supported, what SelectRoomsParams fields are returned like
account_id and artist_account_id after validation, and when a NextResponse error
is returned); mirror the cleanup performed in validateGetOrganizationsRequest.ts
so the doc accurately matches the current implementation and returned values.
In `@lib/organizations/validateGetOrganizationsRequest.ts`:
- Around line 14-26: The JSDoc for validateGetOrganizationsRequest is stale:
remove or update lines describing org keys and Recoup admin key behavior and any
mention of organizationId filtering since API keys are now personal only; adjust
the summary and the "For ..." sections to reflect that only personal keys
(returning accountId) are supported and that no org-scoped or admin-all behavior
exists, and ensure the parameter and return descriptions
(GetAccountOrganizationsParams) match the current implementation in
validateGetOrganizationsRequest.
In `@lib/pulse/validateGetPulsesRequest.ts`:
- Around line 17-31: Update the JSDoc for validateGetPulsesRequest to remove the
stale descriptions about org keys and Recoup admin keys and replace them with
the current behavior: all API keys return the authenticated account's pulses
(i.e., the function always returns the authenticated account's accountIds). Keep
the rest of the doc (query params, active handling, and return type) intact and
ensure the summary and param/returns accurately reflect
validateGetPulsesRequest's current authentication semantics.
---
Nitpick comments:
In `@lib/chats/buildGetChatsParams.ts`:
- Around line 4-11: The BuildGetChatsParamsInput interface uses snake_case field
names (account_id, target_account_id, artist_id) while the sibling
BuildGetOrganizationsParamsInput uses camelCase; update BuildGetChatsParamsInput
to use camelCase (accountId, targetAccountId, artistId) to standardize internal
naming, then adjust any callers or serialization logic that expect snake_case
(e.g., in functions that call buildGetChatsParams or serialize its output) to
map to the API parameter names if needed so external API compatibility is
preserved.
In `@lib/sandbox/buildGetSandboxesParams.ts`:
- Around line 4-11: The BuildGetSandboxesParamsInput interface uses snake_case
property names (account_id, target_account_id, sandbox_id) which is inconsistent
with other builders; update the interface to use camelCase names (accountId,
targetAccountId, sandboxId) and adjust any callers or destructuring that
reference BuildGetSandboxesParamsInput to the new property names (search for
usages of BuildGetSandboxesParamsInput and fields in buildGetSandboxesParams,
its consumers, and tests) so the parameter builder matches the naming convention
used by buildGetPulsesParams and buildGetArtistsParams.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 658a7ec9-d6e8-4b23-bb2e-494b1e62b6e3
⛔ Files ignored due to path filters (14)
lib/artists/__tests__/buildGetArtistsParams.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/auth/__tests__/validateAuthContext.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/chat/__tests__/validateChatRequest.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/chats/__tests__/buildGetChatsParams.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/chats/__tests__/getChatsHandler.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/chats/__tests__/validateGetChatsRequest.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/mcp/__tests__/verifyApiKey.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/mcp/tools/chats/__tests__/registerGetChatsTool.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/mcp/tools/pulse/__tests__/registerGetPulsesTool.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/organizations/__tests__/buildGetOrganizationsParams.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/pulse/__tests__/buildGetPulsesParams.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/pulse/__tests__/validateGetPulsesRequest.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/sandbox/__tests__/buildGetSandboxesParams.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/sandbox/__tests__/validateGetSandboxesRequest.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (17)
lib/artists/buildGetArtistsParams.tslib/artists/validateGetArtistsRequest.tslib/chat/validateChatRequest.tslib/chats/buildGetChatsParams.tslib/chats/validateGetChatsRequest.tslib/chats/validateUpdateChatBody.tslib/mcp/tools/chats/registerCompactChatsTool.tslib/mcp/tools/chats/registerGetChatsTool.tslib/mcp/tools/pulse/registerGetPulsesTool.tslib/mcp/verifyApiKey.tslib/organizations/buildGetOrganizationsParams.tslib/organizations/validateGetOrganizationsRequest.tslib/pulse/buildGetPulsesParams.tslib/pulse/validateGetPulsesRequest.tslib/sandbox/buildGetSandboxesParams.tslib/sandbox/validateGetSandboxesFileRequest.tslib/sandbox/validateGetSandboxesRequest.ts
💤 Files with no reviewable changes (5)
- lib/mcp/tools/pulse/registerGetPulsesTool.ts
- lib/mcp/tools/chats/registerCompactChatsTool.ts
- lib/mcp/tools/chats/registerGetChatsTool.ts
- lib/sandbox/validateGetSandboxesFileRequest.ts
- lib/chats/validateUpdateChatBody.ts
Another missed reference to keyDetails.orgId which no longer exists. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
sweetmantech
commented
Mar 19, 2026
| messages: allUiMessages, | ||
| accountId, | ||
| orgId, | ||
| orgId: null, |
Contributor
Author
There was a problem hiding this comment.
why does ChatRequestBody have an orgId param?
Contributor
Author
There was a problem hiding this comment.
ChatRequestBody still has orgId because it's set from the explicit organizationId body param (line 162: orgId = validatedBody.organizationId). This isn't derived from the API key — it's passed by the caller when they want org context for a specific request. It flows through to setupChatRequest for tool/prompt configuration. This is the same pattern as AuthContext.orgId — only set from explicit input, never from key details.
4 tasks
This was referenced Mar 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
getApiKeyDetails— all keys are personal, no org context on the key itselforgIdtracking fromvalidateAuthContext— no longer reads org context from key detailsvalidateAccountIdOverridenow passescurrentAccountIdtocanAccessAccountfor shared-org membership checksorgIdinAuthContextis only set from explicitorganizationIdinput parameterTest plan
account_idoverride works for accounts in shared orgs🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
Refactor