Skip to content

agent: @U0AJM7X8FBR API Only - Remove ORG API Key logic. All API keys are perso#313

Merged
recoup-coding-agent merged 9 commits intotestfrom
agent/-u0ajm7x8fbr-api-only---remove-1773780703986
Mar 19, 2026
Merged

agent: @U0AJM7X8FBR API Only - Remove ORG API Key logic. All API keys are perso#313
recoup-coding-agent merged 9 commits intotestfrom
agent/-u0ajm7x8fbr-api-only---remove-1773780703986

Conversation

@sweetmantech
Copy link
Copy Markdown
Contributor

@sweetmantech sweetmantech commented Mar 17, 2026

Automated PR from coding agent.

Prompt: @U0AJM7X8FBR API Only - Remove ORG API Key logic. All API keys are personal.
• actual: API has different logic to validate Org API keys as opposed to Personal API keys.
• required: All keys are personal keys. If my personal account has access to an org, it can use account_id filtering within that org. All permissions are checked at the personal level access to the org rather than the org level api keys.

Summary by CodeRabbit

  • Refactor
    • Authorization checks now evaluate permissions using the requesting account’s identity rather than an organization identifier, preserving self-access and improving shared-organization membership handling across filters and handlers.
  • Bug Fixes
    • Standardized access-denied behavior: a single consistent 403 error and message is returned for unauthorized account filtering and overrides.

Recoup Agent and others added 2 commits March 17, 2026 20:51
All API keys are now personal. When a personal API key provides an
account_id override, instead of immediately returning 403, we look up
the key owner's org memberships and check if the target account is a
member of any shared org.

- Add canAccessAccountViaAnyOrg() in lib/organizations/ with full test coverage
- Update validateAccountIdOverride() to use it as fallback when orgId is null
- Update validateAuthContext tests to cover new behavior

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown
Contributor

vercel bot commented Mar 17, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
recoup-api Ready Ready Preview Mar 19, 2026 9:28pm

Request Review

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 17, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 396b6511-dd1e-4d98-82df-f7c9ca33f40c

📥 Commits

Reviewing files that changed from the base of the PR and between 4662181 and 40d7ce0.

⛔ Files ignored due to path filters (1)
  • lib/organizations/__tests__/canAccessAccount.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
📒 Files selected for processing (1)
  • lib/organizations/canAccessAccount.ts

📝 Walkthrough

Walkthrough

Replaces org-scoped override checks with account-centric authorization: callers now pass currentAccountId to canAccessAccount, and canAccessAccount was reworked to determine access via self-check, RECOUP membership, or shared organization membership. Denial responses were unified to a single message.

Changes

Cohort / File(s) Summary
Core access logic
lib/organizations/canAccessAccount.ts
Signature changed to require currentAccountId (removed orgId); implements self-check, RECOUP bypass, and shared-membership detection by looking up organizations for currentAccountId.
Auth override validation
lib/auth/validateAccountIdOverride.ts, lib/accounts/validateOverrideAccountId.ts, lib/mcp/resolveAccountId.ts, lib/tasks/validateGetTaskRunQuery.ts, lib/auth/validateAuthContext.ts
Callsites updated to stop passing orgId and instead provide currentAccountId (or omit org arg) to override/access validation; return behavior unchanged.
Request param builders
lib/artists/buildGetArtistsParams.ts, lib/chats/buildGetChatsParams.ts, lib/organizations/buildGetOrganizationsParams.ts, lib/pulse/buildGetPulsesParams.ts, lib/sandbox/buildGetSandboxesParams.ts
Access checks now call canAccessAccount({ targetAccountId, currentAccountId: accountId }); unified denial error "Access denied to specified account_id".
Chat compacting handlers
lib/chats/compactChatsHandler.ts, lib/chats/processCompactChatRequest.ts, lib/mcp/tools/chats/registerCompactChatsTool.ts
Removed orgId from processCompactChatRequest parameters and callsites; authorization inside processCompactChatRequest now uses currentAccountId.
Miscellaneous callsites
lib/chats/..., other minor files
Minor signature adjustments and removal of forwarded orgId arguments; control flow and responses largely preserved.

