fix(catalog): link the caller's account on catalog re-claim (chat#1867)

lib/catalog/__tests__/createCatalogHandler.test.ts

@@ -8,7 +8,7 @@ import { validateAuthContext } from "@/lib/auth/validateAuthContext";
 import { selectPlaycountSnapshots } from "@/lib/supabase/playcount_snapshots/selectPlaycountSnapshots";
 import { selectCatalogById } from "@/lib/supabase/catalogs/selectCatalogById";
 import { insertCatalog } from "@/lib/supabase/catalogs/insertCatalog";
-import { insertAccountCatalog } from "@/lib/supabase/account_catalogs/insertAccountCatalog";
+import { upsertAccountCatalog } from "@/lib/supabase/account_catalogs/upsertAccountCatalog";
 import { selectSongMeasurements } from "@/lib/supabase/song_measurements/selectSongMeasurements";
 import { attachCanonicalArtistToAccount } from "../attachCanonicalArtistToAccount";
 
@@ -23,8 +23,8 @@ vi.mock("@/lib/supabase/playcount_snapshots/selectPlaycountSnapshots", () => ({
 }));
 vi.mock("@/lib/supabase/catalogs/selectCatalogById", () => ({ selectCatalogById: vi.fn() }));
 vi.mock("@/lib/supabase/catalogs/insertCatalog", () => ({ insertCatalog: vi.fn() }));
