fix(artists): deleteArtist must not hard-delete a shared canonical with song/catalog dependencies

lib/artists/__tests__/deleteArtist.test.ts

@@ -0,0 +1,89 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+
+import { deleteArtist } from "../deleteArtist";
+import { deleteAccountArtistId } from "@/lib/supabase/account_artist_ids/deleteAccountArtistId";
+import { getAccountArtistIds } from "@/lib/supabase/account_artist_ids/getAccountArtistIds";
+import { deleteAccountById } from "@/lib/supabase/accounts/deleteAccountById";
+import { selectSongArtists } from "@/lib/supabase/song_artists/selectSongArtists";
+
+vi.mock("@/lib/supabase/account_artist_ids/deleteAccountArtistId", () => ({
+  deleteAccountArtistId: vi.fn(),
+}));
+
+vi.mock("@/lib/supabase/account_artist_ids/getAccountArtistIds", () => ({
+  getAccountArtistIds: vi.fn(),
+}));
+
+vi.mock("@/lib/supabase/accounts/deleteAccountById", () => ({
+  deleteAccountById: vi.fn(),
+}));
+
+vi.mock("@/lib/supabase/song_artists/selectSongArtists", () => ({
+  selectSongArtists: vi.fn(),
+}));
+
+describe("deleteArtist", () => {
+  const artistId = "550e8400-e29b-41d4-a716-446655440000";
+  const requesterAccountId = "660e8400-e29b-41d4-a716-446655440000";
+  const link = { account_id: requesterAccountId, artist_id: artistId };
+
+  beforeEach(() => {
+    vi.clearAllMocks();
+    vi.mocked(deleteAccountArtistId).mockResolvedValue([link] as never);
+    vi.mocked(deleteAccountById).mockResolvedValue(undefined as never);
+  });
+
+  it("throws when the requester link could not be deleted", async () => {
+    vi.mocked(deleteAccountArtistId).mockResolvedValue([] as never);
+
+    await expect(deleteArtist({ artistId, requesterAccountId })).rejects.toThrow(
+      "Failed to delete artist link",
+    );
+    expect(deleteAccountById).not.toHaveBeenCalled();
+  });
+
+  it("does not hard-delete when other roster links remain", async () => {
+    vi.mocked(getAccountArtistIds).mockResolvedValue([
+      { account_id: "another-account", artist_id: artistId },
+    ] as never);
+
+    const result = await deleteArtist({ artistId, requesterAccountId });
+
+    expect(result).toBe(artistId);
+    expect(deleteAccountById).not.toHaveBeenCalled();
+    expect(selectSongArtists).not.toHaveBeenCalled();
+  });
+
+  it("keeps the canonical account when the last link is removed but song dependencies exist", async () => {
+    vi.mocked(getAccountArtistIds).mockResolvedValue([] as never);
+    vi.mocked(selectSongArtists).mockResolvedValue([
+      { id: "sa-1", song: "ISRC1", artist: artistId },
+    ] as never);
+
+    const result = await deleteArtist({ artistId, requesterAccountId });
+
+    expect(result).toBe(artistId);
+    expect(selectSongArtists).toHaveBeenCalledWith({ artists: [artistId] });
+    expect(deleteAccountById).not.toHaveBeenCalled();
+  });
+
+  it("fails closed and keeps the canonical account when the dependency lookup errors", async () => {
+    vi.mocked(getAccountArtistIds).mockResolvedValue([] as never);
+    vi.mocked(selectSongArtists).mockResolvedValue(null);
+
+    const result = await deleteArtist({ artistId, requesterAccountId });
+
+    expect(result).toBe(artistId);
+    expect(deleteAccountById).not.toHaveBeenCalled();
+  });
+
+  it("hard-deletes the account when the last link is removed and no dependencies exist", async () => {
+    vi.mocked(getAccountArtistIds).mockResolvedValue([] as never);
+    vi.mocked(selectSongArtists).mockResolvedValue([] as never);
+
+    const result = await deleteArtist({ artistId, requesterAccountId });
+
+    expect(result).toBe(artistId);
+    expect(deleteAccountById).toHaveBeenCalledWith(artistId);
+  });
+});

lib/artists/deleteArtist.ts

@@ -1,6 +1,7 @@
 import { deleteAccountArtistId } from "@/lib/supabase/account_artist_ids/deleteAccountArtistId";
 import { getAccountArtistIds } from "@/lib/supabase/account_artist_ids/getAccountArtistIds";
 import { deleteAccountById } from "@/lib/supabase/accounts/deleteAccountById";
