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/app/api/sessions/[sessionId]/__tests__/route.test.ts b/app/api/sessions/[sessionId]/__tests__/route.test.ts index 17afe983e..e75ba6961 100644 --- a/app/api/sessions/[sessionId]/__tests__/route.test.ts +++ b/app/api/sessions/[sessionId]/__tests__/route.test.ts @@ -63,6 +63,7 @@ function makePatchReqRaw( const mockRow: SessionRow = { id: "sess_1", account_id: "acc-uuid-1", + artist_id: null, title: "Test session", status: "running", repo_owner: "acme", @@ -186,6 +187,7 @@ describe("GET /api/sessions/[sessionId]", () => { session: { id: "sess_1", userId: "acc-uuid-1", + artistId: null, title: "Test session", status: "running", repoOwner: "acme", diff --git a/app/api/sessions/[sessionId]/chats/[chatId]/read/route.ts b/app/api/sessions/[sessionId]/chats/[chatId]/read/route.ts new file mode 100644 index 000000000..e67d3fe06 --- /dev/null +++ b/app/api/sessions/[sessionId]/chats/[chatId]/read/route.ts @@ -0,0 +1,38 @@ +import { NextRequest, NextResponse } from "next/server"; +import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; +import { markChatReadHandler } from "@/lib/sessions/chats/markChatReadHandler"; + +/** + * OPTIONS handler for CORS preflight requests. + * + * @returns A NextResponse with CORS headers. + */ +export async function OPTIONS() { + return new NextResponse(null, { + status: 200, + headers: getCorsHeaders(), + }); +} + +/** + * POST /api/sessions/{sessionId}/chats/{chatId}/read + * + * Marks a chat as read for the authenticated account. Authenticates via + * Privy Bearer token or x-api-key header. + * + * @param request - The incoming request. + * @param options - Route options containing the async params. + * @param options.params - Route params containing sessionId and chatId. + * @returns A NextResponse with `{ success: true }` on 200, or an error. + */ +export async function POST( + request: NextRequest, + options: { params: Promise<{ sessionId: string; chatId: string }> }, +) { + const { sessionId, chatId } = await options.params; + return markChatReadHandler(request, sessionId, chatId); +} + +export const dynamic = "force-dynamic"; +export const fetchCache = "force-no-store"; +export const revalidate = 0; 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..1a0c31cd8 100644 --- a/lib/chats/getChatsHandler.ts +++ b/lib/chats/getChatsHandler.ts @@ -1,19 +1,30 @@ 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. Chats whose owning session is archived + * (`sessions.status === "archived"`) are excluded — archive is the + * soft-delete path for sessions. + * + * 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 +36,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..0230d5c86 100644 --- a/lib/mcp/tools/chats/registerGetChatsTool.ts +++ b/lib/mcp/tools/chats/registerGetChatsTool.ts @@ -3,25 +3,38 @@ 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. + * Chats whose owning session is archived are excluded. + * + * 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 +46,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 +57,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/sessions/__tests__/baseSessionRow.ts b/lib/sessions/__tests__/baseSessionRow.ts index 8c5fe8095..0f40d0152 100644 --- a/lib/sessions/__tests__/baseSessionRow.ts +++ b/lib/sessions/__tests__/baseSessionRow.ts @@ -9,6 +9,7 @@ export function baseSessionRow(overrides: Partial> = {}): Tab return { id: "sess_1", account_id: "acc-uuid-1", + artist_id: null, title: "Test session", status: "running", repo_owner: null, diff --git a/lib/sessions/__tests__/buildSessionInsertRow.test.ts b/lib/sessions/__tests__/buildSessionInsertRow.test.ts index 2b75c679a..0e2ac881b 100644 --- a/lib/sessions/__tests__/buildSessionInsertRow.test.ts +++ b/lib/sessions/__tests__/buildSessionInsertRow.test.ts @@ -47,4 +47,23 @@ describe("buildSessionInsertRow", () => { }); expect(row.sandbox_state).toEqual({ type: "vercel" }); }); + + it("writes artist_id when artistId is provided", () => { + const row = buildSessionInsertRow({ + accountId: "acc-1", + title: "Berlin", + cloneUrl: DEFAULT_CLONE, + artistId: "artist-uuid-1", + }); + expect(row.artist_id).toBe("artist-uuid-1"); + }); + + it("sets artist_id to null when artistId is omitted", () => { + const row = buildSessionInsertRow({ + accountId: "acc-1", + title: "Berlin", + cloneUrl: DEFAULT_CLONE, + }); + expect(row.artist_id).toBeNull(); + }); }); diff --git a/lib/sessions/__tests__/createSessionHandler.persistence.test.ts b/lib/sessions/__tests__/createSessionHandler.persistence.test.ts index cf54c9eff..043b65e6e 100644 --- a/lib/sessions/__tests__/createSessionHandler.persistence.test.ts +++ b/lib/sessions/__tests__/createSessionHandler.persistence.test.ts @@ -107,6 +107,31 @@ describe("createSessionHandler — persistence", () => { expect(vi.mocked(insertSession).mock.calls[0][0].title).toBe("Hello world"); }); + it("writes artist_id when body carries artistId, and exposes it on the response", async () => { + const artistId = "a25c5dc5-3eb2-4fff-9a5e-39e90c9d4f02"; + vi.mocked(validateCreateSessionBody).mockResolvedValue(okValidated({ body: { artistId } })); + vi.mocked(insertSession).mockResolvedValue(baseSessionRow({ artist_id: artistId })); + vi.mocked(insertChat).mockResolvedValue(baseChatRow()); + + const res = await createSessionHandler(makeCreateSessionReq({ artistId })); + expect(res.status).toBe(200); + + expect(vi.mocked(insertSession).mock.calls[0][0].artist_id).toBe(artistId); + + const body = (await res.json()) as { session: { artistId: string | null } }; + expect(body.session.artistId).toBe(artistId); + }); + + it("writes artist_id as null when artistId is omitted", async () => { + vi.mocked(validateCreateSessionBody).mockResolvedValue(okValidated()); + vi.mocked(insertSession).mockResolvedValue(baseSessionRow()); + vi.mocked(insertChat).mockResolvedValue(baseChatRow()); + + await createSessionHandler(makeCreateSessionReq({})); + + expect(vi.mocked(insertSession).mock.calls[0][0].artist_id).toBeNull(); + }); + it("returns 500 when insertSession fails", async () => { vi.mocked(validateCreateSessionBody).mockResolvedValue(okValidated()); vi.mocked(insertSession).mockResolvedValue(null); diff --git a/lib/sessions/__tests__/validateCreateSessionBody.test.ts b/lib/sessions/__tests__/validateCreateSessionBody.test.ts index 85491c44d..39fc1c42b 100644 --- a/lib/sessions/__tests__/validateCreateSessionBody.test.ts +++ b/lib/sessions/__tests__/validateCreateSessionBody.test.ts @@ -74,4 +74,27 @@ describe("validateCreateSessionBody", () => { expect.objectContaining({ organizationId: orgId }), ); }); + + it("accepts a valid artistId and surfaces it on the validated body", async () => { + vi.mocked(validateAuthContext).mockResolvedValue(okAuth); + + const artistId = "a25c5dc5-3eb2-4fff-9a5e-39e90c9d4f02"; + const result = await validateCreateSessionBody(req({ artistId })); + expect(result).not.toBeInstanceOf(NextResponse); + if (!(result instanceof NextResponse)) { + expect(result.body.artistId).toBe(artistId); + } + }); + + it("rejects a non-UUID artistId with 400", async () => { + vi.mocked(validateAuthContext).mockResolvedValue(okAuth); + + const result = await validateCreateSessionBody(req({ artistId: "not-a-uuid" })); + expect(result).toBeInstanceOf(NextResponse); + if (result instanceof NextResponse) { + expect(result.status).toBe(400); + const body = (await result.json()) as { status: string; error: string }; + expect(body.error).toMatch(/UUID/i); + } + }); }); diff --git a/lib/sessions/buildSessionInsertRow.ts b/lib/sessions/buildSessionInsertRow.ts index 57402034a..c37f84722 100644 --- a/lib/sessions/buildSessionInsertRow.ts +++ b/lib/sessions/buildSessionInsertRow.ts @@ -10,6 +10,12 @@ interface BuildSessionInsertRowInput { * `cloneUrl`. */ cloneUrl: string; + /** + * Optional artist account this session belongs to. Backs the chat + * sidebar's artist filter; omitted for sessions not tied to an + * artist context. + */ + artistId?: string; } /** @@ -18,14 +24,15 @@ interface BuildSessionInsertRowInput { * the default / null-coalescing rules so the handler can stay focused * on HTTP and persistence flow. * - * @param input - The validated body, owning account id, resolved title, and resolved clone URL. + * @param input - The validated body, owning account id, resolved title, resolved clone URL, and optional artist id. * @returns A row ready to pass to `insertSession`. */ export function buildSessionInsertRow(input: BuildSessionInsertRowInput): TablesInsert<"sessions"> { - const { accountId, title, cloneUrl } = input; + const { accountId, title, cloneUrl, artistId } = input; return { id: generateUUID(), account_id: accountId, + artist_id: artistId ?? null, title, status: "running", branch: null, diff --git a/lib/sessions/chats/__tests__/markChatReadHandler.test.ts b/lib/sessions/chats/__tests__/markChatReadHandler.test.ts new file mode 100644 index 000000000..457fb95df --- /dev/null +++ b/lib/sessions/chats/__tests__/markChatReadHandler.test.ts @@ -0,0 +1,79 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { NextRequest, NextResponse } from "next/server"; +import { baseSessionRow } from "@/lib/sessions/__tests__/baseSessionRow"; +import { baseChatRow } from "@/lib/sessions/__tests__/baseChatRow"; + +vi.mock("@/lib/networking/getCorsHeaders", () => ({ + getCorsHeaders: () => ({ "Access-Control-Allow-Origin": "*" }), +})); +vi.mock("@/lib/sessions/chats/validateMarkChatReadRequest", () => ({ + validateMarkChatReadRequest: vi.fn(), +})); +vi.mock("@/lib/supabase/chat_reads/upsertChatRead", () => ({ + upsertChatRead: vi.fn(), +})); + +const { validateMarkChatReadRequest } = await import( + "@/lib/sessions/chats/validateMarkChatReadRequest" +); +const { upsertChatRead } = await import("@/lib/supabase/chat_reads/upsertChatRead"); +const { markChatReadHandler } = await import("@/lib/sessions/chats/markChatReadHandler"); + +const accountId = "acc-uuid-1"; + +function makeReq(): NextRequest { + return new NextRequest("https://example.com/api/sessions/sess_1/chats/chat_1/read", { + method: "POST", + }); +} + +function mockValidated() { + vi.mocked(validateMarkChatReadRequest).mockResolvedValue({ + auth: { accountId, orgId: null, authToken: "tok" }, + session: baseSessionRow({ id: "sess_1", account_id: accountId }), + chat: baseChatRow({ id: "chat_1", session_id: "sess_1" }), + }); +} + +describe("markChatReadHandler", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("forwards the NextResponse from validateMarkChatReadRequest as-is", async () => { + const failure = NextResponse.json({ status: "error", error: "Forbidden" }, { status: 403 }); + vi.mocked(validateMarkChatReadRequest).mockResolvedValue(failure); + + const res = await markChatReadHandler(makeReq(), "sess_1", "chat_1"); + expect(res).toBe(failure); + expect(upsertChatRead).not.toHaveBeenCalled(); + }); + + it("returns { success: true } when upsert succeeds", async () => { + mockValidated(); + vi.mocked(upsertChatRead).mockResolvedValue({ + account_id: accountId, + chat_id: "chat_1", + last_read_at: "2026-05-21T00:00:00.000Z", + created_at: "2026-05-21T00:00:00.000Z", + updated_at: "2026-05-21T00:00:00.000Z", + }); + + const res = await markChatReadHandler(makeReq(), "sess_1", "chat_1"); + expect(res.status).toBe(200); + expect(await res.json()).toEqual({ success: true }); + expect(upsertChatRead).toHaveBeenCalledWith(accountId, "chat_1"); + }); + + it("returns 500 when upsertChatRead fails", async () => { + mockValidated(); + vi.mocked(upsertChatRead).mockResolvedValue(null); + + const res = await markChatReadHandler(makeReq(), "sess_1", "chat_1"); + expect(res.status).toBe(500); + expect(await res.json()).toEqual({ + status: "error", + error: "Failed to mark chat as read", + }); + }); +}); diff --git a/lib/sessions/chats/__tests__/validateMarkChatReadRequest.test.ts b/lib/sessions/chats/__tests__/validateMarkChatReadRequest.test.ts new file mode 100644 index 000000000..3149e6e24 --- /dev/null +++ b/lib/sessions/chats/__tests__/validateMarkChatReadRequest.test.ts @@ -0,0 +1,152 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { NextRequest, NextResponse } from "next/server"; +import { baseSessionRow } from "@/lib/sessions/__tests__/baseSessionRow"; +import { baseChatRow } from "@/lib/sessions/__tests__/baseChatRow"; + +vi.mock("@/lib/networking/getCorsHeaders", () => ({ + getCorsHeaders: () => ({ "Access-Control-Allow-Origin": "*" }), +})); +vi.mock("@/lib/auth/validateAuthContext", () => ({ + validateAuthContext: vi.fn(), +})); +vi.mock("@/lib/supabase/sessions/selectSessions", () => ({ + selectSessions: vi.fn(), +})); +vi.mock("@/lib/supabase/chats/selectChats", () => ({ + selectChats: vi.fn(), +})); + +const { validateAuthContext } = await import("@/lib/auth/validateAuthContext"); +const { selectSessions } = await import("@/lib/supabase/sessions/selectSessions"); +const { selectChats } = await import("@/lib/supabase/chats/selectChats"); +const { validateMarkChatReadRequest } = await import( + "@/lib/sessions/chats/validateMarkChatReadRequest" +); + +const accountId = "acc-uuid-1"; + +function makeReq(): NextRequest { + return new NextRequest("https://example.com/api/sessions/sess_1/chats/chat_1/read", { + method: "POST", + }); +} + +describe("validateMarkChatReadRequest", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("forwards the auth NextResponse when validateAuthContext rejects", async () => { + const failure = NextResponse.json({ error: "Unauthorized" }, { status: 401 }); + vi.mocked(validateAuthContext).mockResolvedValue(failure); + + const res = await validateMarkChatReadRequest(makeReq(), "sess_1", "chat_1"); + expect(res).toBe(failure); + expect(selectSessions).not.toHaveBeenCalled(); + }); + + it("returns 404 when the session is missing", async () => { + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId, + orgId: null, + authToken: "tok", + }); + vi.mocked(selectSessions).mockResolvedValue([]); + vi.mocked(selectChats).mockResolvedValue([baseChatRow({ id: "chat_1" })]); + + const res = await validateMarkChatReadRequest(makeReq(), "sess_missing", "chat_1"); + expect(res).toBeInstanceOf(NextResponse); + if (res instanceof NextResponse) { + expect(res.status).toBe(404); + expect(await res.json()).toEqual({ + status: "error", + error: "Session not found", + }); + } + }); + + it("returns 403 when the session belongs to a different account", async () => { + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId, + orgId: null, + authToken: "tok", + }); + vi.mocked(selectSessions).mockResolvedValue([ + baseSessionRow({ id: "sess_1", account_id: "acc-OTHER" }), + ]); + vi.mocked(selectChats).mockResolvedValue([baseChatRow({ id: "chat_1", session_id: "sess_1" })]); + + const res = await validateMarkChatReadRequest(makeReq(), "sess_1", "chat_1"); + expect(res).toBeInstanceOf(NextResponse); + if (res instanceof NextResponse) { + expect(res.status).toBe(403); + expect(await res.json()).toEqual({ status: "error", error: "Forbidden" }); + } + }); + + it("returns 404 when the chat is missing", async () => { + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId, + orgId: null, + authToken: "tok", + }); + vi.mocked(selectSessions).mockResolvedValue([ + baseSessionRow({ id: "sess_1", account_id: accountId }), + ]); + vi.mocked(selectChats).mockResolvedValue([]); + + const res = await validateMarkChatReadRequest(makeReq(), "sess_1", "chat_missing"); + expect(res).toBeInstanceOf(NextResponse); + if (res instanceof NextResponse) { + expect(res.status).toBe(404); + expect(await res.json()).toEqual({ + status: "error", + error: "Chat not found", + }); + } + }); + + it("returns 404 when the chat belongs to a different session", async () => { + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId, + orgId: null, + authToken: "tok", + }); + vi.mocked(selectSessions).mockResolvedValue([ + baseSessionRow({ id: "sess_1", account_id: accountId }), + ]); + vi.mocked(selectChats).mockResolvedValue([ + baseChatRow({ id: "chat_1", session_id: "sess_OTHER" }), + ]); + + const res = await validateMarkChatReadRequest(makeReq(), "sess_1", "chat_1"); + expect(res).toBeInstanceOf(NextResponse); + if (res instanceof NextResponse) { + expect(res.status).toBe(404); + expect(await res.json()).toEqual({ + status: "error", + error: "Chat not found", + }); + } + }); + + it("returns { auth, session, chat } on the happy path", async () => { + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId, + orgId: null, + authToken: "tok", + }); + vi.mocked(selectSessions).mockResolvedValue([ + baseSessionRow({ id: "sess_1", account_id: accountId }), + ]); + vi.mocked(selectChats).mockResolvedValue([baseChatRow({ id: "chat_1", session_id: "sess_1" })]); + + const res = await validateMarkChatReadRequest(makeReq(), "sess_1", "chat_1"); + expect(res).not.toBeInstanceOf(NextResponse); + if (!(res instanceof NextResponse)) { + expect(res.auth.accountId).toBe(accountId); + expect(res.session.id).toBe("sess_1"); + expect(res.chat.id).toBe("chat_1"); + } + }); +}); diff --git a/lib/sessions/chats/markChatReadHandler.ts b/lib/sessions/chats/markChatReadHandler.ts new file mode 100644 index 000000000..39223bc89 --- /dev/null +++ b/lib/sessions/chats/markChatReadHandler.ts @@ -0,0 +1,35 @@ +import { NextRequest, NextResponse } from "next/server"; +import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; +import { validateMarkChatReadRequest } from "@/lib/sessions/chats/validateMarkChatReadRequest"; +import { upsertChatRead } from "@/lib/supabase/chat_reads/upsertChatRead"; + +/** + * Handles `POST /api/sessions/{sessionId}/chats/{chatId}/read`. + * Upserts `chat_reads.last_read_at` for the authenticated account so + * subsequent list-chats calls report `hasUnread: false` for this chat. + * + * @param request - The incoming request. + * @param sessionId - The parent session id. + * @param chatId - The chat id to mark as read. + * @returns A NextResponse with `{ success: true }` on 200, or an error. + */ +export async function markChatReadHandler( + request: NextRequest, + sessionId: string, + chatId: string, +): Promise { + const validated = await validateMarkChatReadRequest(request, sessionId, chatId); + if (validated instanceof NextResponse) { + return validated; + } + + const row = await upsertChatRead(validated.auth.accountId, chatId); + if (!row) { + return NextResponse.json( + { status: "error", error: "Failed to mark chat as read" }, + { status: 500, headers: getCorsHeaders() }, + ); + } + + return NextResponse.json({ success: true }, { status: 200, headers: getCorsHeaders() }); +} diff --git a/lib/sessions/chats/validateMarkChatReadRequest.ts b/lib/sessions/chats/validateMarkChatReadRequest.ts new file mode 100644 index 000000000..46ff53c61 --- /dev/null +++ b/lib/sessions/chats/validateMarkChatReadRequest.ts @@ -0,0 +1,66 @@ +import { NextRequest, NextResponse } from "next/server"; +import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; +import { validateAuthContext } from "@/lib/auth/validateAuthContext"; +import type { AuthContext } from "@/lib/auth/validateAuthContext"; +import { selectSessions } from "@/lib/supabase/sessions/selectSessions"; +import { selectChats } from "@/lib/supabase/chats/selectChats"; +import type { Tables } from "@/types/database.types"; + +export interface ValidatedMarkChatReadRequest { + auth: AuthContext; + session: Tables<"sessions">; + chat: Tables<"chats">; +} + +/** + * Validates `POST /api/sessions/{sessionId}/chats/{chatId}/read`: + * 1. Authenticates the caller + * 2. Loads the session and chat + * 3. Confirms the account owns the session and the chat belongs to it + * + * @param request - The incoming request. + * @param sessionId - The parent session id. + * @param chatId - The chat id to mark as read. + * @returns A NextResponse on failure, or the validated auth + session + chat. + */ +export async function validateMarkChatReadRequest( + request: NextRequest, + sessionId: string, + chatId: string, +): Promise { + const auth = await validateAuthContext(request); + if (auth instanceof NextResponse) { + return auth; + } + + const [sessionRows, chatRows] = await Promise.all([ + selectSessions({ id: sessionId }), + selectChats({ id: chatId }), + ]); + + const session = sessionRows[0] ?? null; + const chat = chatRows[0] ?? null; + + if (!session) { + return NextResponse.json( + { status: "error", error: "Session not found" }, + { status: 404, headers: getCorsHeaders() }, + ); + } + + if (session.account_id !== auth.accountId) { + return NextResponse.json( + { status: "error", error: "Forbidden" }, + { status: 403, headers: getCorsHeaders() }, + ); + } + + if (!chat || chat.session_id !== sessionId) { + return NextResponse.json( + { status: "error", error: "Chat not found" }, + { status: 404, headers: getCorsHeaders() }, + ); + } + + return { auth, session, chat }; +} diff --git a/lib/sessions/createSessionHandler.ts b/lib/sessions/createSessionHandler.ts index 87e81066b..e48020537 100644 --- a/lib/sessions/createSessionHandler.ts +++ b/lib/sessions/createSessionHandler.ts @@ -58,6 +58,7 @@ export async function createSessionHandler(request: NextRequest): Promise) { return { id: row.id, userId: row.account_id, + artistId: row.artist_id, title: row.title, status: row.status, repoOwner: row.repo_owner, diff --git a/lib/sessions/validateCreateSessionBody.ts b/lib/sessions/validateCreateSessionBody.ts index d9aa07175..0b6059b68 100644 --- a/lib/sessions/validateCreateSessionBody.ts +++ b/lib/sessions/validateCreateSessionBody.ts @@ -21,6 +21,7 @@ import type { AuthContext } from "@/lib/auth/validateAuthContext"; export const createSessionBodySchema = z.object({ title: z.string().optional(), organizationId: z.string().uuid("organizationId must be a valid UUID").optional(), + artistId: z.string().uuid("artistId must be a valid UUID").optional(), }); export type CreateSessionBody = z.infer; diff --git a/lib/supabase/chat_messages/__tests__/upsertChatMessages.test.ts b/lib/supabase/chat_messages/__tests__/upsertChatMessages.test.ts new file mode 100644 index 000000000..8c995b26a --- /dev/null +++ b/lib/supabase/chat_messages/__tests__/upsertChatMessages.test.ts @@ -0,0 +1,59 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { upsertChatMessages } from "@/lib/supabase/chat_messages/upsertChatMessages"; +import { upsertChatMessage } from "@/lib/supabase/chat_messages/upsertChatMessage"; + +vi.mock("@/lib/supabase/chat_messages/upsertChatMessage", () => ({ + upsertChatMessage: vi.fn(), +})); + +const rows: any[] = [ + { id: "m1", chat_id: "c1", role: "user", parts: [], created_at: "t" }, + { id: "m2", chat_id: "c1", role: "assistant", parts: [], created_at: "t" }, +]; + +beforeEach(() => { + vi.clearAllMocks(); +}); + +describe("upsertChatMessages", () => { + it("returns 0 without calling the helper for an empty batch", async () => { + const n = await upsertChatMessages([]); + expect(n).toBe(0); + expect(upsertChatMessage).not.toHaveBeenCalled(); + }); + + it("delegates each row write-once to the single-row helper", async () => { + vi.mocked(upsertChatMessage).mockResolvedValue({ + ok: true, + row: null, + isDuplicate: false, + } as any); + + const n = await upsertChatMessages(rows); + + expect(n).toBe(2); + expect(upsertChatMessage).toHaveBeenCalledTimes(2); + expect(upsertChatMessage).toHaveBeenCalledWith(rows[0], { update: false }); + }); + + it("counts duplicates (write-once skips) as success", async () => { + vi.mocked(upsertChatMessage).mockResolvedValue({ + ok: true, + row: null, + isDuplicate: true, + } as any); + + const n = await upsertChatMessages(rows); + + expect(n).toBe(2); + }); + + it("logs and keeps going on a row failure, then throws at the end", async () => { + vi.mocked(upsertChatMessage) + .mockResolvedValueOnce({ ok: true, row: null, isDuplicate: false } as any) + .mockResolvedValueOnce({ ok: false, error: "row boom" } as any); + + await expect(upsertChatMessages(rows)).rejects.toThrow("Failed to upsert 1 of 2 chat_messages"); + expect(upsertChatMessage).toHaveBeenCalledTimes(2); + }); +}); diff --git a/lib/supabase/chat_messages/upsertChatMessages.ts b/lib/supabase/chat_messages/upsertChatMessages.ts new file mode 100644 index 000000000..9004fd022 --- /dev/null +++ b/lib/supabase/chat_messages/upsertChatMessages.ts @@ -0,0 +1,29 @@ +import type { TablesInsert } from "@/types/database.types"; +import { upsertChatMessage } from "./upsertChatMessage"; + +/** + * Upsert many chat messages by delegating each row to the single-row + * `upsertChatMessage` helper — no query is defined here. Write-once + * (`update: false` → DO NOTHING on conflict), so re-runs never overwrite + * already-migrated messages. A single bad row doesn't block the rest; + * throws if any row ultimately fails. Returns the number of rows attempted + * (duplicates count as success). + * + * Used by the Phase 2 backfill. + */ +export async function upsertChatMessages(rows: TablesInsert<"chat_messages">[]): Promise { + let succeeded = 0; + for (const row of rows) { + const result = await upsertChatMessage(row, { update: false }); + if ("error" in result) { + console.error(` ❌ Skipping message ${row.id}:`, result.error); + } else { + succeeded++; + } + } + + if (succeeded !== rows.length) { + throw new Error(`Failed to upsert ${rows.length - succeeded} of ${rows.length} chat_messages`); + } + return succeeded; +} diff --git a/lib/supabase/chat_reads/upsertChatRead.ts b/lib/supabase/chat_reads/upsertChatRead.ts new file mode 100644 index 000000000..a36121ab7 --- /dev/null +++ b/lib/supabase/chat_reads/upsertChatRead.ts @@ -0,0 +1,38 @@ +import supabase from "@/lib/supabase/serverClient"; +import type { Tables } from "@/types/database.types"; + +/** + * Upserts a `chat_reads` row for the given account and chat, setting + * `last_read_at` to the current time. Idempotent — safe to call repeatedly. + * + * @param accountId - The owning account id. + * @param chatId - The chat id to mark as read. + * @returns The upserted row, or `null` if the write failed. + */ +export async function upsertChatRead( + accountId: string, + chatId: string, +): Promise | null> { + const now = new Date().toISOString(); + + const { data, error } = await supabase + .from("chat_reads") + .upsert( + { + account_id: accountId, + chat_id: chatId, + last_read_at: now, + updated_at: now, + }, + { onConflict: "account_id,chat_id" }, + ) + .select() + .maybeSingle(); + + if (error) { + console.error("[upsertChatRead] error:", error); + return null; + } + + return data; +} diff --git a/lib/supabase/chats/__tests__/selectChatsWithSessions.test.ts b/lib/supabase/chats/__tests__/selectChatsWithSessions.test.ts new file mode 100644 index 000000000..0c88d4115 --- /dev/null +++ b/lib/supabase/chats/__tests__/selectChatsWithSessions.test.ts @@ -0,0 +1,134 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { selectChatsWithSessions } from "@/lib/supabase/chats/selectChatsWithSessions"; + +const fromMock = vi.fn(); +const selectMock = vi.fn(); +const neqMock = 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().neq().in().eq().order() -> resolves to { data, error } + const builder = { + select: selectMock, + neq: neqMock, + in: inMock, + eq: eqMock, + order: orderMock, + }; + fromMock.mockReturnValue(builder); + selectMock.mockReturnValue(builder); + neqMock.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(String(selectArg)).toContain("status"); + + expect(neqMock).toHaveBeenCalledWith("session.status", "archived"); + 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("always excludes archived sessions, even with no other filters", async () => { + await selectChatsWithSessions({}); + + expect(neqMock).toHaveBeenCalledWith("session.status", "archived"); + }); + + 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..ee3d204db --- /dev/null +++ b/lib/supabase/chats/selectChatsWithSessions.ts @@ -0,0 +1,60 @@ +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, status ) +` 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`. Chats whose session is archived + * (`sessions.status === "archived"`) are excluded — archive is the + * soft-delete path, and archived sessions should not surface in chat + * listings. Results are 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).neq("session.status", "archived"); + 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 ?? []; +} diff --git a/lib/supabase/memories/selectMemories.ts b/lib/supabase/memories/selectMemories.ts index f1168162f..ed9ffc92d 100644 --- a/lib/supabase/memories/selectMemories.ts +++ b/lib/supabase/memories/selectMemories.ts @@ -17,11 +17,16 @@ export default async function selectMemories( ascending?: boolean; limit?: number; memoryId?: string; + /** Zero-based inclusive row range for a single page (PostgREST `.range`). + * When set, also adds `id` as a stable secondary sort so paginated reads + * don't skip/duplicate rows at page boundaries. */ + range?: { from: number; to: number }; }, ): Promise[] | null> { const ascending = options?.ascending ?? false; const limit = options?.limit; const memoryId = options?.memoryId; + const range = options?.range; let query = supabase .from("memories") @@ -37,6 +42,10 @@ export default async function selectMemories( query = query.limit(limit); } + if (range) { + query = query.order("id", { ascending }).range(range.from, range.to); + } + const { data, error } = await query; if (error) { diff --git a/lib/supabase/rooms/selectRooms.ts b/lib/supabase/rooms/selectRooms.ts index 33e1debef..b6e0b389e 100644 --- a/lib/supabase/rooms/selectRooms.ts +++ b/lib/supabase/rooms/selectRooms.ts @@ -8,6 +8,10 @@ export interface SelectRoomsParams { account_ids?: string[]; /** Filter by artist ID */ artist_id?: string; + /** Zero-based inclusive row range for a single page (PostgREST `.range`). + * When set, also adds `id` as a stable secondary sort so paginated reads + * don't skip/duplicate rows at page boundaries. */ + range?: { from: number; to: number }; } /** @@ -21,7 +25,7 @@ export interface SelectRoomsParams { * @returns Array of rooms or null if error */ export async function selectRooms(params: SelectRoomsParams = {}): Promise { - const { account_ids, artist_id } = params; + const { account_ids, artist_id, range } = params; // If account_ids is an empty array, return empty (no accounts to look up) if (account_ids !== undefined && account_ids.length === 0) { @@ -41,6 +45,10 @@ export async function selectRooms(params: SelectRoomsParams = {}): Promise types/database.types.ts", + "backfill:rooms-to-sessions": "npx tsx scripts/backfillRoomsToSessions.ts", "eval": "braintrust eval --external-packages playwright playwright-core chromium-bidi @browserbasehq/stagehand @composio/core @composio/vercel" }, "dependencies": { @@ -57,6 +58,7 @@ "resend": "^6.6.0", "sharp": "^0.34.5", "stripe": "^17.4.0", + "uuid": "^14.0.0", "viem": "^2.21.26", "workflow": "^4.2.4", "x402-fetch": "^0.7.3", @@ -70,6 +72,7 @@ "@types/node": "^20", "@types/react": "^19", "@types/react-dom": "^19", + "@types/uuid": "^11.0.0", "@typescript-eslint/eslint-plugin": "^8.29.1", "@typescript-eslint/parser": "^8.29.1", "eslint": "^9.24.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 7b4a331ad..e953e3bcb 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -128,6 +128,9 @@ importers: stripe: specifier: ^17.4.0 version: 17.7.0 + uuid: + specifier: ^14.0.0 + version: 14.0.0 viem: specifier: ^2.21.26 version: 2.40.3(bufferutil@4.0.9)(typescript@5.9.3)(utf-8-validate@5.0.10)(zod@4.1.13) @@ -162,6 +165,9 @@ importers: '@types/react-dom': specifier: ^19 version: 19.2.3(@types/react@19.2.7) + '@types/uuid': + specifier: ^11.0.0 + version: 11.0.0 '@typescript-eslint/eslint-plugin': specifier: ^8.29.1 version: 8.48.0(@typescript-eslint/parser@8.48.0(eslint@9.39.1(jiti@1.21.7))(typescript@5.9.3))(eslint@9.39.1(jiti@1.21.7))(typescript@5.9.3) @@ -3018,6 +3024,10 @@ packages: '@types/unist@3.0.3': resolution: {integrity: sha512-ko/gIFJRv177XgZsZcBwnqJN5x/Gien8qNOn0D5bQU/zAzVf9Zt3BlcUiLqhV9y4ARk0GbT3tnUiPNgnTXzc/Q==} + '@types/uuid@11.0.0': + resolution: {integrity: sha512-HVyk8nj2m+jcFRNazzqyVKiZezyhDKrGUA3jlEcg/nZ6Ms+qHwocba1Y/AaVaznJTAM9xpdFSh+ptbNrhOGvZA==} + deprecated: This is a stub types definition. uuid provides its own type definitions, so you do not need this installed. + '@types/uuid@8.3.4': resolution: {integrity: sha512-c/I8ZRb51j+pYGAu5CrFMRxqZ2ke4y2grEBO5AUjgSkSk+qT2Ea+OdWElz/OiMf5MNpn2b17kuVBwZLQJXzihw==} @@ -7667,6 +7677,10 @@ packages: resolution: {integrity: sha512-XQegIaBTVUjSHliKqcnFqYypAd4S+WCYt5NIeRs6w/UAry7z8Y9j5ZwRRL4kzq9U3sD6v+85er9FvkEaBpji2w==} hasBin: true + uuid@14.0.0: + resolution: {integrity: sha512-Qo+uWgilfSmAhXCMav1uYFynlQO7fMFiMVZsQqZRMIXp0O7rR7qjkj+cPvBHLgBqi960QCoo/PH2/6ZtVqKvrg==} + hasBin: true + uuid@3.4.0: resolution: {integrity: sha512-HjSDRw6gZE5JMggctHBcjVak08+KEVhSIiDzFnT9S9aegmp85S/bReBVTb4QTFaRNptJ9kuYaNhnbNEOkbKb/A==} deprecated: uuid@10 and below is no longer supported. For ESM codebases, update to uuid@latest. For CommonJS codebases, use uuid@11 (but be aware this version will likely be deprecated in 2028). @@ -11709,6 +11723,10 @@ snapshots: '@types/unist@3.0.3': {} + '@types/uuid@11.0.0': + dependencies: + uuid: 14.0.0 + '@types/uuid@8.3.4': {} '@types/ws@7.4.7': @@ -17808,6 +17826,8 @@ snapshots: uuid@13.0.0: {} + uuid@14.0.0: {} + uuid@3.4.0: {} uuid@8.3.2: {} diff --git a/scripts/backfill/__tests__/migrateRoom.test.ts b/scripts/backfill/__tests__/migrateRoom.test.ts new file mode 100644 index 000000000..5da4d8c9e --- /dev/null +++ b/scripts/backfill/__tests__/migrateRoom.test.ts @@ -0,0 +1,129 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +import { migrateRoom } from "@/scripts/backfill/migrateRoom"; +import { selectSessions } from "@/lib/supabase/sessions/selectSessions"; +import { insertSession } from "@/lib/supabase/sessions/insertSession"; +import { selectChats } from "@/lib/supabase/chats/selectChats"; +import { insertChat } from "@/lib/supabase/chats/insertChat"; +import selectMemories from "@/lib/supabase/memories/selectMemories"; +import { upsertChatMessages } from "@/lib/supabase/chat_messages/upsertChatMessages"; + +vi.mock("@/lib/supabase/sessions/selectSessions", () => ({ selectSessions: vi.fn() })); +vi.mock("@/lib/supabase/sessions/insertSession", () => ({ insertSession: vi.fn() })); +vi.mock("@/lib/supabase/chats/selectChats", () => ({ selectChats: vi.fn() })); +vi.mock("@/lib/supabase/chats/insertChat", () => ({ insertChat: vi.fn() })); +vi.mock("@/lib/supabase/memories/selectMemories", () => ({ default: vi.fn() })); +vi.mock("@/lib/supabase/chat_messages/upsertChatMessages", () => ({ + upsertChatMessages: vi.fn(), +})); + +const room: any = { + id: "room-1", + account_id: "acc-1", + artist_id: null, + topic: "Hello", + updated_at: "2026-01-01T00:00:00Z", +}; + +beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(selectSessions).mockResolvedValue([]); + vi.mocked(selectChats).mockResolvedValue([]); + vi.mocked(selectMemories).mockResolvedValue([ + { + id: "m1", + room_id: "room-1", + content: { role: "user", parts: [{ type: "text", text: "hi" }] }, + updated_at: "2026-01-01T00:00:00Z", + } as any, + { + id: "m2", + room_id: "room-1", + content: { role: "assistant", parts: null }, // malformed — null parts + updated_at: "2026-01-01T00:00:01Z", + } as any, + ]); + + vi.mocked(insertSession).mockResolvedValue({ id: "s1" } as any); + + vi.mocked(insertChat).mockResolvedValue({ id: "room-1" } as any); + vi.mocked(upsertChatMessages).mockResolvedValue(1); +}); + +describe("migrateRoom", () => { + it("performs NO writes in dry-run mode but still reads + reports stats", async () => { + const stats = await migrateRoom(room, { dryRun: true }); + + expect(insertSession).not.toHaveBeenCalled(); + expect(insertChat).not.toHaveBeenCalled(); + expect(upsertChatMessages).not.toHaveBeenCalled(); + + expect(selectSessions).toHaveBeenCalled(); + expect(selectChats).toHaveBeenCalled(); + expect(selectMemories).toHaveBeenCalledWith( + "room-1", + expect.objectContaining({ range: { from: 0, to: 999 } }), + ); + + expect(stats).toMatchObject({ + status: "migrated", + messagesWritten: 1, // only the well-formed memory + messagesMalformed: 1, // the null-parts memory + memoryCount: 2, + }); + }); + + it("writes session, chat, and only well-formed messages in a real run", async () => { + const stats = await migrateRoom(room, { dryRun: false }); + + expect(insertSession).toHaveBeenCalledTimes(1); + expect(insertSession).toHaveBeenCalledWith( + expect.objectContaining({ + account_id: "acc-1", + artist_id: null, + }), + ); + expect(insertChat).toHaveBeenCalledTimes(1); + // one batch call containing only the well-formed message + expect(upsertChatMessages).toHaveBeenCalledTimes(1); + // `parts` stores the full UIMessage (matching the workflow's native + // persist path), not the bare parts array. + expect(upsertChatMessages).toHaveBeenCalledWith([ + expect.objectContaining({ + id: "m1", + chat_id: "room-1", + role: "user", + parts: { id: "m1", role: "user", parts: [{ type: "text", text: "hi" }] }, + }), + ]); + expect(stats.messagesWritten).toBe(1); + expect(stats.messagesMalformed).toBe(1); + }); + + it("skips rooms with no account_id", async () => { + const stats = await migrateRoom({ ...room, account_id: null }, { dryRun: false }); + + expect(stats.status).toBe("skipped"); + expect(insertSession).not.toHaveBeenCalled(); + expect(insertChat).not.toHaveBeenCalled(); + }); + + it("passes room.artist_id into insertSession for straggler rooms", async () => { + await migrateRoom({ ...room, artist_id: "artist-1" }, { dryRun: false }); + + expect(insertSession).toHaveBeenCalledWith(expect.objectContaining({ artist_id: "artist-1" })); + }); + + it("skips session/chat inserts when they already exist (idempotent re-run)", async () => { + vi.mocked(selectSessions).mockResolvedValue([{ id: "s1" } as any]); + + vi.mocked(selectChats).mockResolvedValue([{ id: "room-1" } as any]); + + const stats = await migrateRoom(room, { dryRun: false }); + + expect(insertSession).not.toHaveBeenCalled(); + expect(insertChat).not.toHaveBeenCalled(); + expect(stats.sessionExisted).toBe(true); + expect(stats.chatExisted).toBe(true); + }); +}); diff --git a/scripts/backfill/__tests__/paginate.test.ts b/scripts/backfill/__tests__/paginate.test.ts new file mode 100644 index 000000000..691a31ec5 --- /dev/null +++ b/scripts/backfill/__tests__/paginate.test.ts @@ -0,0 +1,38 @@ +import { describe, it, expect, vi } from "vitest"; +import { paginate } from "@/scripts/backfill/paginate"; + +describe("paginate", () => { + it("stops after a single page when it is not full (< PAGE_SIZE)", async () => { + const fetchPage = vi.fn().mockResolvedValueOnce([1, 2, 3]); + + const rows = await paginate(fetchPage); + + expect(rows).toEqual([1, 2, 3]); + expect(fetchPage).toHaveBeenCalledTimes(1); + expect(fetchPage).toHaveBeenCalledWith(0, 999); + }); + + it("keeps paging while pages are full, advancing the range each time", async () => { + const full = Array.from({ length: 1000 }, (_, i) => i); + const fetchPage = vi + .fn() + .mockResolvedValueOnce(full) // page 1 — full, keep going + .mockResolvedValueOnce([1000, 1001]); // page 2 — short, stop + + const rows = await paginate(fetchPage); + + expect(rows).toHaveLength(1002); + expect(fetchPage).toHaveBeenCalledTimes(2); + expect(fetchPage).toHaveBeenNthCalledWith(1, 0, 999); + expect(fetchPage).toHaveBeenNthCalledWith(2, 1000, 1999); + }); + + it("returns an empty array and queries once when the first page is empty", async () => { + const fetchPage = vi.fn().mockResolvedValueOnce([]); + + const rows = await paginate(fetchPage); + + expect(rows).toEqual([]); + expect(fetchPage).toHaveBeenCalledTimes(1); + }); +}); diff --git a/scripts/backfill/__tests__/selectAllRooms.test.ts b/scripts/backfill/__tests__/selectAllRooms.test.ts new file mode 100644 index 000000000..ed999e29b --- /dev/null +++ b/scripts/backfill/__tests__/selectAllRooms.test.ts @@ -0,0 +1,43 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { selectAllRooms } from "@/scripts/backfill/selectAllRooms"; +import { selectRooms } from "@/lib/supabase/rooms/selectRooms"; + +vi.mock("@/lib/supabase/rooms/selectRooms", () => ({ selectRooms: vi.fn() })); + +beforeEach(() => { + vi.clearAllMocks(); +}); + +describe("selectAllRooms", () => { + it("returns a single short page without a second query", async () => { + vi.mocked(selectRooms).mockResolvedValueOnce([{ id: "r1" }, { id: "r2" }] as any); + + const rooms = await selectAllRooms(); + + expect(rooms).toHaveLength(2); + expect(selectRooms).toHaveBeenCalledTimes(1); + expect(selectRooms).toHaveBeenCalledWith({ range: { from: 0, to: 999 } }); + }); + + it("pages through the PostgREST 1,000-row cap until a short page", async () => { + const fullPage = Array.from({ length: 1000 }, (_, i) => ({ id: `r${i}` })); + vi.mocked(selectRooms) + .mockResolvedValueOnce(fullPage as any) + .mockResolvedValueOnce([{ id: "r1000" }] as any); + + const rooms = await selectAllRooms(); + + expect(rooms).toHaveLength(1001); + expect(selectRooms).toHaveBeenCalledTimes(2); + expect(selectRooms).toHaveBeenNthCalledWith(2, { range: { from: 1000, to: 1999 } }); + }); + + it("treats a null result as an empty page", async () => { + vi.mocked(selectRooms).mockResolvedValueOnce(null as any); + + const rooms = await selectAllRooms(); + + expect(rooms).toEqual([]); + expect(selectRooms).toHaveBeenCalledTimes(1); + }); +}); diff --git a/scripts/backfill/migrateRoom.ts b/scripts/backfill/migrateRoom.ts new file mode 100644 index 000000000..66fa13d8e --- /dev/null +++ b/scripts/backfill/migrateRoom.ts @@ -0,0 +1,145 @@ +import { v5 as uuidv5 } from "uuid"; +import { selectSessions } from "@/lib/supabase/sessions/selectSessions"; +import { insertSession } from "@/lib/supabase/sessions/insertSession"; +import { selectChats } from "@/lib/supabase/chats/selectChats"; +import { insertChat } from "@/lib/supabase/chats/insertChat"; +import selectMemories from "@/lib/supabase/memories/selectMemories"; +import { upsertChatMessages } from "@/lib/supabase/chat_messages/upsertChatMessages"; +import type { Json, Tables, TablesInsert } from "@/types/database.types"; +import { paginate } from "./paginate"; + +// Fixed namespace so uuidv5(room.id) yields the same sessionId every run. +// A re-run after partial failure then finds the existing session via the +// guard below instead of minting an orphan. +const SESSION_NAMESPACE = "f47ac10b-58cc-4372-a567-0e02b2c3d479"; + +/** Rooms at/above this many memories are flagged in the run summary. */ +export const OVERSIZED_THRESHOLD = 1000; + +export interface RoomStats { + /** "skipped" = no account_id; "migrated" = processed (real or dry). */ + status: "migrated" | "skipped"; + /** Session/chat already existed → insert was a no-op (idempotent re-run). */ + sessionExisted: boolean; + chatExisted: boolean; + /** Messages written (real run) or that would be written (dry run). */ + messagesWritten: number; + /** Memories whose content lacks a usable { role, parts } shape. */ + messagesMalformed: number; + /** Total memories read for the room. */ + memoryCount: number; +} + +interface MigrateOptions { + dryRun?: boolean; +} + +/** Legacy memories store content as { role, parts, content }. */ +function isWellFormedContent(content: unknown): content is { role: string; parts: Json } { + if (!content || typeof content !== "object") return false; + const candidate = content as { role?: unknown; parts?: unknown }; + return typeof candidate.role === "string" && candidate.parts != null; +} + +async function migrateMessages( + roomId: string, + dryRun: boolean, +): Promise<{ written: number; malformed: number; memoryCount: number }> { + const memories = await paginate((from, to) => + selectMemories(roomId, { ascending: true, range: { from, to } }).then(m => m ?? []), + ); + const rows: TablesInsert<"chat_messages">[] = []; + let malformed = 0; + + for (const memory of memories) { + const content = memory.content; + if (!isWellFormedContent(content)) { + malformed++; + continue; + } + // The workflow persists the FULL UIMessage in the `parts` column + // (see lib/chat/persistLatestUserMessage / persistAssistantMessage), + // and the read path (getSessionChatHandler) returns `parts` verbatim + // expecting a UIMessage. Store the same shape so migrated history + // deserializes identically to natively-written chats. + rows.push({ + id: memory.id, + chat_id: roomId, + role: content.role, + parts: { id: memory.id, role: content.role, parts: content.parts }, + created_at: memory.updated_at, + }); + } + + // Write-once per message, so re-runs never overwrite already-migrated rows. + const written = dryRun ? rows.length : await upsertChatMessages(rows); + + return { written, malformed, memoryCount: memories.length }; +} + +export async function migrateRoom( + room: Tables<"rooms">, + { dryRun = false }: MigrateOptions = {}, +): Promise { + if (!room.account_id) { + console.warn(`⚠️ Skipping room ${room.id} — no account_id`); + return { + status: "skipped", + sessionExisted: false, + chatExisted: false, + messagesWritten: 0, + messagesMalformed: 0, + memoryCount: 0, + }; + } + + const sessionId = uuidv5(room.id, SESSION_NAMESPACE); + const title = room.topic ?? "Untitled"; + + // insertSession / insertChat are plain inserts; guard with a select so + // re-runs are idempotent and don't error on the existing primary key. + const existingSession = await selectSessions({ id: sessionId }); + const sessionExisted = Boolean(existingSession?.length); + if (!sessionExisted && !dryRun) { + const session = await insertSession({ + id: sessionId, + account_id: room.account_id, + artist_id: room.artist_id, + title, + created_at: room.updated_at, + updated_at: room.updated_at, + }); + if (!session) throw new Error(`Failed to insert session for room ${room.id}`); + } + + // Preserve room.id as chat.id so /chat/[roomId] URLs keep working. + const existingChat = await selectChats({ id: room.id }); + const chatExisted = existingChat.length > 0; + if (!chatExisted && !dryRun) { + const chat = await insertChat({ + id: room.id, + session_id: sessionId, + title, + created_at: room.updated_at, + updated_at: room.updated_at, + }); + if (!chat) throw new Error(`Failed to insert chat for room ${room.id}`); + } + + const { written, malformed, memoryCount } = await migrateMessages(room.id, dryRun); + + const verb = dryRun ? "Would migrate" : "Migrated"; + const malformedNote = malformed ? `, ${malformed} malformed skipped` : ""; + console.log( + `✅ ${verb} room ${room.id} → session ${sessionId} (${written} messages${malformedNote})`, + ); + + return { + status: "migrated", + sessionExisted, + chatExisted, + messagesWritten: written, + messagesMalformed: malformed, + memoryCount, + }; +} diff --git a/scripts/backfill/paginate.ts b/scripts/backfill/paginate.ts new file mode 100644 index 000000000..5da32159b --- /dev/null +++ b/scripts/backfill/paginate.ts @@ -0,0 +1,19 @@ +const PAGE_SIZE = 1000; + +/** + * Collects every row from a paged query by calling `fetchPage(from, to)` until + * it returns a short page (< PAGE_SIZE). Keeps the pagination loop OUT of the + * thin `lib/supabase` query helpers — they stay single-query; this composes + * them. Used by the Phase 2 backfill to read past the PostgREST 1,000-row cap. + */ +export async function paginate( + fetchPage: (from: number, to: number) => Promise, +): Promise { + const rows: T[] = []; + for (let from = 0; ; from += PAGE_SIZE) { + const page = await fetchPage(from, from + PAGE_SIZE - 1); + rows.push(...page); + if (page.length < PAGE_SIZE) break; + } + return rows; +} diff --git a/scripts/backfill/selectAllRooms.ts b/scripts/backfill/selectAllRooms.ts new file mode 100644 index 000000000..0e8a74e7e --- /dev/null +++ b/scripts/backfill/selectAllRooms.ts @@ -0,0 +1,12 @@ +import { selectRooms } from "@/lib/supabase/rooms/selectRooms"; +import type { Tables } from "@/types/database.types"; +import { paginate } from "./paginate"; + +/** + * Every room, read past the PostgREST 1,000-row cap. The for-loop lives here + * (outside `lib/supabase`); `selectRooms` stays a thin single-query helper — + * `paginate` just calls it one page at a time. + */ +export async function selectAllRooms(): Promise[]> { + return paginate((from, to) => selectRooms({ range: { from, to } }).then(rows => rows ?? [])); +} diff --git a/scripts/backfillRoomsToSessions.ts b/scripts/backfillRoomsToSessions.ts new file mode 100644 index 000000000..b1dea52b5 --- /dev/null +++ b/scripts/backfillRoomsToSessions.ts @@ -0,0 +1,112 @@ +/** + * Phase 2 backfill: migrate rooms + memories → sessions + chats + chat_messages. + * + * Real run: + * SUPABASE_URL=… SUPABASE_KEY=… pnpm backfill:rooms-to-sessions + * + * Dry run (reads only, no writes — prints what would happen): + * SUPABASE_URL=… SUPABASE_KEY=… DRY_RUN=1 pnpm backfill:rooms-to-sessions + * (or pass `--dry-run`) + * + * Idempotent: deterministic session ids + existence guards + write-once + * message upserts make re-runs safe after a partial failure. + */ + +import selectRoom from "@/lib/supabase/rooms/selectRoom"; +import { migrateRoom, OVERSIZED_THRESHOLD, type RoomStats } from "./backfill/migrateRoom"; +import { selectAllRooms } from "./backfill/selectAllRooms"; +import type { Tables } from "@/types/database.types"; + +const dryRun = process.argv.includes("--dry-run") || process.env.DRY_RUN === "1"; + +// `--limit=N` processes only the first N rooms; `--room=` processes a +// single room. Both are for validating the script's code path (or +// re-migrating one room) before/without an unbounded run. +const limitFlag = process.argv.find(a => a.startsWith("--limit=")); +const limit = limitFlag ? Number.parseInt(limitFlag.split("=")[1], 10) : undefined; +const roomFlag = process.argv.find(a => a.startsWith("--room=")); +const roomId = roomFlag ? roomFlag.split("=")[1] : undefined; + +// Rooms are processed in concurrent batches of this size. Each room's +// session/chat/message ids are independent (distinct rooms never touch the +// same row), so cross-room concurrency is safe and idempotency holds. +const concurrencyFlag = process.argv.find(a => a.startsWith("--concurrency=")); +const CONCURRENCY = concurrencyFlag + ? Math.max(1, Number.parseInt(concurrencyFlag.split("=")[1], 10)) + : 20; + +async function main() { + console.log( + `🚀 Phase 2 backfill: rooms → sessions/chats/chat_messages${dryRun ? " [DRY RUN — no writes]" : ""}${roomId ? ` [room ${roomId}]` : limit ? ` [limit ${limit}]` : ""}\n`, + ); + + let rooms: Tables<"rooms">[]; + if (roomId) { + const room = await selectRoom(roomId); + rooms = room ? [room] : []; + console.log(room ? `Targeting room ${roomId}\n` : `Room ${roomId} not found\n`); + } else { + const allRooms = await selectAllRooms(); + rooms = limit ? allRooms.slice(0, limit) : allRooms; + console.log( + `Found ${allRooms.length} rooms${limit ? `, processing first ${rooms.length}` : ""}\n`, + ); + } + + let migrated = 0; + let skipped = 0; + let failed = 0; + let sessionsNew = 0; + let sessionsExisting = 0; + let chatsNew = 0; + let chatsExisting = 0; + let messages = 0; + let malformed = 0; + let oversized = 0; + + const tally = (stats: RoomStats) => { + if (stats.status === "skipped") { + skipped++; + return; + } + migrated++; + if (stats.sessionExisted) sessionsExisting++; + else sessionsNew++; + if (stats.chatExisted) chatsExisting++; + else chatsNew++; + messages += stats.messagesWritten; + malformed += stats.messagesMalformed; + if (stats.memoryCount >= OVERSIZED_THRESHOLD) oversized++; + }; + + // Process rooms in concurrent batches. `allSettled` isolates failures so + // one bad room doesn't sink the rest of its batch. + for (let i = 0; i < rooms.length; i += CONCURRENCY) { + const batch = rooms.slice(i, i + CONCURRENCY); + const results = await Promise.allSettled(batch.map(room => migrateRoom(room, { dryRun }))); + results.forEach((result, j) => { + if (result.status === "fulfilled") { + tally(result.value); + } else { + console.error(`❌ Failed to migrate room ${batch[j].id}:`, result.reason); + failed++; + } + }); + } + + console.log(`\n📊 ${dryRun ? "DRY RUN — no writes performed" : "Done"}`); + console.log( + `Rooms: ${migrated} ${dryRun ? "would migrate" : "migrated"}, ${skipped} skipped (no account_id), ${failed} failed`, + ); + console.log(`Sessions: ${sessionsNew} new, ${sessionsExisting} already exist`); + console.log(`Chats: ${chatsNew} new, ${chatsExisting} already exist`); + console.log( + `Messages: ${messages} ${dryRun ? "would write" : "written"}` + + (malformed ? ` | ${malformed} malformed (null parts) skipped` : "") + + (oversized ? ` | ${oversized} room(s) ≥${OVERSIZED_THRESHOLD} memories` : ""), + ); + + if (failed > 0) process.exit(1); +} + +main(); diff --git a/types/database.types.ts b/types/database.types.ts index cc7d03e1e..191472132 100644 --- a/types/database.types.ts +++ b/types/database.types.ts @@ -2878,6 +2878,7 @@ export type Database = { sessions: { Row: { account_id: string; + artist_id: string | null; branch: string | null; cached_diff: Json | null; cached_diff_updated_at: string | null; @@ -2907,6 +2908,7 @@ export type Database = { }; Insert: { account_id: string; + artist_id?: string | null; branch?: string | null; cached_diff?: Json | null; cached_diff_updated_at?: string | null; @@ -2936,6 +2938,7 @@ export type Database = { }; Update: { account_id?: string; + artist_id?: string | null; branch?: string | null; cached_diff?: Json | null; cached_diff_updated_at?: string | null; @@ -2971,6 +2974,13 @@ export type Database = { referencedRelation: "accounts"; referencedColumns: ["id"]; }, + { + foreignKeyName: "sessions_artist_id_fkey"; + columns: ["artist_id"]; + isOneToOne: false; + referencedRelation: "accounts"; + referencedColumns: ["id"]; + }, ]; }; social_fans: {