-vi.mock("@/lib/supabase/account_catalogs/insertAccountCatalog", () => ({
-  insertAccountCatalog: vi.fn(),
+vi.mock("@/lib/supabase/account_catalogs/upsertAccountCatalog", () => ({
+  upsertAccountCatalog: vi.fn(),
 }));
 vi.mock("@/lib/supabase/song_measurements/selectSongMeasurements", () => ({
   selectSongMeasurements: vi.fn(),
@@ -84,7 +84,7 @@ describe("createCatalogHandler", () => {
 
     expect(res.status).toBe(200);
     expect(insertCatalog).toHaveBeenCalledWith("Bad Bunny — Catalog");
-    expect(insertAccountCatalog).toHaveBeenCalledWith({ account: accountId, catalog: catalogId });
+    expect(upsertAccountCatalog).toHaveBeenCalledWith({ account: accountId, catalog: catalogId });
     expect(body).toEqual({ status: "success", catalog, songs_added: 0 });
     expect(selectPlaycountSnapshots).not.toHaveBeenCalled();
   });
@@ -154,6 +154,25 @@ describe("createCatalogHandler", () => {
     expect(body).toEqual({ status: "success", catalog, songs_added: 0 });
   });
 
+  it("re-claims link the caller's account so the report can read measurements", async () => {
+    // Regression: the re-claim branch returned the existing catalog without an
+    // account_catalogs link, so a reader whose account differed from the
+    // original claimer (account divergence / prior double-account race) got a
+    // report 404 — getCatalogMeasurementsHandler 404s on a missing link.
+    vi.mocked(validateCreateCatalogBody).mockReturnValue({ snapshot: snapshotId });
+    okAuth();
+    vi.mocked(selectPlaycountSnapshots).mockResolvedValue([
+      { id: snapshotId, account: accountId, catalog: catalogId, isrcs: ["A"] } as never,
+    ]);
+    vi.mocked(selectCatalogById).mockResolvedValue(catalog);
+    vi.mocked(selectSongMeasurements).mockResolvedValue([]);
+
+    const res = await createCatalogHandler(makeRequest());
+
+    expect(res.status).toBe(200);
+    expect(upsertAccountCatalog).toHaveBeenCalledWith({ account: accountId, catalog: catalogId });
+  });
+
   it("re-claims still attach the canonical artist to the roster (chat#1850 P1)", async () => {
     vi.mocked(validateCreateCatalogBody).mockReturnValue({ snapshot: snapshotId });
     okAuth();

lib/catalog/__tests__/createSnapshotCatalog.test.ts

@@ -2,15 +2,15 @@ import { describe, it, expect, vi, beforeEach } from "vitest";
 
 import { createSnapshotCatalog } from "../createSnapshotCatalog";
 import { insertCatalog } from "@/lib/supabase/catalogs/insertCatalog";
-import { insertAccountCatalog } from "@/lib/supabase/account_catalogs/insertAccountCatalog";
+import { upsertAccountCatalog } from "@/lib/supabase/account_catalogs/upsertAccountCatalog";
 import { insertCatalogSongs } from "@/lib/supabase/catalog_songs/insertCatalogSongs";
 import { updatePlaycountSnapshot } from "@/lib/supabase/playcount_snapshots/updatePlaycountSnapshot";
 import { selectSongMeasurements } from "@/lib/supabase/song_measurements/selectSongMeasurements";
 import { attachCanonicalArtistToAccount } from "../attachCanonicalArtistToAccount";
 
 vi.mock("@/lib/supabase/catalogs/insertCatalog", () => ({ insertCatalog: vi.fn() }));
-vi.mock("@/lib/supabase/account_catalogs/insertAccountCatalog", () => ({
-  insertAccountCatalog: vi.fn(),
+vi.mock("@/lib/supabase/account_catalogs/upsertAccountCatalog", () => ({
+  upsertAccountCatalog: vi.fn(),
 }));
 vi.mock("@/lib/supabase/catalog_songs/insertCatalogSongs", () => ({ insertCatalogSongs: vi.fn() }));
 vi.mock("@/lib/supabase/playcount_snapshots/updatePlaycountSnapshot", () => ({
@@ -48,7 +48,7 @@ describe("createSnapshotCatalog", () => {
     const result = await createSnapshotCatalog({ accountId, snapshot, name: "Bad Bunny Catalog" });
 
     expect(insertCatalog).toHaveBeenCalledWith("Bad Bunny Catalog");
-    expect(insertAccountCatalog).toHaveBeenCalledWith({ account: accountId, catalog: catalogId });
+    expect(upsertAccountCatalog).toHaveBeenCalledWith({ account: accountId, catalog: catalogId });
     // ISRCs come from measurements by snapshot, NOT snapshot.isrcs (null here)
     expect(selectSongMeasurements).toHaveBeenCalledWith({ snapshot: snapshotId });
     expect(insertCatalogSongs).toHaveBeenCalledWith([

lib/catalog/createCatalogHandler.ts

@@ -7,7 +7,7 @@ import { createSnapshotCatalog } from "./createSnapshotCatalog";
 import { selectPlaycountSnapshots } from "@/lib/supabase/playcount_snapshots/selectPlaycountSnapshots";
 import { selectCatalogById } from "@/lib/supabase/catalogs/selectCatalogById";
 import { insertCatalog } from "@/lib/supabase/catalogs/insertCatalog";
-import { insertAccountCatalog } from "@/lib/supabase/account_catalogs/insertAccountCatalog";
+import { upsertAccountCatalog } from "@/lib/supabase/account_catalogs/upsertAccountCatalog";
 import { selectSongMeasurements } from "@/lib/supabase/song_measurements/selectSongMeasurements";
 import { attachCanonicalArtistToAccount } from "./attachCanonicalArtistToAccount";
 
@@ -45,7 +45,7 @@ export async function createCatalogHandler(request: NextRequest): Promise<NextRe
 
     if (!validated.snapshot) {
       const catalog = await insertCatalog(validated.name ?? DEFAULT_CATALOG_NAME);
-      await insertAccountCatalog({ account: accountId, catalog: catalog.id });
+      await upsertAccountCatalog({ account: accountId, catalog: catalog.id });
       return successResponse({ catalog, songs_added: 0 });
     }
 
@@ -57,12 +57,17 @@ export async function createCatalogHandler(request: NextRequest): Promise<NextRe
       return errorResponse("Snapshot belongs to a different account", 403);
     }
 
-    // Idempotent re-claim: the run already produced a catalog. Still attach
-    // the canonical artist (chat#1850 P1) so claims made before the roster
-    // attach shipped heal on the next click; the attach is itself idempotent.
+    // Idempotent re-claim: the run already produced a catalog. Link the caller
+    // to it (chat#1867): the report's measurements endpoint 404s on a missing
+    // account_catalogs row, so a reader whose account differs from the original
+    // claimer (account divergence / prior double-account race) otherwise sees
+    // "No valuation found". upsertAccountCatalog is idempotent, so this heals
+    // an unlinked reader without duplicating an existing link. Still attach the
+    // canonical artist (chat#1850 P1); the attach is itself idempotent.
     if (snapshot.catalog) {
       const existing = await selectCatalogById(snapshot.catalog);
       if (existing) {
+        await upsertAccountCatalog({ account: accountId, catalog: existing.id });
         const measurements = await selectSongMeasurements({ snapshot: snapshot.id });
         const isrcs = [...new Set(measurements.map(m => m.song))];
         if (isrcs.length > 0) {

lib/catalog/createSnapshotCatalog.ts

@@ -1,6 +1,6 @@
 import { Tables } from "@/types/database.types";
 import { insertCatalog } from "@/lib/supabase/catalogs/insertCatalog";
-import { insertAccountCatalog } from "@/lib/supabase/account_catalogs/insertAccountCatalog";
+import { upsertAccountCatalog } from "@/lib/supabase/account_catalogs/upsertAccountCatalog";
 import { insertCatalogSongs } from "@/lib/supabase/catalog_songs/insertCatalogSongs";
 import { updatePlaycountSnapshot } from "@/lib/supabase/playcount_snapshots/updatePlaycountSnapshot";
 import { selectSongMeasurements } from "@/lib/supabase/song_measurements/selectSongMeasurements";
@@ -33,7 +33,7 @@ export async function createSnapshotCatalog(params: {
   const { accountId, snapshot, name } = params;
 
   const catalog = await insertCatalog(name ?? DEFAULT_CATALOG_NAME);
-  await insertAccountCatalog({ account: accountId, catalog: catalog.id });
+  await upsertAccountCatalog({ account: accountId, catalog: catalog.id });
 
   const measurements = await selectSongMeasurements({ snapshot: snapshot.id });
   const isrcs = [...new Set(measurements.map(m => m.song))];

lib/supabase/account_catalogs/__tests__/upsertAccountCatalog.test.ts

@@ -0,0 +1,38 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+
+import { upsertAccountCatalog } from "../upsertAccountCatalog";
+
+const mockFrom = vi.fn();
+const mockUpsert = vi.fn();
+
+vi.mock("@/lib/supabase/serverClient", () => ({
+  default: {
+    from: (...args: unknown[]) => mockFrom(...args),
+  },
+}));
+
+describe("upsertAccountCatalog", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    mockFrom.mockReturnValue({ upsert: mockUpsert });
+    mockUpsert.mockResolvedValue({ error: null });
+  });
+
+  it("idempotently links an account to a catalog (upsert, ignore duplicates)", async () => {
+    await upsertAccountCatalog({ account: "acct-1", catalog: "cat-1" });
+
+    expect(mockFrom).toHaveBeenCalledWith("account_catalogs");
+    expect(mockUpsert).toHaveBeenCalledWith(
+      { account: "acct-1", catalog: "cat-1" },
+      { onConflict: "account,catalog", ignoreDuplicates: true },
+    );
+  });
+
+  it("throws when the upsert fails", async () => {
+    mockUpsert.mockResolvedValue({ error: { message: "boom" } });
+
+    await expect(upsertAccountCatalog({ account: "acct-1", catalog: "cat-1" })).rejects.toThrow(
+      /Failed to link account_catalogs/,
+    );
+  });
+});

lib/supabase/account_catalogs/upsertAccountCatalog.ts

@@ -0,0 +1,29 @@
+import supabase from "../serverClient";
+
+/**
+ * Links a catalog to an account by upserting an `account_catalogs` row.
+ *
+ * Idempotent: linking an account already linked to the catalog is a no-op
+ * (`onConflict` on the `(account, catalog)` unique constraint) rather than a
+ * unique-violation, so claim paths can call it unconditionally to guarantee
+ * ownership without first checking for an existing link.
+ *
+ * @param params.account - Account id (owner)
+ * @param params.catalog - Catalog id to link
+ * @throws Error if the upsert fails
+ */
+export async function upsertAccountCatalog(params: {
+  account: string;
+  catalog: string;
+}): Promise<void> {
+  const { error } = await supabase
+    .from("account_catalogs")
+    .upsert(
+      { account: params.account, catalog: params.catalog },
+      { onConflict: "account,catalog", ignoreDuplicates: true },
+    );
+
+  if (error) {
+    throw new Error(`Failed to link account_catalogs: ${error.message}`);
+  }
+}