feat(chats): admins (RECOUP_ORG) can access any chat — read + write

[sweetmantech]

Gives Recoup admins (RECOUP_ORG_ID members) access to any account's chats — read and write — so support/admins can inspect and manage a customer's conversations by chat id.

Approach

validateChatAccess (the shared guard for all chat-by-id operations) now grants access to the room's owner, and falls back to an admin check when the caller isn't the owner. No account_id input anywhere — the chat is identified by the path id and the owner is resolved server-side.

• The admin check (checkIsAdmin → RECOUP_ORG_ID membership) runs only after the ownership check fails, so the common owner path never pays the extra lookup.
• Because validateChatAccess is shared, this applies to all chat-by-id endpoints: reads (GET messages, getChatArtist) and mutations (updateChat, deleteTrailingMessages, copyChatMessages). Admins can read and write any account's chats.
• Non-admins are unchanged: owner-only, 403 otherwise.

Evolution of this PR

1. Started as an account_id query override.
2. Pivoted to an admin bypass with an opt-in allowAdmin flag (read-only), to match docs#247 rolling the param back (c3bf9bb).
3. Dropped the flag (this is the final shape) — internal YAGNI/KISS call: admins already have broad cross-account power and chat ops are resource-scoped by id, so an unconditional admin bypass is the coherent model. Read + write admin access is intentional.

Tests

• admin gets access to a room they don't own (and the admin check is reached only after ownership fails)
• owner path never consults admin status
• non-admin non-owner → 403
• invalid id → 400; missing room → 404; auth failure passthrough

No docs change — the contract already reflects no param. Decision recorded in recoupable/chat#1811.

[vercel]

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

Project	Deployment	Actions	Updated (UTC)
api	Ready	Preview	Jun 23, 2026 1:42pm

[coderabbitai]

Warning

Review limit reached

@sweetmantech, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 18 minutes and 41 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses rolling per-developer review limits. Reviews become available again as older review attempts age out of the rolling limit window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c66d75de-a3fd-4f29-b25a-c28baadac179

📥 Commits

Reviewing files that changed from the base of the PR and between c9174a0 and 429fdb7.

⛔ Files ignored due to path filters (2)

• lib/chats/__tests__/getChatArtistHandler.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
• lib/chats/__tests__/validateChatAccess.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**

📒 Files selected for processing (1)

• lib/chats/validateChatAccess.ts

📝 Walkthrough

Walkthrough

validateChatAccess gains an optional ValidateChatAccessOptions interface with an accountId override, passed into validateAuthContext. validateGetChatMessagesQuery extracts account_id/accountId from query parameters, validates it as a UUID (returning 400 on failure), and forwards it to validateChatAccess.

Changes

Account ID Override for Chat Access

Layer / File(s)	Summary
ValidateChatAccessOptions contract and auth wiring lib/chats/validateChatAccess.ts	Exports ValidateChatAccessOptions interface with optional accountId, extends validateChatAccess to accept options = {}, and threads options.accountId into validateAuthContext(request, { accountId: options.accountId }).
Query parameter extraction and validation lib/chats/validateGetChatMessagesQuery.ts	Adds accountIdSchema (UUID), reads account_id/accountId from search params, returns a 400 CORS-headered error on invalid UUID, and conditionally passes { accountId: accountIdParam } to validateChatAccess.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

• Admin account override: let admins call any account-scoped endpoint in another account's context chat#1811: The PR directly implements the admin account-override for GET /api/chats/{id}/messages by adding accountId parameter support to validateChatAccess and validateGetChatMessagesQuery.

Possibly related PRs

• recoupable/api#405: Both PRs propagate an accountId override into the authorization flow by calling validateAuthContext(request, { accountId: ... }), with this PR doing so in validateChatAccess and the referenced PR doing so in validateChatRequest.

Poem

🔑 A key for another's door,
With an accountId passed just so,
UUID-checked at the query shore,
Four-hundred if the format's poor,
Clean options, clean auth — let the admins flow!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Solid & Clean Code	⚠️ Warning	The PR lacks a conflict guard for the account_id/accountId query parameter handling. When both query parameters are present with different values, the code silently prefers account_id instead of re...	Implement the proposed conflict guard: extract both snake_case and camelCase parameters separately, check if both are provided and differ, and return 400 with a conflict error message before proceeding.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/chat-messages-account-override

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[cubic-dev-ai]

No issues found across 4 files

Confidence score: 5/5

• Automated review surfaced no issues in the provided summaries.
• No files require special attention.

Architecture diagram

