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

[sweetmantech]

Problem

Promoted deleteArtist-guard item from recoupable/chat#1860.

lib/artists/deleteArtist.ts removes the caller's own roster link (correctly scoped), but then hard-deletes the canonical artist account via deleteAccountById(artistId) whenever zero roster links remain across ALL accounts. account_artist_ids.artist_id is ON DELETE CASCADE, so this cascades into song_artists and destroys the shared song graph. A canonical artist with a catalog but a single roster link gets permanently destroyed when that one account removes it: irreversible cross-customer data loss.

Fix

• New lib/supabase/song_artists/getSongArtistExistsByArtist.ts: head-count existence check for song_artists rows by artist account id. Fails safe: on query error it returns true so an unknown state is never treated as "no dependencies".
• deleteArtist now checks for song dependencies before the deleteAccountById call. When dependencies exist, only the caller's link is removed and the canonical account + song graph stay intact.

Behavior

• Last link removed, canonical has song_artists rows: link removed, NO hard delete, song graph intact.
• Last link removed, no dependencies: hard-deletes as today.
• Non-last-link removal: unchanged (dependency check not even queried).

Verification

• TDD: failing tests written first (confirmed RED), then implementation to GREEN.
• pnpm exec vitest run lib/artists lib/supabase/song_artists: 19 files, 110 tests, all passing (includes 4 new deleteArtist guard tests + 3 new supabase-helper tests).
• pnpm exec tsc --noEmit: 200 pre-existing errors on main, identical count with this change: zero new errors, none in touched files.

Preview verification pending — tracked in chat#1860.

🤖 Generated with Claude Code

---

Summary by cubic

Prevent hard-deleting a canonical artist that still has song dependencies. deleteArtist now removes the caller’s roster link and only hard-deletes when no links remain and no song_artists rows exist; lookup errors keep the canonical.

• Bug Fixes

  • Guarded deleteAccountById in lib/artists/deleteArtist.ts using selectSongArtists({ artists: [artistId] }); null or non-empty => keep canonical; empty => delete.

• Refactors

  • Replaced selectSongArtistsBySongs/selectSongArtistsByArtist with selectSongArtists({ songs?, artists? }) and updated attachCanonicalArtistToAccount to use it; removed old helpers and updated tests.

^{Written for commit 7cf611a. Summary will update on new commits.}

[vercel]

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

Project	Deployment	Actions	Updated (UTC)
api	Ready	Preview	Jul 9, 2026 5:46pm

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 24 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

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

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

How do review limits work?

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

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 9dfe460a-9a99-4e72-8973-8717c15ebe6a

📥 Commits

Reviewing files that changed from the base of the PR and between 1b81272 and 7cf611a.

⛔ Files ignored due to path filters (4)

• lib/artists/__tests__/deleteArtist.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
• lib/catalog/__tests__/attachCanonicalArtistToAccount.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
• lib/supabase/song_artists/__tests__/selectSongArtists.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
• lib/supabase/song_artists/__tests__/selectSongArtistsBySongs.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**

📒 Files selected for processing (4)

• lib/artists/deleteArtist.ts
• lib/catalog/attachCanonicalArtistToAccount.ts
• lib/supabase/song_artists/selectSongArtists.ts
• lib/supabase/song_artists/selectSongArtistsBySongs.ts

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/deleteartist-shared-canonical-guard

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[cubic-dev-ai]

All reported issues were addressed across 4 files

Confidence score: 5/5

• Safe to merge after the addressed issues were fixed.

Architecture diagram

sequenceDiagram
    participant Client as Caller (e.g. API route)
    participant deleteArtist as deleteArtist()
    participant DeleteLink as deleteAccountArtistId()
    participant GetLinks as getAccountArtistIds()
    participant CheckSongs as getSongArtistExistsByArtist()
    participant DeleteAccount as deleteAccountById()
    participant DB as Database (Supabase)

    Note over Client,DB: NEW: deleteArtist song-dependency guard flow

    Client->>deleteArtist: deleteArtist({artistId, requesterAccountId})
    
    deleteArtist->>DeleteLink: deleteAccountArtistId(link)
    DeleteLink-->>deleteArtist: deleted link or []
    
    alt Link deletion failed
        deleteArtist-->>Client: throw "Failed to delete artist link"
    else Link deleted successfully
        deleteArtist->>GetLinks: getAccountArtistIds(artistId)
        GetLinks-->>deleteArtist: remaining roster links
        
        alt Other roster links still exist
            Note over deleteArtist: Shared canonical, just remove link
            deleteArtist-->>Client: return artistId
        else No remaining links (last link removed)
            Note over deleteArtist,DeleteAccount: NEW: Check song dependencies before hard-delete
            deleteArtist->>CheckSongs: getSongArtistExistsByArtist(artistId)
            
            CheckSongs->>DB: SELECT count(*) FROM song_artists WHERE artist = artistId (head only)
            
            alt Query error
                DB-->>CheckSongs: error
                Note over CheckSongs: Fails safe → treat as dependencies exist
                CheckSongs-->>deleteArtist: true
            else No errors
                DB-->>CheckSongs: count (0 or >0)
                CheckSongs-->>deleteArtist: (count ?? 0) > 0
            end
            
            alt hasSongDependencies is true
                Note over deleteArtist: Canonical has songs — keep account, song graph intact
                deleteArtist-->>Client: return artistId
            else hasSongDependencies is false
                Note over deleteArtist: Clean slate — hard-delete as before
                deleteArtist->>DeleteAccount: deleteAccountById(artistId)
                DeleteAccount->>DB: DELETE FROM accounts WHERE id = artistId
                Note over DB: CASCADE deletes account_artist_ids row
                DeleteAccount-->>deleteArtist: void
                deleteArtist-->>Client: return artistId
            end
        end
    end