Sequence Diagram

sequenceDiagram
    participant Caller as Caller (API handler / builder)
    participant CA as canAccessAccount
    participant DB as DB / Organizations
    participant SEL as selectAccountOrganizationIds

    Caller->>CA: canAccessAccount({ targetAccountId, currentAccountId })
    alt targetAccountId == currentAccountId
        CA-->>Caller: Access granted
    else
        CA->>DB: getAccountOrganizations(currentAccountId)
        DB-->>CA: currentAccountOrgs
        alt currentAccountOrgs contains RECOUP_ORG_ID
            CA-->>Caller: Access granted (RECOUP)
        else
            CA->>SEL: selectAccountOrganizationIds(targetAccountId, currentAccountOrgIds)
            SEL-->>DB: query shared memberships
            DB-->>SEL: sharedOrgIds
            alt sharedOrgIds not empty
                SEL-->>CA: shared present
                CA-->>Caller: Access granted
            else
                CA-->>Caller: Access denied
            end
        end
    end
Loading

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly Related PRs

Poem

🔐 A slimmed call passes one ID through,
Self slips by, RECOUP and peers review,
One answer speaks when access is denied,
Old orgs unpassed, the checks now use who tried,
Clean paths, small change — the gates stay true.

🚥 Pre-merge checks | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Solid & Clean Code ⚠️ Warning PR violates DRY principle with 36+ hardcoded error messages, duplicated validation patterns across five files, and unused variable in compactChatsHandler.ts. Extract error string to shared constant, create reusable validateTargetAccountAccess utility function, remove unused orgId variable, and ensure consistent error messaging.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch agent/-u0ajm7x8fbr-api-only---remove-1773780703986
📝 Coding Plan
  • Generate coding plan for human review comments

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

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Removed empty JSDoc comments that conflicted with formatter changes.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
These files only added empty or trivial JSDoc comments with no
functional changes. Reverted to match test branch.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Merged canAccessAccountViaAnyOrg into canAccessAccount via new optional
currentAccountId param. canAccessAccount now handles all access paths:
1. Org key with explicit orgId (existing)
2. RECOUP_ORG_ID universal admin bypass (existing)
3. Personal key shared-org fallback (new - from canAccessAccountViaAnyOrg)
4. Admin personal key bypass via RECOUP_ORG_ID membership (new)

validateAccountIdOverride simplified to single canAccessAccount call.
Deleted canAccessAccountViaAnyOrg.ts and its tests.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
lib/organizations/canAccessAccount.ts (1)

61-64: Consider making the return type contract explicit in selectAccountOrganizationIds.

The Array.isArray(shared) check is necessary because selectAccountOrganizationIds returns null on Supabase errors (line 21 of the implementation), despite the JSDoc documenting this. Adding an explicit return type annotation—Promise<string[] | null>—would make this contract clear to TypeScript and all callers, eliminating the need for defensive checks and improving consistency across the codebase.