sequenceDiagram
    participant Client as Admin Client
    participant Route as GET /api/chats/{id}/messages
    participant Query as validateGetChatMessagesQuery
    participant Access as validateChatAccess
    participant Auth as validateAuthContext
    participant Override as validateAccountIdOverride
    participant DB as Database

    Note over Client,DB: Admin account-override flow for fetching chat messages

    Client->>Route: GET /api/chats/{id}/messages?account_id=<target_uuid>
    Route->>Query: validateGetChatMessagesQuery(request, roomId)
    
    Query->>Query: Parse roomId UUID
    Query->>Query: Parse optional account_id/accountId from query params
    
    alt No account_id provided
        Query->>Access: validateChatAccess(request, roomId)
    else account_id provided
        Query->>Query: Validate UUID format
        alt Invalid UUID
            Query-->>Route: 400 Bad Request
            Route-->>Client: 400 JSON error response
        end
        Query->>Access: validateChatAccess(request, roomId, { accountId: <target> })
    end

    Access->>Auth: validateAuthContext(request, { accountId: <optional_target> })
    Auth->>Override: validateAccountIdOverride(auth, accountId)
    
    alt Override granted (Recoup org member/admin)
        Override-->>Auth: accountId (overridden)
        Auth-->>Access: authResult with overridden accountId
    else Override denied (non-admin / no access)
        Override-->>Auth: NextResponse 403
        Auth-->>Access: NextResponse 403
        Access-->>Route: 403 Forbidden
        Route-->>Client: 403 JSON error response
    end

    Access->>Access: Resolve room ownership against overridden accountId
    Access->>DB: selectRoom(roomId)
    DB-->>Access: Room record
    Access->>Access: buildGetChatsParams({ account_id: <target> })
    Access-->>Route: { roomId, room, accountId: target }
    Route-->>Client: 200 + chat messages for target account

Loading

<sub>Requires human review: Adds admin account override to a chat messages endpoint, modifying authorization logic and query validation. Even though the change is localized with tests, it alters a critical auth chokepoint and introduces new access control paths that require human review.

Re-trigger cubic</sub>

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

lib/chats/validateGetChatMessagesQuery.ts (1)

8-52: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Model the override as an exported query schema/type.

accountIdSchema is local, and the validation contract has no exported inferred type. Consider promoting the query shape to an exported Zod schema/type, which also helps keep this validation function smaller. As per coding guidelines, lib/**/validate*.ts: "Create validate functions in validateBody.ts or validateQuery.ts files that export both the schema and inferred TypeScript type." As per path instructions, lib/**/validate*.ts: "Use Zod for schema validation" and "Export inferred types for validated data."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/chats/validateGetChatMessagesQuery.ts` around lines 8 - 52, The
accountIdSchema is local and the query validation contract lacks an exported
inferred type. Create an exported Zod schema that models the query parameters
shape (including the optional accountId override) in the
validateGetChatMessagesQuery file, and export the inferred TypeScript type using
z.infer. This exposes the validation contract explicitly and follows the coding
guidelines requiring validate*.ts files to export both the schema and inferred
type for validated data, which will also help keep the
validateGetChatMessagesQuery function more concise.

Sources: Coding guidelines, Path instructions

lib/chats/validateChatAccess.ts (1)

35-82: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift

Split validateChatAccess before adding more access branches.

This function is now 48 lines and mixes chat-id validation, auth override resolution, room lookup, access-param building, and response shaping. Extract a focused helper into its own matching file if you keep growing this path. As per coding guidelines, **/*.{js,ts,tsx,jsx,py,java,cs,go,rb,php}: "Flag functions longer than 20 lines" and "Keep functions small and focused." As per path instructions, lib/**/*.ts: "Single responsibility per function" and "File naming rule: The file name MUST match the exported function name."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/chats/validateChatAccess.ts` around lines 35 - 82, The validateChatAccess
function exceeds the 20-line guideline and violates single responsibility by
combining chat-id validation, auth override resolution, room lookup,
access-param building, and response shaping. Extract focused helper functions
for distinct concerns such as room retrieval and validation, and access
parameter building and validation into separate files in the lib directory,
following the file-naming rule where the file name matches the exported function
name (e.g., if you create a helper function for room operations, name the file
to match that function). Keep validateChatAccess as a thin orchestrator that
delegates to these helpers.

