diff --git a/app/api/chats/route.ts b/app/api/chats/route.ts index 3a3070950..453fd1b51 100644 --- a/app/api/chats/route.ts +++ b/app/api/chats/route.ts @@ -21,13 +21,16 @@ export async function OPTIONS() { /** * GET /api/chats * - * Retrieves chat rooms for an account. + * Retrieves chats joined with their owning session. Each row carries + * `sessionId` and the owning `accountId` so clients can render canonical + * `/sessions/{sessionId}/chats/{chatId}` URLs. * * Authentication: x-api-key header or Authorization Bearer token required. * * Query parameters: - * - account_id (required): UUID of the account whose chats to retrieve - * - artist_account_id (optional): Filter to chats for a specific artist (UUID) + * - account_id (optional): UUID of the target account (validated for access). + * - artist_account_id (optional): Accepted for backward compatibility but + * ignored — reserved for the artist-surface migration. * * @param request - The request object containing query parameters * @returns A NextResponse with chats data or an error diff --git a/lib/chats/__tests__/getChatsHandler.test.ts b/lib/chats/__tests__/getChatsHandler.test.ts index 45c80c769..7ed6478d4 100644 --- a/lib/chats/__tests__/getChatsHandler.test.ts +++ b/lib/chats/__tests__/getChatsHandler.test.ts @@ -4,7 +4,8 @@ import { getChatsHandler } from "../getChatsHandler"; import { validateAuthContext } from "@/lib/auth/validateAuthContext"; import { canAccessAccount } from "@/lib/organizations/canAccessAccount"; -import { selectRooms } from "@/lib/supabase/rooms/selectRooms"; +import { isRecoupAdmin } from "@/lib/organizations/isRecoupAdmin"; +import { selectChatsWithSessions } from "@/lib/supabase/chats/selectChatsWithSessions"; vi.mock("@/lib/auth/validateAuthContext", () => ({ validateAuthContext: vi.fn(), @@ -14,28 +15,20 @@ vi.mock("@/lib/organizations/canAccessAccount", () => ({ canAccessAccount: vi.fn(), })); -vi.mock("@/lib/supabase/account_organization_ids/getAccountOrganizations", () => ({ - getAccountOrganizations: vi.fn(), +vi.mock("@/lib/organizations/isRecoupAdmin", () => ({ + isRecoupAdmin: vi.fn(), })); -vi.mock("@/lib/supabase/rooms/selectRooms", () => ({ - selectRooms: vi.fn(), +vi.mock("@/lib/supabase/chats/selectChatsWithSessions", () => ({ + selectChatsWithSessions: vi.fn(), })); vi.mock("@/lib/networking/getCorsHeaders", () => ({ getCorsHeaders: vi.fn(() => ({ "Access-Control-Allow-Origin": "*" })), })); -vi.mock("@/lib/const", () => ({ - RECOUP_ORG_ID: "recoup-org-id", -})); - /** * Creates a mock NextRequest with the given URL and headers. - * - * @param url - The URL for the request - * @param apiKey - The API key header value - * @returns A mock NextRequest */ function createMockRequest(url: string, apiKey = "test-api-key"): NextRequest { return { @@ -46,9 +39,16 @@ function createMockRequest(url: string, apiKey = "test-api-key"): NextRequest { } as unknown as NextRequest; } +/** Fixture helper — a session embed with optional artist id. */ +function sessionEmbed(accountId: string, artistId: string | null = null) { + return { id: "sess-1", account_id: accountId, artist_id: artistId }; +} + describe("getChatsHandler", () => { beforeEach(() => { vi.clearAllMocks(); + // Default: caller is NOT a Recoup admin — admin tests override. + vi.mocked(isRecoupAdmin).mockResolvedValue(false); }); describe("authentication", () => { @@ -63,41 +63,81 @@ describe("getChatsHandler", () => { expect(response.status).toBe(401); expect(json.status).toBe("error"); - expect(selectRooms).not.toHaveBeenCalled(); + expect(selectChatsWithSessions).not.toHaveBeenCalled(); }); }); describe("personal key behavior", () => { - it("returns chats for personal key owner without account_id param", async () => { + it("returns chats projected to the new shape for personal key", async () => { const accountId = "123e4567-e89b-12d3-a456-426614174000"; - const mockChats = [ + const artistId = "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa"; + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId, + orgId: null, + authToken: "test-token", + }); + vi.mocked(selectChatsWithSessions).mockResolvedValue([ + { + id: "chat-1", + title: "Hello", + session_id: "sess-1", + updated_at: "2024-01-02T00:00:00Z", + created_at: "2024-01-01T00:00:00Z", + active_stream_id: null, + last_assistant_message_at: null, + model_id: null, + session: sessionEmbed(accountId, artistId), + }, + ]); + + const request = createMockRequest("http://localhost/api/chats"); + const response = await getChatsHandler(request); + const json = await response.json(); + + expect(response.status).toBe(200); + expect(json.status).toBe("success"); + expect(json.chats).toEqual([ { id: "chat-1", - account_id: accountId, - artist_id: null, - topic: "Chat 1", - updated_at: "2024-01-01T00:00:00Z", + title: "Hello", + accountId, + sessionId: "sess-1", + updatedAt: "2024-01-02T00:00:00Z", + artistId, }, - ]; + ]); + expect(selectChatsWithSessions).toHaveBeenCalledWith({ + accountIds: [accountId], + artistAccountId: undefined, + }); + }); + it("surfaces artistId as null when session has no artist", async () => { + const accountId = "123e4567-e89b-12d3-a456-426614174000"; vi.mocked(validateAuthContext).mockResolvedValue({ accountId, - orgId: null, // Personal key + orgId: null, authToken: "test-token", }); - vi.mocked(selectRooms).mockResolvedValue(mockChats); + vi.mocked(selectChatsWithSessions).mockResolvedValue([ + { + id: "chat-1", + title: "Hello", + session_id: "sess-1", + updated_at: "2024-01-02T00:00:00Z", + created_at: "2024-01-01T00:00:00Z", + active_stream_id: null, + last_assistant_message_at: null, + model_id: null, + session: sessionEmbed(accountId, null), + }, + ]); const request = createMockRequest("http://localhost/api/chats"); const response = await getChatsHandler(request); const json = await response.json(); - expect(response.status).toBe(200); - expect(json.status).toBe("success"); - expect(json.chats).toEqual(mockChats); - expect(selectRooms).toHaveBeenCalledWith({ - account_ids: [accountId], - artist_id: undefined, - }); + expect(json.chats[0].artistId).toBeNull(); }); it("returns 403 when personal key tries to filter by account_id", async () => { @@ -106,7 +146,7 @@ describe("getChatsHandler", () => { vi.mocked(validateAuthContext).mockResolvedValue({ accountId, - orgId: null, // Personal key + orgId: null, authToken: "test-token", }); vi.mocked(canAccessAccount).mockResolvedValue(false); @@ -117,128 +157,74 @@ describe("getChatsHandler", () => { expect(response.status).toBe(403); expect(json.status).toBe("error"); - expect(json.error).toBe("Access denied to specified account_id"); - expect(selectRooms).not.toHaveBeenCalled(); + expect(selectChatsWithSessions).not.toHaveBeenCalled(); }); }); - describe("org key behavior", () => { - it("returns chats for org key owner without account_id param", async () => { - const orgId = "123e4567-e89b-12d3-a456-426614174000"; - const mockChats = [ - { - id: "chat-1", - account_id: orgId, - artist_id: null, - topic: "Chat 1", - updated_at: "2024-01-01T00:00:00Z", - }, - ]; + describe("artist filter", () => { + it("passes artist_account_id through to the select", async () => { + const accountId = "123e4567-e89b-12d3-a456-426614174000"; + const artistAccountId = "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa"; vi.mocked(validateAuthContext).mockResolvedValue({ - accountId: orgId, - orgId, + accountId, + orgId: null, authToken: "test-token", }); - vi.mocked(selectRooms).mockResolvedValue(mockChats); + vi.mocked(selectChatsWithSessions).mockResolvedValue([]); - const request = createMockRequest("http://localhost/api/chats"); + const request = createMockRequest( + `http://localhost/api/chats?artist_account_id=${artistAccountId}`, + ); const response = await getChatsHandler(request); - const json = await response.json(); expect(response.status).toBe(200); - expect(json.status).toBe("success"); - expect(json.chats).toEqual(mockChats); - expect(selectRooms).toHaveBeenCalledWith({ - account_ids: [orgId], - artist_id: undefined, + expect(selectChatsWithSessions).toHaveBeenCalledWith({ + accountIds: [accountId], + artistAccountId, }); }); + }); - it("allows org key to filter by account_id for org member", async () => { - const orgId = "123e4567-e89b-12d3-a456-426614174000"; - const memberId = "223e4567-e89b-12d3-a456-426614174000"; - const mockChats = [ - { - id: "chat-1", - account_id: memberId, - artist_id: null, - topic: "Chat 1", - updated_at: "2024-01-01T00:00:00Z", - }, - ]; - + describe("admin behavior", () => { + it("passes undefined accountIds when caller is in Recoup org (membership-based)", async () => { + const adminAccountId = "admin-account-123"; vi.mocked(validateAuthContext).mockResolvedValue({ - accountId: orgId, - orgId, + accountId: adminAccountId, + orgId: null, authToken: "test-token", }); - vi.mocked(canAccessAccount).mockResolvedValue(true); - vi.mocked(selectRooms).mockResolvedValue(mockChats); + vi.mocked(isRecoupAdmin).mockResolvedValue(true); + vi.mocked(selectChatsWithSessions).mockResolvedValue([]); - const request = createMockRequest(`http://localhost/api/chats?account_id=${memberId}`); + const request = createMockRequest("http://localhost/api/chats"); const response = await getChatsHandler(request); - const json = await response.json(); expect(response.status).toBe(200); - expect(json.status).toBe("success"); - expect(selectRooms).toHaveBeenCalledWith({ - account_ids: [memberId], - artist_id: undefined, - }); - }); - - it("returns 403 when org key tries to access non-member account", async () => { - const orgId = "123e4567-e89b-12d3-a456-426614174000"; - const nonMemberId = "323e4567-e89b-12d3-a456-426614174000"; - - vi.mocked(validateAuthContext).mockResolvedValue({ - accountId: orgId, - orgId, - authToken: "test-token", + expect(selectChatsWithSessions).toHaveBeenCalledWith({ + accountIds: undefined, + artistAccountId: undefined, }); - vi.mocked(canAccessAccount).mockResolvedValue(false); - - const request = createMockRequest(`http://localhost/api/chats?account_id=${nonMemberId}`); - const response = await getChatsHandler(request); - const json = await response.json(); - - expect(response.status).toBe(403); - expect(json.status).toBe("error"); - expect(json.error).toBe("Access denied to specified account_id"); - expect(selectRooms).not.toHaveBeenCalled(); }); - }); - - describe("Recoup admin key behavior", () => { - it("returns chats for admin account without account_id param", async () => { - const recoupOrgId = "recoup-org-id"; - const mockChats = [ - { - id: "chat-1", - account_id: recoupOrgId, - artist_id: null, - topic: "Chat 1", - updated_at: "2024-01-01T00:00:00Z", - }, - ]; + it("scopes admin to the target account when account_id is provided", async () => { + const adminAccountId = "admin-account-123"; + const target = "323e4567-e89b-12d3-a456-426614174000"; vi.mocked(validateAuthContext).mockResolvedValue({ - accountId: recoupOrgId, - orgId: recoupOrgId, + accountId: adminAccountId, + orgId: null, authToken: "test-token", }); - vi.mocked(selectRooms).mockResolvedValue(mockChats); + vi.mocked(canAccessAccount).mockResolvedValue(true); + vi.mocked(selectChatsWithSessions).mockResolvedValue([]); - const request = createMockRequest("http://localhost/api/chats"); + const request = createMockRequest(`http://localhost/api/chats?account_id=${target}`); const response = await getChatsHandler(request); - const json = await response.json(); expect(response.status).toBe(200); - expect(json.status).toBe("success"); - expect(selectRooms).toHaveBeenCalledWith({ - account_ids: [recoupOrgId], - artist_id: undefined, + expect(selectChatsWithSessions).toHaveBeenCalledWith({ + accountIds: [target], + artistAccountId: undefined, }); }); }); @@ -257,7 +243,7 @@ describe("getChatsHandler", () => { expect(response.status).toBe(400); expect(json.status).toBe("error"); - expect(selectRooms).not.toHaveBeenCalled(); + expect(selectChatsWithSessions).not.toHaveBeenCalled(); }); it("returns 400 when artist_account_id is invalid UUID", async () => { @@ -273,45 +259,12 @@ describe("getChatsHandler", () => { expect(response.status).toBe(400); expect(json.status).toBe("error"); - expect(selectRooms).not.toHaveBeenCalled(); + expect(selectChatsWithSessions).not.toHaveBeenCalled(); }); }); - describe("successful responses", () => { - it("returns filtered chats when artist_account_id is provided", async () => { - const accountId = "123e4567-e89b-12d3-a456-426614174000"; - const artistId = "223e4567-e89b-12d3-a456-426614174001"; - const mockChats = [ - { - id: "chat-1", - account_id: accountId, - artist_id: artistId, - topic: "Artist Chat", - updated_at: "2024-01-01T00:00:00Z", - }, - ]; - - vi.mocked(validateAuthContext).mockResolvedValue({ - accountId, - orgId: null, // Personal key - authToken: "test-token", - }); - vi.mocked(selectRooms).mockResolvedValue(mockChats); - - const request = createMockRequest(`http://localhost/api/chats?artist_account_id=${artistId}`); - const response = await getChatsHandler(request); - const json = await response.json(); - - expect(response.status).toBe(200); - expect(json.status).toBe("success"); - expect(json.chats).toEqual(mockChats); - expect(selectRooms).toHaveBeenCalledWith({ - account_ids: [accountId], - artist_id: artistId, - }); - }); - - it("returns empty array when no chats found", async () => { + describe("error handling", () => { + it("returns 500 when selectChatsWithSessions returns null", async () => { const accountId = "123e4567-e89b-12d3-a456-426614174000"; vi.mocked(validateAuthContext).mockResolvedValue({ @@ -319,20 +272,18 @@ describe("getChatsHandler", () => { orgId: null, authToken: "test-token", }); - vi.mocked(selectRooms).mockResolvedValue([]); + vi.mocked(selectChatsWithSessions).mockResolvedValue(null); const request = createMockRequest("http://localhost/api/chats"); const response = await getChatsHandler(request); const json = await response.json(); - expect(response.status).toBe(200); - expect(json.status).toBe("success"); - expect(json.chats).toEqual([]); + expect(response.status).toBe(500); + expect(json.status).toBe("error"); + expect(json.error).toBe("Failed to retrieve chats"); }); - }); - describe("error handling", () => { - it("returns 500 when selectRooms returns null", async () => { + it("returns 500 with a generic message when an exception is thrown (no leak of raw message)", async () => { const accountId = "123e4567-e89b-12d3-a456-426614174000"; vi.mocked(validateAuthContext).mockResolvedValue({ @@ -340,7 +291,7 @@ describe("getChatsHandler", () => { orgId: null, authToken: "test-token", }); - vi.mocked(selectRooms).mockResolvedValue(null); + vi.mocked(selectChatsWithSessions).mockRejectedValue(new Error("Database error")); const request = createMockRequest("http://localhost/api/chats"); const response = await getChatsHandler(request); @@ -348,26 +299,60 @@ describe("getChatsHandler", () => { expect(response.status).toBe(500); expect(json.status).toBe("error"); - expect(json.error).toBe("Failed to retrieve chats"); + expect(json.error).toBe("Internal server error"); + // Raw exception message must not leak into the response body. + expect(json.error).not.toContain("Database"); }); + }); - it("returns 500 when an exception is thrown", async () => { + describe("response projection", () => { + it("skips rows whose session embed is missing", async () => { const accountId = "123e4567-e89b-12d3-a456-426614174000"; - + const artistId = "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa"; vi.mocked(validateAuthContext).mockResolvedValue({ accountId, orgId: null, authToken: "test-token", }); - vi.mocked(selectRooms).mockRejectedValue(new Error("Database error")); + vi.mocked(selectChatsWithSessions).mockResolvedValue([ + { + id: "chat-keep", + title: "Keep", + session_id: "sess-1", + updated_at: "2024-01-02T00:00:00Z", + created_at: "2024-01-01T00:00:00Z", + active_stream_id: null, + last_assistant_message_at: null, + model_id: null, + session: sessionEmbed(accountId, artistId), + }, + { + id: "chat-orphan", + title: "Orphan", + session_id: "sess-2", + updated_at: "2024-01-03T00:00:00Z", + created_at: "2024-01-01T00:00:00Z", + active_stream_id: null, + last_assistant_message_at: null, + model_id: null, + session: null, + }, + ]); const request = createMockRequest("http://localhost/api/chats"); const response = await getChatsHandler(request); const json = await response.json(); - expect(response.status).toBe(500); - expect(json.status).toBe("error"); - expect(json.error).toBe("Database error"); + expect(json.chats).toEqual([ + { + id: "chat-keep", + title: "Keep", + accountId, + sessionId: "sess-1", + updatedAt: "2024-01-02T00:00:00Z", + artistId, + }, + ]); }); }); }); diff --git a/lib/chats/__tests__/validateGetChatsRequest.test.ts b/lib/chats/__tests__/validateGetChatsRequest.test.ts index b28f253c1..fc0d1a2f3 100644 --- a/lib/chats/__tests__/validateGetChatsRequest.test.ts +++ b/lib/chats/__tests__/validateGetChatsRequest.test.ts @@ -4,8 +4,8 @@ import { validateGetChatsRequest } from "../validateGetChatsRequest"; import { validateAuthContext } from "@/lib/auth/validateAuthContext"; import { canAccessAccount } from "@/lib/organizations/canAccessAccount"; +import { isRecoupAdmin } from "@/lib/organizations/isRecoupAdmin"; -// Mock dependencies vi.mock("@/lib/auth/validateAuthContext", () => ({ validateAuthContext: vi.fn(), })); @@ -14,24 +14,22 @@ vi.mock("@/lib/organizations/canAccessAccount", () => ({ canAccessAccount: vi.fn(), })); -vi.mock("@/lib/supabase/account_organization_ids/getAccountOrganizations", () => ({ - getAccountOrganizations: vi.fn(), +vi.mock("@/lib/organizations/isRecoupAdmin", () => ({ + isRecoupAdmin: vi.fn(), })); vi.mock("@/lib/networking/getCorsHeaders", () => ({ getCorsHeaders: vi.fn(() => new Headers()), })); -vi.mock("@/lib/const", () => ({ - RECOUP_ORG_ID: "recoup-org-id", -})); - describe("validateGetChatsRequest", () => { beforeEach(() => { vi.clearAllMocks(); + // Default: caller is NOT a Recoup admin. Admin tests override. + vi.mocked(isRecoupAdmin).mockResolvedValue(false); }); - it("should return error if auth fails", async () => { + it("returns auth error if validateAuthContext fails", async () => { vi.mocked(validateAuthContext).mockResolvedValue( NextResponse.json({ status: "error", error: "Unauthorized" }, { status: 401 }), ); @@ -40,226 +38,141 @@ describe("validateGetChatsRequest", () => { const result = await validateGetChatsRequest(request); expect(result).toBeInstanceOf(NextResponse); - const response = result as NextResponse; - expect(response.status).toBe(401); + expect((result as NextResponse).status).toBe(401); }); - it("should return single account ID for personal key", async () => { - const mockAccountId = "personal-account-123"; + it("scopes personal Bearer to the caller's account", async () => { + const accountId = "personal-account-123"; vi.mocked(validateAuthContext).mockResolvedValue({ - accountId: mockAccountId, - orgId: null, // Personal key + accountId, + orgId: null, authToken: "test-token", }); - const request = new NextRequest("http://localhost/api/chats", { - headers: { "x-api-key": "test-api-key" }, - }); + const request = new NextRequest("http://localhost/api/chats"); const result = await validateGetChatsRequest(request); expect(result).not.toBeInstanceOf(NextResponse); - expect(result).toEqual({ - account_ids: [mockAccountId], - artist_id: undefined, - }); + expect(result).toEqual({ accountIds: [accountId], artistAccountId: undefined }); }); - it("should return account_ids for org key", async () => { - const mockOrgId = "org-123"; + it("scopes org key to the caller's org account (target unspecified)", async () => { + const orgId = "org-123"; vi.mocked(validateAuthContext).mockResolvedValue({ - accountId: mockOrgId, - orgId: mockOrgId, + accountId: orgId, + orgId, authToken: "test-token", }); - const request = new NextRequest("http://localhost/api/chats", { - headers: { "x-api-key": "test-api-key" }, - }); + const request = new NextRequest("http://localhost/api/chats"); const result = await validateGetChatsRequest(request); - expect(result).not.toBeInstanceOf(NextResponse); - expect(result).toEqual({ - account_ids: [mockOrgId], - artist_id: undefined, - }); + expect(result).toEqual({ accountIds: [orgId], artistAccountId: undefined }); }); - it("should return account_ids for Recoup admin key", async () => { - const recoupOrgId = "recoup-org-id"; + it("returns undefined accountIds for Recoup admin (membership-based check)", async () => { + // Bearer-authed caller whose accountId is a member of RECOUP_ORG. + // orgId stays null (Bearer never sets it) — the admin check goes + // through isRecoupAdmin → account_organization_ids instead. + const adminAccountId = "admin-account-123"; vi.mocked(validateAuthContext).mockResolvedValue({ - accountId: recoupOrgId, - orgId: recoupOrgId, + accountId: adminAccountId, + orgId: null, authToken: "test-token", }); + vi.mocked(isRecoupAdmin).mockResolvedValue(true); - const request = new NextRequest("http://localhost/api/chats", { - headers: { "x-api-key": "recoup-admin-key" }, - }); + const request = new NextRequest("http://localhost/api/chats"); const result = await validateGetChatsRequest(request); - expect(result).not.toBeInstanceOf(NextResponse); - expect(result).toEqual({ - account_ids: [recoupOrgId], - artist_id: undefined, - }); + expect(isRecoupAdmin).toHaveBeenCalledWith(adminAccountId); + expect(result).toEqual({ accountIds: undefined, artistAccountId: undefined }); }); - it("should parse artist_account_id query parameter correctly", async () => { - const mockAccountId = "account-123"; - const artistId = "a1111111-1111-4111-8111-111111111111"; + it("scopes admin to a specific account when account_id is supplied", async () => { + const adminAccountId = "admin-account-123"; + const target = "a1111111-1111-4111-8111-111111111111"; vi.mocked(validateAuthContext).mockResolvedValue({ - accountId: mockAccountId, + accountId: adminAccountId, orgId: null, authToken: "test-token", }); + vi.mocked(canAccessAccount).mockResolvedValue(true); - const request = new NextRequest(`http://localhost/api/chats?artist_account_id=${artistId}`, { - headers: { "x-api-key": "test-api-key" }, - }); + const request = new NextRequest(`http://localhost/api/chats?account_id=${target}`); const result = await validateGetChatsRequest(request); - expect(result).not.toBeInstanceOf(NextResponse); - expect(result).toEqual({ - account_ids: [mockAccountId], - artist_id: artistId, + expect(canAccessAccount).toHaveBeenCalledWith({ + targetAccountId: target, + currentAccountId: adminAccountId, }); + expect(result).toEqual({ accountIds: [target], artistAccountId: undefined }); }); - it("should reject invalid artist_account_id query parameter", async () => { + it("rejects personal key trying to filter by account_id they can't access", async () => { + const accountId = "a1111111-1111-4111-8111-111111111111"; + const other = "b2222222-2222-4222-8222-222222222222"; vi.mocked(validateAuthContext).mockResolvedValue({ - accountId: "account-123", + accountId, orgId: null, authToken: "test-token", }); + vi.mocked(canAccessAccount).mockResolvedValue(false); - const request = new NextRequest("http://localhost/api/chats?artist_account_id=invalid", { - headers: { "x-api-key": "test-api-key" }, - }); + const request = new NextRequest(`http://localhost/api/chats?account_id=${other}`); const result = await validateGetChatsRequest(request); expect(result).toBeInstanceOf(NextResponse); - const response = result as NextResponse; - expect(response.status).toBe(400); + expect((result as NextResponse).status).toBe(403); }); - it("should reject personal key trying to filter by account_id", async () => { - const mockAccountId = "a1111111-1111-4111-8111-111111111111"; - const otherAccountId = "b2222222-2222-4222-8222-222222222222"; + it("passes artist_account_id through to the scope", async () => { + const accountId = "personal-account-123"; + const artistAccountId = "a1111111-1111-4111-8111-111111111111"; vi.mocked(validateAuthContext).mockResolvedValue({ - accountId: mockAccountId, - orgId: null, // Personal key + accountId, + orgId: null, authToken: "test-token", }); - const request = new NextRequest(`http://localhost/api/chats?account_id=${otherAccountId}`, { - headers: { "x-api-key": "test-api-key" }, - }); + const request = new NextRequest( + `http://localhost/api/chats?artist_account_id=${artistAccountId}`, + ); const result = await validateGetChatsRequest(request); - expect(result).toBeInstanceOf(NextResponse); - const response = result as NextResponse; - expect(response.status).toBe(403); + expect(result).toEqual({ accountIds: [accountId], artistAccountId }); }); - it("should allow org key to filter by account_id within org", async () => { - const mockOrgId = "c3333333-3333-4333-8333-333333333333"; - const targetAccountId = "d4444444-4444-4444-8444-444444444444"; + it("composes account_id + artist_account_id when both supplied", async () => { + const callerAccountId = "caller-account-123"; + const target = "a1111111-1111-4111-8111-111111111111"; + const artistAccountId = "c3333333-3333-4333-8333-333333333333"; vi.mocked(validateAuthContext).mockResolvedValue({ - accountId: mockOrgId, - orgId: mockOrgId, + accountId: callerAccountId, + orgId: null, authToken: "test-token", }); vi.mocked(canAccessAccount).mockResolvedValue(true); - const request = new NextRequest(`http://localhost/api/chats?account_id=${targetAccountId}`, { - headers: { "x-api-key": "test-api-key" }, - }); + const request = new NextRequest( + `http://localhost/api/chats?account_id=${target}&artist_account_id=${artistAccountId}`, + ); const result = await validateGetChatsRequest(request); - expect(canAccessAccount).toHaveBeenCalledWith({ - targetAccountId, - currentAccountId: mockOrgId, - }); - expect(result).not.toBeInstanceOf(NextResponse); - expect(result).toEqual({ - account_ids: [targetAccountId], - artist_id: undefined, - }); + expect(result).toEqual({ accountIds: [target], artistAccountId }); }); - it("should reject org key filtering by account_id not in org", async () => { - const mockOrgId = "f6666666-6666-4666-8666-666666666666"; - const notInOrgId = "b8888888-8888-4888-8888-888888888888"; + it("returns 400 for invalid artist_account_id UUID", async () => { vi.mocked(validateAuthContext).mockResolvedValue({ - accountId: mockOrgId, - orgId: mockOrgId, + accountId: "account-123", + orgId: null, authToken: "test-token", }); - vi.mocked(canAccessAccount).mockResolvedValue(false); - const request = new NextRequest(`http://localhost/api/chats?account_id=${notInOrgId}`, { - headers: { "x-api-key": "test-api-key" }, - }); + const request = new NextRequest("http://localhost/api/chats?artist_account_id=invalid"); const result = await validateGetChatsRequest(request); - expect(canAccessAccount).toHaveBeenCalledWith({ - targetAccountId: notInOrgId, - currentAccountId: mockOrgId, - }); expect(result).toBeInstanceOf(NextResponse); - const response = result as NextResponse; - expect(response.status).toBe(403); - }); - - it("should allow Recoup admin to filter by any account_id", async () => { - const recoupOrgId = "recoup-org-id"; - const anyAccountId = "a1111111-1111-4111-8111-111111111111"; - vi.mocked(validateAuthContext).mockResolvedValue({ - accountId: recoupOrgId, - orgId: recoupOrgId, - authToken: "test-token", - }); - vi.mocked(canAccessAccount).mockResolvedValue(true); // Admin always has access - - const request = new NextRequest(`http://localhost/api/chats?account_id=${anyAccountId}`, { - headers: { "x-api-key": "recoup-admin-key" }, - }); - const result = await validateGetChatsRequest(request); - - expect(canAccessAccount).toHaveBeenCalledWith({ - targetAccountId: anyAccountId, - currentAccountId: recoupOrgId, - }); - expect(result).not.toBeInstanceOf(NextResponse); - expect(result).toEqual({ - account_ids: [anyAccountId], - artist_id: undefined, - }); - }); - - it("should allow combining account_id and artist_account_id filters", async () => { - const mockOrgId = "c3333333-3333-4333-8333-333333333333"; - const targetAccountId = "d4444444-4444-4444-8444-444444444444"; - const artistId = "e5555555-5555-4555-8555-555555555555"; - vi.mocked(validateAuthContext).mockResolvedValue({ - accountId: mockOrgId, - orgId: mockOrgId, - authToken: "test-token", - }); - vi.mocked(canAccessAccount).mockResolvedValue(true); - - const request = new NextRequest( - `http://localhost/api/chats?account_id=${targetAccountId}&artist_account_id=${artistId}`, - { - headers: { "x-api-key": "test-api-key" }, - }, - ); - const result = await validateGetChatsRequest(request); - - expect(result).not.toBeInstanceOf(NextResponse); - expect(result).toEqual({ - account_ids: [targetAccountId], - artist_id: artistId, - }); + expect((result as NextResponse).status).toBe(400); }); }); diff --git a/lib/chats/getChatsHandler.ts b/lib/chats/getChatsHandler.ts index da180ddfa..0f7b254a7 100644 --- a/lib/chats/getChatsHandler.ts +++ b/lib/chats/getChatsHandler.ts @@ -1,19 +1,28 @@ import { NextRequest, NextResponse } from "next/server"; import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; import { validateGetChatsRequest } from "@/lib/chats/validateGetChatsRequest"; -import { selectRooms } from "@/lib/supabase/rooms/selectRooms"; +import { selectChatsWithSessions } from "@/lib/supabase/chats/selectChatsWithSessions"; /** * Handler for retrieving chats. + * * Requires authentication via x-api-key header or Authorization bearer token. * - * For personal keys: Returns array of chats for the account (if exists). - * For org keys: Returns array of chats for accounts in the organization. - * For Recoup admin key: Returns array of ALL chat records. + * Returns chats joined with their owning session so the response carries the + * `sessionId`, owning `accountId`, and `artistId` per row, enabling clients + * to render canonical `/sessions/{sid}/chats/{cid}` URLs and filter by + * artist context. + * + * Scope: + * - Personal/org key: chats belonging to the caller's account. + * - Personal/org key with `account_id`: chats for that account (when the + * caller can access it). + * - Recoup admin: all chats (or a specific account when `account_id` is set). * * Optional query parameters: - * - account_id: Filter to a specific account (validated against org membership) - * - artist_account_id: Filter to chats for a specific artist (UUID) + * - `account_id`: target account override (validated against access). + * - `artist_account_id`: scope chats to those whose owning session has the + * given artist context (`sessions.artist_id`). Composes with `account_id`. * * @param request - The request object containing query parameters * @returns A NextResponse with chats data @@ -25,43 +34,44 @@ export async function getChatsHandler(request: NextRequest): Promise { + if (!row.session) return []; + return [ + { + id: row.id, + title: row.title, + accountId: row.session.account_id, + sessionId: row.session_id, + updatedAt: row.updated_at, + artistId: row.session.artist_id, + }, + ]; + }); + return NextResponse.json( - { - status: "success", - chats, - }, - { - status: 200, - headers: getCorsHeaders(), - }, + { status: "success", chats }, + { status: 200, headers: getCorsHeaders() }, ); } catch (error) { + // Never leak raw exception messages on 500 — they can expose internal + // structure or DB errors. Caller gets a fixed string; the real cause + // stays in server logs. console.error("[ERROR] getChatsHandler:", error); return NextResponse.json( - { - status: "error", - error: error instanceof Error ? error.message : "Internal server error", - }, - { - status: 500, - headers: getCorsHeaders(), - }, + { status: "error", error: "Internal server error" }, + { status: 500, headers: getCorsHeaders() }, ); } } diff --git a/lib/chats/validateGetChatsRequest.ts b/lib/chats/validateGetChatsRequest.ts index e32fa127e..522249aa3 100644 --- a/lib/chats/validateGetChatsRequest.ts +++ b/lib/chats/validateGetChatsRequest.ts @@ -1,79 +1,87 @@ import type { NextRequest } from "next/server"; import { NextResponse } from "next/server"; +import { z } from "zod"; import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; import { validateAuthContext } from "@/lib/auth/validateAuthContext"; -import type { SelectRoomsParams } from "@/lib/supabase/rooms/selectRooms"; -import { buildGetChatsParams } from "./buildGetChatsParams"; -import { z } from "zod"; +import { canAccessAccount } from "@/lib/organizations/canAccessAccount"; +import { isRecoupAdmin } from "@/lib/organizations/isRecoupAdmin"; const getChatsQuerySchema = z.object({ account_id: z.string().uuid("account_id must be a valid UUID").optional(), artist_account_id: z.string().uuid("artist_account_id must be a valid UUID").optional(), }); +/** + * Validated scope params for the chats listing. + * `accountIds === undefined` signals admin scope (no account filter). + * `artistAccountId` is a separate, composable filter on `sessions.artist_id`. + */ +export interface GetChatsScope { + accountIds?: string[]; + artistAccountId?: string; +} + /** * 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 + * Auth via x-api-key or Authorization Bearer token. Returns the account scope: + * - Recoup admin (no target) → `{ accountIds: undefined }` (all chats). + * - Recoup admin with `account_id` → `{ accountIds: [target] }`. + * - Org/personal key with `account_id` → `{ accountIds: [target] }` if + * `canAccessAccount` allows, else 403. + * - Personal/org key without target → `{ accountIds: [callerAccountId] }`. * - * Query parameters: - * - account_id: Filter to a specific account (validated against org membership) - * - artist_account_id: Filter by artist ID + * Recoup-admin status is derived from `account_organization_ids` membership + * (not `auth.orgId`) because Bearer-authed callers never set `auth.orgId` — + * the previous `orgId === RECOUP_ORG_ID` branch was unreachable for them. * - * @param request - The NextRequest object - * @returns A NextResponse with an error if validation fails, or SelectRoomsParams + * `artistAccountId` (from `?artist_account_id=`) passes through verbatim + * and composes with the account scope at the SQL layer. */ export async function validateGetChatsRequest( request: NextRequest, -): Promise { - // Parse query parameters first +): Promise { const { searchParams } = new URL(request.url); - const queryParams = { + const queryResult = getChatsQuerySchema.safeParse({ account_id: searchParams.get("account_id") ?? undefined, artist_account_id: searchParams.get("artist_account_id") ?? undefined, - }; - - const queryResult = getChatsQuerySchema.safeParse(queryParams); + }); if (!queryResult.success) { const firstError = queryResult.error.issues[0]; return NextResponse.json( - { - status: "error", - error: firstError.message, - }, + { status: "error", error: firstError.message }, { status: 400, headers: getCorsHeaders() }, ); } - const { account_id: target_account_id, artist_account_id: artist_id } = queryResult.data; + const { account_id: targetAccountId, artist_account_id: artistAccountId } = queryResult.data; - // Use validateAuthContext for authentication const authResult = await validateAuthContext(request); if (authResult instanceof NextResponse) { return authResult; } + const { accountId } = authResult; - const { accountId: account_id } = authResult; - - // Use shared function to build params - const { params, error } = await buildGetChatsParams({ - account_id, - target_account_id, - artist_id, - }); + if (targetAccountId) { + const hasAccess = await canAccessAccount({ + targetAccountId, + currentAccountId: accountId, + }); + if (!hasAccess) { + return NextResponse.json( + { status: "error", error: "Access denied to specified account_id" }, + { status: 403, headers: getCorsHeaders() }, + ); + } + return { accountIds: [targetAccountId], artistAccountId }; + } - if (error) { - return NextResponse.json( - { - status: "error", - error, - }, - { status: 403, headers: getCorsHeaders() }, - ); + // Recoup admin → all chats. Membership-based so Bearer-authed admins + // get the same scope as x-api-key admins (auth.orgId is null for + // Bearer regardless of the caller's org memberships). + if (await isRecoupAdmin(accountId)) { + return { accountIds: undefined, artistAccountId }; } - return params; + return { accountIds: [accountId], artistAccountId }; } diff --git a/lib/mcp/tools/chats/__tests__/registerGetChatsTool.test.ts b/lib/mcp/tools/chats/__tests__/registerGetChatsTool.test.ts index 4cceb5de3..f2c7e0fc9 100644 --- a/lib/mcp/tools/chats/__tests__/registerGetChatsTool.test.ts +++ b/lib/mcp/tools/chats/__tests__/registerGetChatsTool.test.ts @@ -5,27 +5,31 @@ import type { ServerRequest, ServerNotification } from "@modelcontextprotocol/sd import { registerGetChatsTool } from "../registerGetChatsTool"; -const mockSelectRooms = vi.fn(); +const mockSelectChatsWithSessions = vi.fn(); const mockCanAccessAccount = vi.fn(); +const mockIsRecoupAdmin = vi.fn(); -vi.mock("@/lib/supabase/rooms/selectRooms", () => ({ - selectRooms: (...args: unknown[]) => mockSelectRooms(...args), +vi.mock("@/lib/supabase/chats/selectChatsWithSessions", () => ({ + selectChatsWithSessions: (...args: unknown[]) => mockSelectChatsWithSessions(...args), })); vi.mock("@/lib/organizations/canAccessAccount", () => ({ canAccessAccount: (...args: unknown[]) => mockCanAccessAccount(...args), })); +vi.mock("@/lib/organizations/isRecoupAdmin", () => ({ + isRecoupAdmin: (...args: unknown[]) => mockIsRecoupAdmin(...args), +})); + type ServerRequestHandlerExtra = RequestHandlerExtra; /** * Creates a mock extra object with optional authInfo. - * - * @param authInfo - Optional auth info object containing account ID. - * @param authInfo.accountId - The account ID for authentication. - * @returns A mock ServerRequestHandlerExtra object. */ -function createMockExtra(authInfo?: { accountId?: string }): ServerRequestHandlerExtra { +function createMockExtra(authInfo?: { + accountId?: string; + orgId?: string | null; +}): ServerRequestHandlerExtra { return { authInfo: authInfo ? { @@ -34,6 +38,7 @@ function createMockExtra(authInfo?: { accountId?: string }): ServerRequestHandle clientId: authInfo.accountId, extra: { accountId: authInfo.accountId, + orgId: authInfo.orgId ?? null, }, } : undefined, @@ -46,9 +51,11 @@ describe("registerGetChatsTool", () => { beforeEach(() => { vi.clearAllMocks(); + // Default: caller is NOT a Recoup admin. Admin tests override. + mockIsRecoupAdmin.mockResolvedValue(false); mockServer = { - registerTool: vi.fn((name, config, handler) => { + registerTool: vi.fn((_name, _config, handler) => { registeredHandler = handler; }), } as unknown as McpServer; @@ -60,20 +67,20 @@ describe("registerGetChatsTool", () => { expect(mockServer.registerTool).toHaveBeenCalledWith( "get_chats", expect.objectContaining({ - description: "Get chat conversations for accounts.", + description: expect.stringContaining("chat"), }), expect.any(Function), ); }); it("returns empty chats array when no records exist", async () => { - mockSelectRooms.mockResolvedValue([]); + mockSelectChatsWithSessions.mockResolvedValue([]); const result = await registeredHandler({}, createMockExtra({ accountId: "account-123" })); - expect(mockSelectRooms).toHaveBeenCalledWith({ - account_ids: ["account-123"], - artist_id: undefined, + expect(mockSelectChatsWithSessions).toHaveBeenCalledWith({ + accountIds: ["account-123"], + artistAccountId: undefined, }); expect(result).toEqual({ content: [ @@ -85,49 +92,53 @@ describe("registerGetChatsTool", () => { }); }); - it("returns chats array with records when they exist", async () => { - mockSelectRooms.mockResolvedValue([ + it("returns chats projected to the new wire shape including artistId", async () => { + mockSelectChatsWithSessions.mockResolvedValue([ { id: "chat-456", - account_id: "account-123", - artist_id: null, - topic: "Test Chat", + title: "Test Chat", + session_id: "sess-1", updated_at: "2024-01-01T00:00:00Z", + created_at: "2024-01-01T00:00:00Z", + active_stream_id: null, + last_assistant_message_at: null, + model_id: null, + session: { id: "sess-1", account_id: "account-123", artist_id: "artist-789" }, }, ]); const result = await registeredHandler({}, createMockExtra({ accountId: "account-123" })); - expect(result).toEqual({ - content: [ - { - type: "text", - text: expect.stringContaining('"chats":['), - }, - ], - }); - expect(result).toEqual({ - content: [ - { - type: "text", - text: expect.stringContaining('"id":"chat-456"'), - }, - ], - }); + const textNode = (result as { content: { type: string; text: string }[] }).content[0]; + expect(textNode.text).toContain('"sessionId":"sess-1"'); + expect(textNode.text).toContain('"accountId":"account-123"'); + expect(textNode.text).toContain('"artistId":"artist-789"'); }); - it("allows account_id override with access", async () => { - mockCanAccessAccount.mockResolvedValue(true); - mockSelectRooms.mockResolvedValue([ + it("surfaces artistId as null when session has no artist", async () => { + mockSelectChatsWithSessions.mockResolvedValue([ { id: "chat-456", - account_id: "target-account-789", - artist_id: null, - topic: "Test Chat", + title: "Test Chat", + session_id: "sess-1", updated_at: "2024-01-01T00:00:00Z", + created_at: "2024-01-01T00:00:00Z", + active_stream_id: null, + last_assistant_message_at: null, + model_id: null, + session: { id: "sess-1", account_id: "account-123", artist_id: null }, }, ]); + const result = await registeredHandler({}, createMockExtra({ accountId: "account-123" })); + const textNode = (result as { content: { type: string; text: string }[] }).content[0]; + expect(textNode.text).toContain('"artistId":null'); + }); + + it("allows account_id override with access", async () => { + mockCanAccessAccount.mockResolvedValue(true); + mockSelectChatsWithSessions.mockResolvedValue([]); + await registeredHandler( { account_id: "target-account-789" }, createMockExtra({ accountId: "org-account-id" }), @@ -137,9 +148,9 @@ describe("registerGetChatsTool", () => { targetAccountId: "target-account-789", currentAccountId: "org-account-id", }); - expect(mockSelectRooms).toHaveBeenCalledWith({ - account_ids: ["target-account-789"], - artist_id: undefined, + expect(mockSelectChatsWithSessions).toHaveBeenCalledWith({ + accountIds: ["target-account-789"], + artistAccountId: undefined, }); }); @@ -174,25 +185,35 @@ describe("registerGetChatsTool", () => { }); }); - it("filters by artist_account_id when provided", async () => { - const artistChats = [ - { id: "chat-1", account_id: "account-123", artist_id: "artist-456", topic: "Artist Chat" }, - ]; - mockSelectRooms.mockResolvedValue(artistChats); + it("passes artist_account_id through to the select", async () => { + mockSelectChatsWithSessions.mockResolvedValue([]); await registeredHandler( { artist_account_id: "artist-456" }, createMockExtra({ accountId: "account-123" }), ); - expect(mockSelectRooms).toHaveBeenCalledWith({ - account_ids: ["account-123"], - artist_id: "artist-456", + expect(mockSelectChatsWithSessions).toHaveBeenCalledWith({ + accountIds: ["account-123"], + artistAccountId: "artist-456", + }); + }); + + it("passes undefined accountIds for Recoup admin (membership-based)", async () => { + mockIsRecoupAdmin.mockResolvedValue(true); + mockSelectChatsWithSessions.mockResolvedValue([]); + + await registeredHandler({}, createMockExtra({ accountId: "admin-account-123" })); + + expect(mockIsRecoupAdmin).toHaveBeenCalledWith("admin-account-123"); + expect(mockSelectChatsWithSessions).toHaveBeenCalledWith({ + accountIds: undefined, + artistAccountId: undefined, }); }); - it("returns error when selectRooms fails", async () => { - mockSelectRooms.mockResolvedValue(null); + it("returns error when selectChatsWithSessions fails", async () => { + mockSelectChatsWithSessions.mockResolvedValue(null); const result = await registeredHandler({}, createMockExtra({ accountId: "account-123" })); diff --git a/lib/mcp/tools/chats/registerGetChatsTool.ts b/lib/mcp/tools/chats/registerGetChatsTool.ts index 7f999b4eb..d667964ba 100644 --- a/lib/mcp/tools/chats/registerGetChatsTool.ts +++ b/lib/mcp/tools/chats/registerGetChatsTool.ts @@ -3,25 +3,37 @@ import type { RequestHandlerExtra } from "@modelcontextprotocol/sdk/shared/proto import type { ServerRequest, ServerNotification } from "@modelcontextprotocol/sdk/types.js"; import { z } from "zod"; import type { McpAuthInfo } from "@/lib/mcp/verifyApiKey"; -import { selectRooms } from "@/lib/supabase/rooms/selectRooms"; import { getToolResultSuccess } from "@/lib/mcp/getToolResultSuccess"; import { getToolResultError } from "@/lib/mcp/getToolResultError"; -import { buildGetChatsParams } from "@/lib/chats/buildGetChatsParams"; +import { canAccessAccount } from "@/lib/organizations/canAccessAccount"; +import { isRecoupAdmin } from "@/lib/organizations/isRecoupAdmin"; +import { selectChatsWithSessions } from "@/lib/supabase/chats/selectChatsWithSessions"; const getChatsSchema = z.object({ account_id: z.string().optional().describe("The account ID to filter chats for."), - artist_account_id: z.string().optional().describe("The artist account ID to filter chats for."), + artist_account_id: z + .string() + .optional() + .describe( + "Filter chats to those whose owning session is in the specified artist context " + + "(matches `sessions.artist_id`). Composes with `account_id`.", + ), }); export type GetChatsArgs = z.infer; /** * Registers the "get_chats" tool on the MCP server. - * Retrieves chat conversations (rooms) for accounts. * - * For personal keys: Returns chats for the key owner's account. - * For org keys: Returns chats for all accounts in the organization. - * For Recoup admin key: Returns ALL chat records. + * Returns chats joined with their owning session so each row carries + * `sessionId`, owning `accountId`, and `artistId`. Scope mirrors + * GET /api/chats: personal/org → caller's account; Recoup admin → all; + * or a specific account when `account_id` is supplied and the caller + * can access it. `artist_account_id` further scopes by artist context. + * + * Admin status is derived from `account_organization_ids` membership so + * that Bearer-authed callers get the same admin scope as x-api-key + * callers with an org-bound key. * * @param server - The MCP server instance to register the tool on. */ @@ -33,7 +45,7 @@ export function registerGetChatsTool(server: McpServer): void { inputSchema: getChatsSchema, }, async (args: GetChatsArgs, extra: RequestHandlerExtra) => { - const { account_id, artist_account_id } = args; + const { account_id: targetAccountId, artist_account_id: artistAccountId } = args; const authInfo = extra.authInfo as McpAuthInfo | undefined; const accountId = authInfo?.extra?.accountId; @@ -44,22 +56,39 @@ export function registerGetChatsTool(server: McpServer): void { ); } - const { params, error } = await buildGetChatsParams({ - account_id: accountId, - target_account_id: account_id, - artist_id: artist_account_id, - }); - - if (error) { - return getToolResultError(error); + let accountIds: string[] | undefined; + if (targetAccountId) { + const hasAccess = await canAccessAccount({ + targetAccountId, + currentAccountId: accountId, + }); + if (!hasAccess) { + return getToolResultError("Access denied to specified account_id"); + } + accountIds = [targetAccountId]; + } else { + accountIds = (await isRecoupAdmin(accountId)) ? undefined : [accountId]; } - const chats = await selectRooms(params); - - if (chats === null) { + const rows = await selectChatsWithSessions({ accountIds, artistAccountId }); + if (rows === null) { return getToolResultError("Failed to retrieve chats"); } + const chats = rows.flatMap(row => { + if (!row.session) return []; + return [ + { + id: row.id, + title: row.title, + accountId: row.session.account_id, + sessionId: row.session_id, + updatedAt: row.updated_at, + artistId: row.session.artist_id, + }, + ]; + }); + return getToolResultSuccess({ chats }); }, ); diff --git a/lib/organizations/__tests__/isRecoupAdmin.test.ts b/lib/organizations/__tests__/isRecoupAdmin.test.ts new file mode 100644 index 000000000..3a5d96df2 --- /dev/null +++ b/lib/organizations/__tests__/isRecoupAdmin.test.ts @@ -0,0 +1,47 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { isRecoupAdmin } from "@/lib/organizations/isRecoupAdmin"; +import { getAccountOrganizations } from "@/lib/supabase/account_organization_ids/getAccountOrganizations"; + +vi.mock("@/lib/supabase/account_organization_ids/getAccountOrganizations", () => ({ + getAccountOrganizations: vi.fn(), +})); + +vi.mock("@/lib/const", () => ({ + RECOUP_ORG_ID: "recoup-org-id", +})); + +beforeEach(() => { + vi.clearAllMocks(); +}); + +describe("isRecoupAdmin", () => { + it("returns false for an empty/missing accountId without querying", async () => { + expect(await isRecoupAdmin("")).toBe(false); + expect(getAccountOrganizations).not.toHaveBeenCalled(); + }); + + it("returns true when the account is a member of the Recoup org", async () => { + vi.mocked(getAccountOrganizations).mockResolvedValue([ + { organization_id: "some-other-org" } as never, + { organization_id: "recoup-org-id" } as never, + ]); + + expect(await isRecoupAdmin("acc-1")).toBe(true); + expect(getAccountOrganizations).toHaveBeenCalledWith({ accountId: "acc-1" }); + }); + + it("returns false when the account is in orgs but none is Recoup", async () => { + vi.mocked(getAccountOrganizations).mockResolvedValue([ + { organization_id: "org-a" } as never, + { organization_id: "org-b" } as never, + ]); + + expect(await isRecoupAdmin("acc-1")).toBe(false); + }); + + it("returns false when the account is in no orgs", async () => { + vi.mocked(getAccountOrganizations).mockResolvedValue([]); + + expect(await isRecoupAdmin("acc-1")).toBe(false); + }); +}); diff --git a/lib/organizations/isRecoupAdmin.ts b/lib/organizations/isRecoupAdmin.ts new file mode 100644 index 000000000..af8cfb55a --- /dev/null +++ b/lib/organizations/isRecoupAdmin.ts @@ -0,0 +1,26 @@ +import { getAccountOrganizations } from "@/lib/supabase/account_organization_ids/getAccountOrganizations"; +import { RECOUP_ORG_ID } from "@/lib/const"; + +/** + * Returns `true` iff `accountId` is a member of the Recoup organization + * (`RECOUP_ORG_ID`). Used to grant admin-level scope (read/write across + * all accounts) to Recoup team members regardless of which auth method + * they used. + * + * Membership is read via `account_organization_ids` so this works for + * Bearer-authed callers too — `auth.orgId` is only populated by + * x-api-key org keys, leaving Bearer admins unrecognized if you check + * `auth.orgId === RECOUP_ORG_ID` alone. + * + * `canAccessAccount` deliberately inlines an equivalent check because + * it reuses the same org-list query for the subsequent shared-org check + * — calling this helper there would double the DB query for no benefit. + * + * @param accountId - The account to check. + * @returns `true` if the account is in the Recoup org; `false` otherwise. + */ +export async function isRecoupAdmin(accountId: string): Promise { + if (!accountId) return false; + const orgs = await getAccountOrganizations({ accountId }); + return orgs.some(m => m.organization_id === RECOUP_ORG_ID); +} diff --git a/lib/supabase/chats/__tests__/selectChatsWithSessions.test.ts b/lib/supabase/chats/__tests__/selectChatsWithSessions.test.ts new file mode 100644 index 000000000..2d9de9afe --- /dev/null +++ b/lib/supabase/chats/__tests__/selectChatsWithSessions.test.ts @@ -0,0 +1,123 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { selectChatsWithSessions } from "@/lib/supabase/chats/selectChatsWithSessions"; + +const fromMock = vi.fn(); +const selectMock = vi.fn(); +const inMock = vi.fn(); +const eqMock = vi.fn(); +const orderMock = vi.fn(); + +vi.mock("@/lib/supabase/serverClient", () => ({ + default: { + from: (...args: unknown[]) => fromMock(...args), + }, +})); + +beforeEach(() => { + vi.clearAllMocks(); + // Default chain: from().select().in().eq().order() -> resolves to { data, error } + const builder = { + select: selectMock, + in: inMock, + eq: eqMock, + order: orderMock, + }; + fromMock.mockReturnValue(builder); + selectMock.mockReturnValue(builder); + inMock.mockReturnValue(builder); + eqMock.mockReturnValue(builder); + orderMock.mockResolvedValue({ data: [], error: null }); +}); + +describe("selectChatsWithSessions", () => { + it("queries chats joined with sessions, filtered by account_ids, ordered by updated_at desc", async () => { + const rows = [ + { + id: "chat-1", + title: "Hello", + session_id: "sess-1", + updated_at: "2024-01-02T00:00:00Z", + session: { id: "sess-1", account_id: "acc-1" }, + }, + ]; + orderMock.mockResolvedValueOnce({ data: rows, error: null }); + + const result = await selectChatsWithSessions({ accountIds: ["acc-1", "acc-2"] }); + + expect(fromMock).toHaveBeenCalledWith("chats"); + expect(selectMock).toHaveBeenCalledTimes(1); + const selectArg = selectMock.mock.calls[0]?.[0]; + expect(typeof selectArg).toBe("string"); + expect(String(selectArg)).toContain("session:sessions!inner"); + expect(String(selectArg)).toContain("account_id"); + expect(String(selectArg)).toContain("artist_id"); + + expect(inMock).toHaveBeenCalledWith("session.account_id", ["acc-1", "acc-2"]); + expect(eqMock).not.toHaveBeenCalled(); + expect(orderMock).toHaveBeenCalledWith("updated_at", { ascending: false }); + expect(result).toEqual(rows); + }); + + it("composes accountIds + artistAccountId — both filters applied", async () => { + orderMock.mockResolvedValueOnce({ data: [], error: null }); + + await selectChatsWithSessions({ + accountIds: ["acc-1"], + artistAccountId: "artist-9", + }); + + expect(inMock).toHaveBeenCalledWith("session.account_id", ["acc-1"]); + expect(eqMock).toHaveBeenCalledWith("session.artist_id", "artist-9"); + }); + + it("applies the artist filter alone (admin scope + artist)", async () => { + orderMock.mockResolvedValueOnce({ data: [], error: null }); + + await selectChatsWithSessions({ artistAccountId: "artist-9" }); + + expect(inMock).not.toHaveBeenCalled(); + expect(eqMock).toHaveBeenCalledWith("session.artist_id", "artist-9"); + }); + + it("returns [] without calling .in() when accountIds is undefined (admin: all)", async () => { + const rows = [ + { + id: "chat-1", + title: "Admin", + session_id: "sess-1", + updated_at: "2024-01-02T00:00:00Z", + session: { id: "sess-1", account_id: "acc-1" }, + }, + ]; + orderMock.mockResolvedValueOnce({ data: rows, error: null }); + + const result = await selectChatsWithSessions({}); + + expect(inMock).not.toHaveBeenCalled(); + expect(orderMock).toHaveBeenCalledWith("updated_at", { ascending: false }); + expect(result).toEqual(rows); + }); + + it("short-circuits to [] when accountIds is an empty array", async () => { + const result = await selectChatsWithSessions({ accountIds: [] }); + + expect(result).toEqual([]); + expect(fromMock).not.toHaveBeenCalled(); + }); + + it("returns null on supabase error", async () => { + orderMock.mockResolvedValueOnce({ data: null, error: { message: "boom" } }); + + const result = await selectChatsWithSessions({ accountIds: ["acc-1"] }); + + expect(result).toBeNull(); + }); + + it("returns [] when supabase returns no data and no error", async () => { + orderMock.mockResolvedValueOnce({ data: null, error: null }); + + const result = await selectChatsWithSessions({ accountIds: ["acc-1"] }); + + expect(result).toEqual([]); + }); +}); diff --git a/lib/supabase/chats/selectChatsWithSessions.ts b/lib/supabase/chats/selectChatsWithSessions.ts new file mode 100644 index 000000000..932341ff0 --- /dev/null +++ b/lib/supabase/chats/selectChatsWithSessions.ts @@ -0,0 +1,57 @@ +import supabase from "@/lib/supabase/serverClient"; + +export interface SelectChatsWithSessionsParams { + /** + * Owning account IDs to filter by (via `sessions.account_id`). + * - `undefined` returns chats across all accounts (admin scope). + * - `[]` short-circuits to an empty result. + */ + accountIds?: string[]; + /** + * Optional artist account id to filter by (via `sessions.artist_id`). + * Composes with `accountIds`: scopes the result to chats whose owning + * session both belongs to one of the accountIds AND is in the given + * artist context. + */ + artistAccountId?: string; +} + +const SELECT = ` + *, + session:sessions!inner ( id, account_id, artist_id ) +` as const; + +/** + * Reads chats joined to their owning session, optionally scoped to a set of + * account IDs through `sessions.account_id` and/or an artist context through + * `sessions.artist_id`. Ordered by `chats.updated_at` descending so newest + * activity surfaces first. + * + * Returns `null` when Supabase reports an error so callers can distinguish a + * transient failure from an empty result. Row shape is inferred from the + * typed supabase-js client — both `chats.*` and the embedded `session` + * projection surface to callers without an explicit type alias. + */ +export async function selectChatsWithSessions(params: SelectChatsWithSessionsParams = {}) { + const { accountIds, artistAccountId } = params; + + if (accountIds !== undefined && accountIds.length === 0) { + return []; + } + + let query = supabase.from("chats").select(SELECT); + if (accountIds !== undefined) { + query = query.in("session.account_id", accountIds); + } + if (artistAccountId) { + query = query.eq("session.artist_id", artistAccountId); + } + const { data, error } = await query.order("updated_at", { ascending: false }); + + if (error) { + console.error("[selectChatsWithSessions] error:", error); + return null; + } + + return data ?? []; +}