+import { selectSongArtists } from "@/lib/supabase/song_artists/selectSongArtists";
 
 export interface DeleteArtistParams {
   artistId: string;
@@ -12,7 +13,11 @@ export interface DeleteArtistParams {
  *
  * The validator is responsible for existence and access checks. This helper
  * only removes the direct owner link and deletes the artist account if that
- * link was the last remaining association.
+ * link was the last remaining association AND the canonical artist has no
+ * song dependencies. account_artist_ids.artist_id cascades on account delete,
+ * so hard-deleting a canonical with song_artists rows would destroy the song
+ * graph shared across customers — in that case only the caller's link is
+ * removed and the canonical account is kept.
  *
  * @param params - Delete artist parameters
  * @param params.artistId - Artist account ID to remove
@@ -33,7 +38,13 @@ export async function deleteArtist({
   });
 
   if (remainingLinks.length === 0) {
-    await deleteAccountById(artistId);
+    const songArtists = await selectSongArtists({ artists: [artistId] });
+    // Fail closed: a null (query error) is treated as "has dependencies" so an
+    // unknown state never hard-deletes a canonical that may own a song graph.
+    const hasSongDependencies = songArtists === null || songArtists.length > 0;
+    if (!hasSongDependencies) {
+      await deleteAccountById(artistId);
+    }
   }
 
   return artistId;

lib/catalog/__tests__/attachCanonicalArtistToAccount.test.ts

@@ -1,12 +1,12 @@
 import { describe, it, expect, vi, beforeEach } from "vitest";
 
 import { attachCanonicalArtistToAccount } from "../attachCanonicalArtistToAccount";
-import { selectSongArtistsBySongs } from "@/lib/supabase/song_artists/selectSongArtistsBySongs";
+import { selectSongArtists } from "@/lib/supabase/song_artists/selectSongArtists";
 import { selectAccountArtistId } from "@/lib/supabase/account_artist_ids/selectAccountArtistId";
 import { insertAccountArtistId } from "@/lib/supabase/account_artist_ids/insertAccountArtistId";
 
-vi.mock("@/lib/supabase/song_artists/selectSongArtistsBySongs", () => ({
-  selectSongArtistsBySongs: vi.fn(),
+vi.mock("@/lib/supabase/song_artists/selectSongArtists", () => ({
+  selectSongArtists: vi.fn(),
 }));
 vi.mock("@/lib/supabase/account_artist_ids/selectAccountArtistId", () => ({
   selectAccountArtistId: vi.fn(),
@@ -23,7 +23,7 @@ describe("attachCanonicalArtistToAccount", () => {
   beforeEach(() => vi.clearAllMocks());
 
   it("resolves the dominant artist through song_artists and links it to the account", async () => {
-    vi.mocked(selectSongArtistsBySongs).mockResolvedValue([
+    vi.mocked(selectSongArtists).mockResolvedValue([
       link("A", canonicalId),
       link("B", canonicalId),
       link("B", "collab-1"),
@@ -32,13 +32,13 @@ describe("attachCanonicalArtistToAccount", () => {
 
     const result = await attachCanonicalArtistToAccount({ accountId, isrcs: ["A", "B"] });
 
-    expect(selectSongArtistsBySongs).toHaveBeenCalledWith(["A", "B"]);
+    expect(selectSongArtists).toHaveBeenCalledWith({ songs: ["A", "B"] });
     expect(insertAccountArtistId).toHaveBeenCalledWith(accountId, canonicalId);
     expect(result).toBe(canonicalId);
   });
 
   it("does not insert when the account already has the canonical artist", async () => {
-    vi.mocked(selectSongArtistsBySongs).mockResolvedValue([link("A", canonicalId)]);
+    vi.mocked(selectSongArtists).mockResolvedValue([link("A", canonicalId)]);
     vi.mocked(selectAccountArtistId).mockResolvedValue({
       id: "existing",
       artist_id: canonicalId,
@@ -52,7 +52,7 @@ describe("attachCanonicalArtistToAccount", () => {
   });
 
   it("no-ops when the songs have no artist links yet", async () => {
-    vi.mocked(selectSongArtistsBySongs).mockResolvedValue([]);
+    vi.mocked(selectSongArtists).mockResolvedValue([]);
 
     const result = await attachCanonicalArtistToAccount({ accountId, isrcs: ["A"] });
 
@@ -64,13 +64,13 @@ describe("attachCanonicalArtistToAccount", () => {
   it("no-ops when there are no ISRCs", async () => {
     const result = await attachCanonicalArtistToAccount({ accountId, isrcs: [] });
 
-    expect(selectSongArtistsBySongs).not.toHaveBeenCalled();
+    expect(selectSongArtists).not.toHaveBeenCalled();
     expect(result).toBeNull();
   });
 
   it("never throws: a failed attach must not fail the claim", async () => {
     const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {});
-    vi.mocked(selectSongArtistsBySongs).mockResolvedValue([link("A", canonicalId)]);
+    vi.mocked(selectSongArtists).mockResolvedValue([link("A", canonicalId)]);
     vi.mocked(selectAccountArtistId).mockResolvedValue(null);
     vi.mocked(insertAccountArtistId).mockRejectedValue(new Error("insert failed"));
 

lib/catalog/attachCanonicalArtistToAccount.ts

@@ -1,4 +1,4 @@
-import { selectSongArtistsBySongs } from "@/lib/supabase/song_artists/selectSongArtistsBySongs";
+import { selectSongArtists } from "@/lib/supabase/song_artists/selectSongArtists";
 import { getDominantSongArtist } from "@/lib/songs/getDominantSongArtist";
 import { selectAccountArtistId } from "@/lib/supabase/account_artist_ids/selectAccountArtistId";
 import { insertAccountArtistId } from "@/lib/supabase/account_artist_ids/insertAccountArtistId";
@@ -25,7 +25,7 @@ export async function attachCanonicalArtistToAccount(params: {
   try {
     if (isrcs.length === 0) return null;
 
-    const links = await selectSongArtistsBySongs(isrcs);
+    const links = (await selectSongArtists({ songs: isrcs })) ?? [];
     const artistId = getDominantSongArtist(links);
     if (!artistId) return null;
 

lib/supabase/song_artists/__tests__/selectSongArtists.test.ts

@@ -0,0 +1,74 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+
+import { selectSongArtists } from "../selectSongArtists";
+
+const mockFrom = vi.fn();
+const mockSelect = vi.fn();
+const mockIn = vi.fn();
+
+vi.mock("@/lib/supabase/serverClient", () => ({
+  default: {
+    from: (...args: unknown[]) => mockFrom(...args),
+  },
+}));
+
+describe("selectSongArtists", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    mockFrom.mockReturnValue({ select: mockSelect });
+    mockSelect.mockReturnValue({ in: mockIn });
+  });
+
+  it("filters by song ISRCs", async () => {
+    const rows = [{ id: "sa-1", song: "ISRC1", artist: "artist-1" }];
+    mockIn.mockResolvedValue({ data: rows, error: null });
+
+    const result = await selectSongArtists({ songs: ["ISRC1"] });
+
+    expect(mockFrom).toHaveBeenCalledWith("song_artists");
+    expect(mockIn).toHaveBeenCalledWith("song", ["ISRC1"]);
+    expect(result).toEqual(rows);
+  });
+
+  it("filters by artist account IDs", async () => {
+    const rows = [{ id: "sa-1", song: "ISRC1", artist: "artist-1" }];
+    mockIn.mockResolvedValue({ data: rows, error: null });
+
+    const result = await selectSongArtists({ artists: ["artist-1"] });
+
+    expect(mockIn).toHaveBeenCalledWith("artist", ["artist-1"]);
+    expect(result).toEqual(rows);
+  });
+
+  it("returns an empty array without querying when the filter list is empty", async () => {
+    const result = await selectSongArtists({ artists: [] });
+
+    expect(result).toEqual([]);
+    expect(mockFrom).not.toHaveBeenCalled();
+  });
+
+  it("chunks large filter lists to keep .in() within URL limits", async () => {
+    mockIn.mockResolvedValue({ data: [], error: null });
+    const songs = Array.from({ length: 450 }, (_, i) => `ISRC${i}`);
+
+    await selectSongArtists({ songs });
+
+    // 450 / 200 => 3 chunks
+    expect(mockIn).toHaveBeenCalledTimes(3);
+  });
+
+  it("returns null on query error (callers decide how to treat an unknown state)", async () => {
+    const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {});
+    mockIn.mockResolvedValue({ data: null, error: { message: "boom" } });
+
+    const result = await selectSongArtists({ artists: ["artist-1"] });
+
+    expect(result).toBeNull();
+    expect(consoleSpy).toHaveBeenCalled();
+    consoleSpy.mockRestore();
+  });
+
+  it("throws when neither songs nor artists is provided", async () => {
+    await expect(selectSongArtists({})).rejects.toThrow("Must provide either songs or artists");
+  });
+});

lib/supabase/song_artists/selectSongArtists.ts

@@ -0,0 +1,45 @@
+import supabase from "../serverClient";
+import type { Tables } from "@/types/database.types";
+
+// PostgREST encodes .in() filters in the query string — chunk so large
+// inputs (1,000+ ISRCs) can't overflow the URL.
+const CHUNK_SIZE = 200;
+
+/**
+ * Select song_artists rows filtered by song ISRCs or artist account IDs.
+ *
+ * Provide exactly one of `songs` or `artists` (songs takes precedence if both
+ * are given). The filter list is chunked to keep `.in()` within URL limits.
+ *
+ * @param params.songs - Song ISRCs to match on the `song` column
+ * @param params.artists - Artist account IDs to match on the `artist` column
+ * @returns The matching rows, or null on query error
+ */
+export async function selectSongArtists(params: {
+  songs?: string[];
+  artists?: string[];
+}): Promise<Tables<"song_artists">[] | null> {
+  const { songs, artists } = params;
+  const column = songs ? "song" : artists ? "artist" : null;
+  const values = songs ?? artists;
+
+  if (!column || !values) {
+    throw new Error("Must provide either songs or artists");
+  }
+  if (values.length === 0) return [];
+
+  const rows: Tables<"song_artists">[] = [];
+  for (let i = 0; i < values.length; i += CHUNK_SIZE) {
+    const chunk = values.slice(i, i + CHUNK_SIZE);
+    const { data, error } = await supabase.from("song_artists").select("*").in(column, chunk);
+
+    if (error) {
+      console.error("Error fetching song_artists:", error);
+      return null;
+    }
+
+    rows.push(...(data ?? []));
+  }
+
+  return rows;
+}