Sources: Coding guidelines, Path instructions

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@lib/chats/validateGetChatMessagesQuery.ts`:
- Around line 35-52: The validateGetChatMessagesQuery function currently
silently prefers the account_id parameter when both account_id and accountId
query parameters are present with different values. Add a conflict detection
check after retrieving both searchParams.get("account_id") and
searchParams.get("accountId") - if both parameters are present and have
different values, return a NextResponse.json error response with status 400 and
an appropriate error message indicating conflicting parameters, before
proceeding to parse and validate the accountIdParam.

---

Nitpick comments:
In `@lib/chats/validateChatAccess.ts`:
- Around line 35-82: The validateChatAccess function exceeds the 20-line
guideline and violates single responsibility by combining chat-id validation,
auth override resolution, room lookup, access-param building, and response
shaping. Extract focused helper functions for distinct concerns such as room
retrieval and validation, and access parameter building and validation into
separate files in the lib directory, following the file-naming rule where the
file name matches the exported function name (e.g., if you create a helper
function for room operations, name the file to match that function). Keep
validateChatAccess as a thin orchestrator that delegates to these helpers.

In `@lib/chats/validateGetChatMessagesQuery.ts`:
- Around line 8-52: The accountIdSchema is local and the query validation
contract lacks an exported inferred type. Create an exported Zod schema that
models the query parameters shape (including the optional accountId override) in
the validateGetChatMessagesQuery file, and export the inferred TypeScript type
using z.infer. This exposes the validation contract explicitly and follows the
coding guidelines requiring validate*.ts files to export both the schema and
inferred type for validated data, which will also help keep the
validateGetChatMessagesQuery function more concise.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 682fa215-64f1-43fa-8321-0e3d87af1e88

📥 Commits

Reviewing files that changed from the base of the PR and between 8c962ef and c9174a0.

⛔ Files ignored due to path filters (2)

• lib/chats/__tests__/validateChatAccess.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
• lib/chats/__tests__/validateGetChatMessagesQuery.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**

📒 Files selected for processing (2)

• lib/chats/validateChatAccess.ts
• lib/chats/validateGetChatMessagesQuery.ts

[sweetmantech]

Reworked to admin bypass, no param (commit 710c9bd3), to match the shipped docs contract — docs#247 rolled back the account_id query param (c3bf9bb).

Replaced the account_id/accountId query override (and the camelCase alias) with an opt-in allowAdmin flag on validateChatAccess that grants RECOUP_ORG admins access to any room via the existing checkIsAdmin. Only the messages read path opts in; the shared mutation call sites (update / delete-trailing / copy) leave it off, so admin write access isn't broadened. Rationale in recoupable/chat#1811.

Note: tsc surfaces pre-existing type drift in several unrelated chat __tests__ mocks (missing authToken/status/messages fields), pulled in via the test/main database-types — not from this PR. The implementation files are type-clean and the build graph excludes __tests__.

[cubic-dev-ai]

1 issue found across 5 files (changes from recent commits).

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[sweetmantech]

Dropped the allowAdmin flag (commit 1c48115b) after internal review — going with an unconditional admin bypass (read + write to any chat).

Rationale: RECOUP_ORG admins already hold broad cross-account power (delete any artist, read any account), and chat ops are resource-scoped by chatId, so a blanket admin bypass is the coherent, simpler model rather than scoping reads vs. writes. This intentionally broadens admin access to the chat mutation endpoints (updateChat, deleteTrailingMessages, copyChatMessages) that share validateChatAccess, not just the messages read — recorded in recoupable/chat#1811. The admin check is ordered after the ownership check, so normal users never pay the extra lookup. Non-admins unchanged (403).

[cubic-dev-ai]

0 issues found across 4 files (changes from recent commits).

<sub>Requires human review: Modifies authorization logic in chat access guard; adds admin bypass for all chat operations. Requires human review due to security and business logic impact.

Re-trigger cubic</sub>

[sweetmantech]

Manual testing on preview — ✅ admin bypass works (read + write)

Tested against the preview deploy https://api-git-feat-chat-messages-account-override-recoup.vercel.app. Zero customer data touched — used a throwaway chat created on behalf of a disposable agent account, deleted after.

Setup: admin fb678396… (RECOUP_ORG member), throwaway room R owned by agent account d6929db1…, so the admin is a non-owner of R.

Test	Request	Result
Admin read — the new bypass	admin GET /api/chats/R/messages (preview)	200 ✅ — admin isn't the owner, so 200 is only reachable via the RECOUP_ORG bypass
Admin write — read+write decision	admin PATCH /api/chats {chatId:R,…} (preview)	200 ✅ — bypass authorizes the mutation too
Owner baseline	owner agent A GET …/messages	200 ✅
Non-admin non-owner	agent B GET …/messages	403 ✅
Cleanup	delete R → re-read	404 ✅ (no leftover)

Confirms: a RECOUP_ORG admin can read and write any chat by id (the bypass in validateChatAccess); non-admins stay owner-gated (403). Logically, the admin calls would have been 403 before this PR — the admin is a non-owner, and non-owners 403 without the bypass (see the agent B row).

Environment caveats (honest about what I could/couldn't A/B):

• The admin token is a Privy token scoped to the preview/dev Privy app, so prod rejects it (401) — I couldn't run the same admin calls on prod as a before/after. (Symmetrically, prod-minted API keys 401 on preview — preview verifies keys against a different secret.) Net: admin Privy creds work only on preview; agent API keys work only on prod.
• The non-admin 403 was therefore exercised with agent keys on prod (where they authenticate). It's unchanged ownership behavior and is also covered by the unit tests added here.

Tested 2026-06-23.