Loading

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

Re-trigger cubic</sub>

[sweetmantech]

Preview verification — deleteArtist shared-canonical guard

Preview: https://api-4j85w909x-recoup.vercel.app (deployment 5367840084, built from 19033b14 — the PR head)
When: 2026-07-09 16:43 UTC
Caller: non-staff Privy token for account …ocnxy81t.

Guardrails (non-destructive — no delete proceeds)

#	Path	Request	Documented	Actual	Verdict
A	Auth	DELETE /api/artists/{id} no credential	401	HTTP 401	✅
B	Validation	DELETE /api/artists/not-a-uuid	400	HTTP 400 "id must be a valid UUID"	✅
C	Not found	DELETE /api/artists/<random UUID>	404	HTTP 404 "Artist not found"	✅
D	Access	DELETE /api/artists/<artist caller can't access>	403	HTTP 403 "Unauthorized delete attempt"	✅ no delete on a non-owned artist

Branch 1 — no dependencies → hard-delete (unchanged behavior, verified end-to-end)

Self-contained throwaway fixture so nothing shared was touched:

1. POST /api/artists {name:"[TEST DELETE ME] …"} → 201, artist 322e30f8… (caller becomes sole roster link).
2. GET /api/songs?artist_account_id=322e30f8… → 0 songs (no song_artists dependency).
3. DELETE /api/artists/322e30f8… → HTTP 200 {"success":true,"artistId":"322e30f8…"}.
4. DELETE /api/artists/322e30f8… again → HTTP 404 "Artist not found" → confirms the canonical account was hard-deleted (last link + no deps → deletes, as before). ✅

Branch 2 — has song dependencies → keep canonical (the new guard)

This is the destructive branch the PR adds. It cannot be safely executed live: there is no API to write song_artists, and the only way to trigger it is a last-link delete of a canonical that already has a song graph — which on the shared prod DB would risk the exact cross-customer data loss the guard exists to prevent. Instead I verified the data condition the guard keys on is real and populated in prod (via GET /api/songs?artist_account_id=, which reads the same song_artists table as getSongArtistExistsByArtist):

Canonical	song_artists rows
Ana Bárbara (f584e4f5…)	154
Elvis Crespo (b1814076…)	322
Luis Enrique (e943ceaf…)	192
Charlie Zaa (99997dc7…)	116

For any of these, getSongArtistExistsByArtist returns true, so a last-link delete now removes only the caller's link and keeps the canonical + its song graph — pre-guard, that same delete would have cascaded and destroyed 100s of song_artists rows. The branch logic itself is covered by the unit suite: 110 tests green (pnpm exec vitest run lib/artists lib/supabase/song_artists), incl. the 4 deleteArtist guard cases (last-link-with-deps keeps; last-link-no-deps deletes; other-links-remain skips the check; failed-link-delete throws) and the 3 helper cases (true/false/fail-safe-true-on-error).

Verdict

Guardrails (401/400/404/403) and the no-dependency hard-delete path are verified live end-to-end; the protective guard's dependency signal is confirmed real against multiple 100+-song canonicals. The destructive last-link-with-deps branch is unit-covered and intentionally not executed against shared prod data. No regressions observed. Throwaway fixture fully cleaned up (hard-deleted).

🤖 Generated with Claude Code

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[sweetmantech]

Made the song_artists lib generic (review follow-up)

Per feedback that supabase libs should be generic/reusable rather than single-purpose, replaced getSongArtistExistsByArtist (boolean, fail-safe baked into the lib) with a generic selectSongArtistsByArtist(artist) in d986c433. It returns rows (null on error), mirroring the existing selectSongArtistsBySongs.

The fail-closed policy moved into deleteArtist where it belongs as domain logic:

const songArtists = await selectSongArtistsByArtist(artistId);
// Fail closed: null (query error) or any rows => keep the canonical.
const hasSongDependencies = songArtists === null || songArtists.length > 0;
if (!hasSongDependencies) {
  await deleteAccountById(artistId);
}

So the safety-critical bias is unchanged: a query error still keeps the canonical (never hard-deletes on an unknown state). Added a deleteArtist test for that null/error path. 111 tests green; tsc + lint + prettier clean on touched files.

Note: the one efficiency trade-off is this now select("*")s the rows instead of a head/count existence check — negligible here (guard only runs on a last-link delete), and worth it for a reusable lib.

[cubic-dev-ai]

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

<sub>Requires human review: Bug fix for artist deletion logic with song dependency check.

Re-trigger cubic</sub>

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[sweetmantech]

Consolidated to one param-driven selectSongArtists (review follow-up)

Good call — this matches the shared-select convention already used elsewhere (e.g. getAccountArtistIds({ artistIds?, accountIds? })). In 7cf611a2 the two single-column helpers (selectSongArtistsBySongs + selectSongArtistsByArtist) are replaced by one:

selectSongArtists({ songs?, artists? }): Promise<Tables<"song_artists">[] | null>

Chunked .in() (keeps the 1,000+ ISRC URL-limit handling), null on error. Both callers updated:

• attachCanonicalArtistToAccount → selectSongArtists({ songs: isrcs }) (coalesces null → [] for its best-effort path — behavior unchanged)
• deleteArtist → selectSongArtists({ artists: [artistId] }) (fail-closed: null or any rows keeps the canonical)

172 tests green across lib/artists, lib/supabase/song_artists, lib/catalog; tsc + lint + prettier clean. Net: one reusable lib file instead of two single-purpose ones.

[cubic-dev-ai]

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

<sub>Requires human review: Prevents data loss by guarding hard-delete of canonical artists with song dependencies; adds new helper to check song_artists rows. Refactors existing helper to support multiple filters. Core business logic changes require human review.

Re-trigger cubic</sub>

[sweetmantech]

Guard-200 verified live — the last-link + has-dependencies branch

Follow-up to the earlier run (which only covered the no-dependency hard-delete). This exercises the branch the PR actually adds: last roster link removed on a canonical that has song_artists → link removed, canonical + song graph kept, HTTP 200.

Preview: https://api-2j8hgd0s4-recoup.vercel.app (built from head 7cf611a2, i.e. after both review refactors).
When: 2026-07-09 19:05 UTC. Caller: non-staff account 2d3f7de4….

To prove the guard fired (and not the trivial "other links remain" path), I needed to be the sole roster link at delete time. Found a real canonical — artist account 0067a08e-0963-46e2-a488-7a38e94f7138 — that had 2 song_artists rows and 0 roster links (a song-graph artist nobody had claimed). Made myself its only link, then deleted:

Step	Action	Result
baseline	DB state of 0067a08e…	exists, song_artists=2, roster_links=0
1	POST /api/accounts/artists {artistId} (become sole link)	HTTP 200
2	DB after add	my_link_present=true, total_roster_links=1 (only me)
3	DELETE /api/artists/0067a08e… (last link + has deps)	HTTP 200 {"success":true}
4	DB after delete	artist_account_exists=true, song_artists_count=2, my_link_present=false, total_roster_links=0
5	GET /api/songs?artist_account_id=0067a08e…	2 songs (graph intact)

This is the whole point of the PR: pre-guard, that same sole-link delete would have hit deleteAccountById → cascaded account_artist_ids.artist_id → destroyed both the canonical account and its 2 song_artists rows (artist_account_exists=false, song_artists_count=0). With the guard, both survive. Combined with the earlier no-deps run (throwaway artist → 200 → re-DELETE 404, i.e. hard-delete still happens when there are no dependencies), both branches are now verified live, plus the guardrails (401/400/404/403).

The fixture is left exactly as found (link removed → back to 0 roster links, 2 songs, account intact) — net-zero, no shared data mutated. Read-only DB queries were used only to locate a sole-link candidate and to verify survival; the delete itself went through the real API.

Coverage summary (this PR)

• ✅ Guardrails: 401 / 400 / 404 / 403
• ✅ No-dependency branch: last link, no songs → hard-deletes (200 → 404)
• ✅ Guard branch: last link, has songs → keeps canonical + song graph (200) ← this run
• ✅ Unit: 172 tests green across lib/artists, lib/supabase/song_artists, lib/catalog (incl. fail-closed-on-error case)

🤖 Generated with Claude Code