Currently, error handling differs between callers: canAccessAccount uses the defensive check pattern while checkAccountArtistAccess uses an explicit if (!userOrgAccess) with a comment. An explicit return type would encourage consistent, idiomatic null-checking patterns and surface failures explicitly if desired for logging/debugging.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/organizations/canAccessAccount.ts` around lines 61 - 64, Add an explicit
return type Promise<string[] | null> to selectAccountOrganizationIds (and update
its JSDoc) so the null-on-error contract is surfaced to TypeScript; then update
callers: in canAccessAccount replace the Array.isArray(shared) defensive check
with an explicit null/empty check consistent with checkAccountArtistAccess
(e.g., if (!shared) handle error/deny access, otherwise check shared.length >
0), keeping the function name selectAccountOrganizationIds and callers
canAccessAccount and checkAccountArtistAccess to locate changes.
🤖 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/organizations/canAccessAccount.ts`:
- Around line 1-3: Run Prettier to fix formatting issues in canAccessAccount.ts:
apply `prettier --write` (or your project's formatter) to this file so imports
and whitespace match project style, then stage the updated file; ensure import
lines (RECOUP_ORG_ID, getAccountOrganizations, selectAccountOrganizationIds) and
any surrounding code in canAccessAccount.ts are reformatted consistently to pass
CI.

---

Nitpick comments:
In `@lib/organizations/canAccessAccount.ts`:
- Around line 61-64: Add an explicit return type Promise<string[] | null> to
selectAccountOrganizationIds (and update its JSDoc) so the null-on-error
contract is surfaced to TypeScript; then update callers: in canAccessAccount
replace the Array.isArray(shared) defensive check with an explicit null/empty
check consistent with checkAccountArtistAccess (e.g., if (!shared) handle
error/deny access, otherwise check shared.length > 0), keeping the function name
selectAccountOrganizationIds and callers canAccessAccount and
checkAccountArtistAccess to locate changes.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e5f5a3ac-42da-4580-8722-4964186a2c93

📥 Commits

Reviewing files that changed from the base of the PR and between 0355d39 and a001907.

⛔ Files ignored due to path filters (42)
  • lib/admins/privy/__tests__/getPrivyLoginsHandler.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/admins/privy/__tests__/validateGetPrivyLoginsQuery.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/ai/__tests__/getAvailableModels.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/ai/__tests__/getModel.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/artists/__tests__/buildGetArtistsParams.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/artists/__tests__/checkAccountArtistAccess.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/artists/__tests__/createArtistInDb.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/artists/__tests__/validateGetArtistsRequest.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/auth/__tests__/validateAuthContext.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/chat/__tests__/handleChatCompletion.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/chat/__tests__/integration/chatEndToEnd.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/chat/__tests__/saveChatCompletion.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/chat/__tests__/setupConversation.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/chat/__tests__/setupToolsForRequest.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/chat/__tests__/validateChatRequest.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/chat/toolChains/__tests__/getPrepareStepResult.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/chats/__tests__/updateChatHandler.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/chats/__tests__/validateUpdateChatBody.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/credits/__tests__/getCreditUsage.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/credits/__tests__/handleChatCredits.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/github/__tests__/deleteAccountGithubRepos.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/mcp/__tests__/getMcpTools.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/mcp/__tests__/verifyApiKey.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/mcp/tools/artists/__tests__/registerCreateNewArtistTool.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/messages/__tests__/convertToUiMessages.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/organizations/__tests__/canAccessAccount.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/organizations/__tests__/validateOrganizationAccess.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/pulse/__tests__/buildGetPulsesParams.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/pulse/__tests__/getPulsesHandler.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/pulse/__tests__/updatePulsesHandler.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/pulse/__tests__/validateGetPulsesRequest.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/rooms/__tests__/copyRoom.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/supabase/account_artist_ids/__tests__/insertAccountArtistId.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/supabase/account_artist_ids/__tests__/selectAccountArtistId.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/supabase/account_organization_ids/__tests__/selectAccountOrganizationIds.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/supabase/account_sandboxes/__tests__/insertAccountSandbox.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/supabase/account_workspace_ids/__tests__/selectAccountWorkspaceId.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/supabase/accounts/__tests__/selectAccountWithSocials.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/supabase/artist_organization_ids/__tests__/selectArtistOrganizationIds.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/supabase/pulse_accounts/__tests__/selectPulseAccounts.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/supabase/pulse_accounts/__tests__/upsertPulseAccount.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/tasks/__tests__/deleteTask.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
📒 Files selected for processing (2)
  • lib/auth/validateAccountIdOverride.ts
  • lib/organizations/canAccessAccount.ts

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 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/organizations/canAccessAccount.ts`:
- Around line 45-54: When orgId === null and you fall through to the "personal
key — check shared org membership" path, you must allow the requester to access
their own account before querying orgs; add an explicit check that if
currentAccountId === targetAccountId return true (i.e., allow self-access)
immediately prior to calling getAccountOrganizations, so functions/variables
currentAccountId, targetAccountId, orgId and the getAccountOrganizations
fallback are updated accordingly.
- Around line 31-43: The org-based branch currently returns true unconditionally
for RECOUP_ORG_ID and only checks targetAccountId membership for others; instead
require that the requester (currentAccountId) is also a member of the org before
granting access. Replace the unconditional check by calling
getAccountOrganizations with accountId: currentAccountId and organizationId:
orgId (or otherwise verify currentAccountId membership) and only allow access if
both the requester is a member of orgId and the targetAccountId membership check
(memberships from getAccountOrganizations for targetAccountId) passes; remove
the unconditional return true when orgId === RECOUP_ORG_ID and ensure you
reference orgId, RECOUP_ORG_ID, getAccountOrganizations, targetAccountId, and
currentAccountId in the updated logic.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: cb808af4-5cf6-44f8-adcb-22998e9c1741

📥 Commits

Reviewing files that changed from the base of the PR and between a001907 and 209a5b8.

📒 Files selected for processing (1)
  • lib/organizations/canAccessAccount.ts

All 5 buildGet*Params functions now pass currentAccountId to
canAccessAccount, enabling shared-org access for personal keys.
Unified error message to "Access denied to specified account_id".
Updated all handler/request/MCP test assertions to match.

1531 tests passing.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
All API keys are personal — orgId is dead code. canAccessAccount
now only accepts { currentAccountId, targetAccountId } and checks
shared org membership directly. Removed orgId from:
- canAccessAccount params
- validateAccountIdOverride params
- buildGet*Params (artists, chats, pulse, sandboxes, organizations)
- processCompactChatRequest
- resolveAccountId (MCP)
- validateOverrideAccountId

Updated all corresponding tests.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
lib/chats/compactChatsHandler.ts (1)

24-28: ⚠️ Potential issue | 🟡 Minor

Remove unused orgId variable from destructuring.

The orgId is destructured from validated on line 24 but is never used in the function. After removing it from the processCompactChatRequest call, it became dead code that should be cleaned up.

Proposed fix
-    const { chatIds, prompt, accountId, orgId } = validated;
+    const { chatIds, prompt, accountId } = validated;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/chats/compactChatsHandler.ts` around lines 24 - 28, Remove the unused
orgId from the object destructuring of validated in compactChatsHandler: update
the line that currently reads "const { chatIds, prompt, accountId, orgId } =
validated;" to only extract chatIds, prompt, and accountId, since orgId is not
used anywhere (not passed into processCompactChatRequest or referenced later);
ensure no other references to orgId remain in this function and run tests or
type checks to confirm no remaining dead variables.
lib/artists/buildGetArtistsParams.ts (1)

6-13: ⚠️ Potential issue | 🟡 Minor

Remove unused orgId parameter from the interface and function destructuring.

The orgId parameter is defined in BuildGetArtistsParamsInput but is never used in the function body. Line 34 destructures it, but the variable is never referenced. Only orgIdFilter is used to compute effectiveOrgId (line 55). Remove orgId from both the interface definition and the destructuring statement to keep the code clean and avoid confusion.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/artists/buildGetArtistsParams.ts` around lines 6 - 13, Remove the unused
orgId property from the BuildGetArtistsParamsInput interface and delete orgId
from the parameter destructuring in the buildGetArtistsParams function; update
any related JSDoc/comment if present and ensure only orgIdFilter (and
accountId/targetAccountId) are used to compute effectiveOrgId in
buildGetArtistsParams so there are no lingering references to orgId.
🧹 Nitpick comments (3)
lib/chats/processCompactChatRequest.ts (1)

35-43: Stale comment references "org key".

The comment on line 36 mentions "org key" which is outdated now that all API keys are personal.

📝 Proposed comment update
   const roomAccountId = room.account_id;
   if (roomAccountId && roomAccountId !== accountId) {
-    // Check if org key has access to this account
+    // Check if current account has access to this room's account
     const hasAccess = await canAccessAccount({
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/chats/processCompactChatRequest.ts` around lines 35 - 43, Update the
outdated inline comment referencing "org key" in the access check inside
processCompactChatRequest (the block that checks roomAccountId !== accountId and
calls canAccessAccount) to reflect that API keys are personal now; replace
"Check if org key has access to this account" with a concise comment like "Check
if current account (API key owner) has access to this account" or similar so the
intent remains clear and accurate.
lib/mcp/resolveAccountId.ts (1)

14-28: Stale documentation references to "org API keys".

The docstring on line 16 and inline comment on line 28 still reference "org API key" validation, but this PR removes the org API key concept entirely. These comments should be updated to reflect the new personal-key-only semantics.

📝 Proposed documentation update
 /**
  * Resolves the accountId from MCP auth info or an override parameter.
- * Validates access when an org API key attempts to use an account_id override.
+ * Validates access when an account_id override is provided.
  *
  * `@param` params - The auth info and optional account_id override.
  * `@returns` The resolved accountId or an error message.
  */
   if (authAccountId) {
-    // If account_id override is provided, validate access (for org API keys)
+    // If account_id override is provided, validate access
     if (accountIdOverride && accountIdOverride !== authAccountId) {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/mcp/resolveAccountId.ts` around lines 14 - 28, The docstring and inline
comment in resolveAccountId still mention "org API keys" which were removed;
update the documentation and the inline comment to reflect the new
personal-key-only semantics: change the docstring summary and description to say
this validates access when a personal API key attempts to use an account_id
override, and replace the inline comment above the authAccountId check to state
"If account_id override is provided, validate access (for personal API keys)" or
similar; ensure references to authInfo, accountIdOverride, and authAccountId
remain accurate and consistent with the new behavior.
lib/accounts/validateOverrideAccountId.ts (1)

15-26: Stale docstring references to "org API key" semantics.

The docstring still describes org API key behavior ("org API key wants to create resources on behalf of another account", "API key belongs to an org with access"). Since all API keys are now personal, this should be updated. Additionally, the redundant JSDoc @param entries (root0, root0.apiKey, root0.targetAccountId) appear auto-generated and can be removed.

📝 Proposed documentation update
 /**
- * Validates that an API key has permission to override to a target accountId.
+ * Validates that a personal API key has permission to access a target accountId.
  *
- * Used when an org API key wants to create resources on behalf of another account.
- * Checks that the API key belongs to an org with access to the target account.
+ * Checks that the authenticated account has access to the target account
+ * (e.g., via shared organization membership or admin privileges).
  *
  * `@param` params.apiKey - The x-api-key header value
  * `@param` params.targetAccountId - The accountId to override to
- * `@param` root0
- * `@param` root0.apiKey
- * `@param` root0.targetAccountId
  * `@returns` The validated accountId or a NextResponse error
  */
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/accounts/validateOverrideAccountId.ts` around lines 15 - 26, The JSDoc
for validateOverrideAccountId contains stale "org API key" language and
redundant auto-generated `@param` tags; update the description to reflect that API
keys are personal (remove references to "org API key" and org-level access) and
simplify the parameter docs to only include relevant params (e.g., params.apiKey
and params.targetAccountId) by removing the redundant root0, root0.apiKey,
root0.targetAccountId entries and ensuring the summary and param descriptions
match the current personal-key semantics.
🤖 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 6-13: Remove the unused orgId property from the
BuildGetArtistsParamsInput interface and delete orgId from the parameter
destructuring in the buildGetArtistsParams function; update any related
JSDoc/comment if present and ensure only orgIdFilter (and
accountId/targetAccountId) are used to compute effectiveOrgId in
buildGetArtistsParams so there are no lingering references to orgId.

In `@lib/chats/compactChatsHandler.ts`:
- Around line 24-28: Remove the unused orgId from the object destructuring of
validated in compactChatsHandler: update the line that currently reads "const {
chatIds, prompt, accountId, orgId } = validated;" to only extract chatIds,
prompt, and accountId, since orgId is not used anywhere (not passed into
processCompactChatRequest or referenced later); ensure no other references to
orgId remain in this function and run tests or type checks to confirm no
remaining dead variables.

---

Nitpick comments:
In `@lib/accounts/validateOverrideAccountId.ts`:
- Around line 15-26: The JSDoc for validateOverrideAccountId contains stale "org
API key" language and redundant auto-generated `@param` tags; update the
description to reflect that API keys are personal (remove references to "org API
key" and org-level access) and simplify the parameter docs to only include
relevant params (e.g., params.apiKey and params.targetAccountId) by removing the
redundant root0, root0.apiKey, root0.targetAccountId entries and ensuring the
summary and param descriptions match the current personal-key semantics.

In `@lib/chats/processCompactChatRequest.ts`:
- Around line 35-43: Update the outdated inline comment referencing "org key" in
the access check inside processCompactChatRequest (the block that checks
roomAccountId !== accountId and calls canAccessAccount) to reflect that API keys
are personal now; replace "Check if org key has access to this account" with a
concise comment like "Check if current account (API key owner) has access to
this account" or similar so the intent remains clear and accurate.

In `@lib/mcp/resolveAccountId.ts`:
- Around line 14-28: The docstring and inline comment in resolveAccountId still
mention "org API keys" which were removed; update the documentation and the
inline comment to reflect the new personal-key-only semantics: change the
docstring summary and description to say this validates access when a personal
API key attempts to use an account_id override, and replace the inline comment
above the authAccountId check to state "If account_id override is provided,
validate access (for personal API keys)" or similar; ensure references to
authInfo, accountIdOverride, and authAccountId remain accurate and consistent
with the new behavior.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 7c38b0c5-01e8-4a20-9cf7-d7cc5f27740a

📥 Commits

Reviewing files that changed from the base of the PR and between af65c45 and 4662181.

⛔ Files ignored due to path filters (17)
  • lib/accounts/__tests__/validateOverrideAccountId.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/artists/__tests__/buildGetArtistsParams.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/artists/__tests__/validateGetArtistsRequest.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/auth/__tests__/validateAuthContext.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/chats/__tests__/buildGetChatsParams.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/chats/__tests__/compactChatsHandler.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/chats/__tests__/processCompactChatRequest.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/chats/__tests__/validateGetChatsRequest.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/mcp/tools/artists/__tests__/registerCreateNewArtistTool.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/mcp/tools/chats/__tests__/registerCompactChatsTool.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/mcp/tools/chats/__tests__/registerGetChatsTool.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/organizations/__tests__/buildGetOrganizationsParams.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/organizations/__tests__/canAccessAccount.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/pulse/__tests__/buildGetPulsesParams.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/pulse/__tests__/validateGetPulsesRequest.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/sandbox/__tests__/buildGetSandboxesParams.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/tasks/__tests__/validateGetTaskRunQuery.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
📒 Files selected for processing (14)
  • lib/accounts/validateOverrideAccountId.ts
  • lib/artists/buildGetArtistsParams.ts
  • lib/auth/validateAccountIdOverride.ts
  • lib/auth/validateAuthContext.ts
  • lib/chats/buildGetChatsParams.ts
  • lib/chats/compactChatsHandler.ts
  • lib/chats/processCompactChatRequest.ts
  • lib/mcp/resolveAccountId.ts
  • lib/mcp/tools/chats/registerCompactChatsTool.ts
  • lib/organizations/buildGetOrganizationsParams.ts
  • lib/organizations/canAccessAccount.ts
  • lib/pulse/buildGetPulsesParams.ts
  • lib/sandbox/buildGetSandboxesParams.ts
  • lib/tasks/validateGetTaskRunQuery.ts
💤 Files with no reviewable changes (3)
  • lib/auth/validateAuthContext.ts
  • lib/tasks/validateGetTaskRunQuery.ts
  • lib/mcp/tools/chats/registerCompactChatsTool.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • lib/organizations/buildGetOrganizationsParams.ts
  • lib/organizations/canAccessAccount.ts
  • lib/pulse/buildGetPulsesParams.ts

Accounts with no org memberships were denied self-access since the
function returned false before reaching shared-org checks. Now
explicitly allows access when currentAccountId === targetAccountId.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@recoup-coding-agent recoup-coding-agent merged commit 36b9464 into test Mar 19, 2026
4 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Mar 19, 2026
sweetmantech added a commit that referenced this pull request Mar 19, 2026
Took test branch versions for files already updated in #313.
Updated validateAuthContext test to match #313's canAccessAccount
interface (no orgId param).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants