Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions app/api/spotify/artist/[id]/route.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import { siteConfig } from "@/lib/config";
import { NextRequest, NextResponse } from "next/server";

/**
* GET /api/spotify/artist/[id]
*
* Fetches a single Spotify artist by ID via the Recoup API proxy.
* Returns { id, name, image, genre, followers } or 404.
*/
export async function GET(
_req: NextRequest,
{ params }: { params: Promise<{ id: string }> },
) {
const { id } = await params;
if (!id) {
return NextResponse.json({ error: "missing id" }, { status: 400 });
}

try {
const res = await fetch(`${siteConfig.apiUrl}/spotify/artist/${id}`);
Comment on lines +14 to +20

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win

SSRF via unvalidated id path parameter.

id is URL-decoded by Next.js before reaching the handler, so a request like /api/spotify/artist/..%2F..%2Fadmin yields id = "../../admin". When interpolated into the fetch URL, fetch() resolves the ../ segments, potentially reaching arbitrary endpoints on the upstream API host (e.g., ${siteConfig.apiUrl}/admin). Validate id before use — Spotify IDs are base-62, so a simple alphanumeric check suffices. As per coding guidelines, use Zod for all validation in **/*.{ts,tsx,js,jsx} files.

🔒 Proposed fix: validate `id` with Zod
+import { z } from "zod";
+
 const { id } = await params;
- if (!id) {
-   return NextResponse.json({ error: "missing id" }, { status: 400 });
- }
+ const parsed = z.string().regex(/^[a-zA-Z0-9]+$/).safeParse(id);
+ if (!parsed.success) {
+   return NextResponse.json({ error: "invalid id" }, { status: 400 });
+ }
+ const safeId = parsed.data;

Then use safeId in the fetch URL:

-    const res = await fetch(`${siteConfig.apiUrl}/spotify/artist/${id}`);
+    const res = await fetch(`${siteConfig.apiUrl}/spotify/artist/${safeId}`);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const { id } = await params;
if (!id) {
return NextResponse.json({ error: "missing id" }, { status: 400 });
}
try {
const res = await fetch(`${siteConfig.apiUrl}/spotify/artist/${id}`);
import { z } from "zod";
const { id } = await params;
const parsed = z.string().regex(/^[a-zA-Z0-9]+$/).safeParse(id);
if (!parsed.success) {
return NextResponse.json({ error: "invalid id" }, { status: 400 });
}
const safeId = parsed.data;
try {
const res = await fetch(`${siteConfig.apiUrl}/spotify/artist/${safeId}`);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/api/spotify/artist/`[id]/route.ts around lines 14 - 20, The `route.ts`
handler is interpolating the decoded `id` path param directly into `fetch()` in
`GET`, which can allow path traversal on the upstream host. Add Zod validation
for `id` in this route, restricting it to Spotify’s base-62/alphanumeric format
before any request is made. Use a validated `safeId` (or equivalent) in the
`${siteConfig.apiUrl}/spotify/artist/...` fetch URL and return a 400 response
when validation fails.

Source: Coding guidelines

Comment on lines +19 to +20

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

No timeout on upstream fetch.

If the upstream API hangs, this request blocks until the serverless function times out. Add an AbortController with a reasonable timeout (e.g., 5–8 s). The same pattern applies to fetchArtistServer.ts and fetchArtist.ts.

⏱️ Proposed fix: add AbortController timeout
+  const controller = new AbortController();
+  const timeout = setTimeout(() => controller.abort(), 8000);
   try {
-    const res = await fetch(`${siteConfig.apiUrl}/spotify/artist/${safeId}`);
+    const res = await fetch(`${siteConfig.apiUrl}/spotify/artist/${safeId}`, {
+      signal: controller.signal,
+    });
+    clearTimeout(timeout);
     if (!res.ok) {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/api/spotify/artist/`[id]/route.ts around lines 19 - 20, The upstream
fetch in the Spotify artist route has no timeout, so a hung API call can block
the request until the function times out. Update the fetch in the route handler
that uses siteConfig.apiUrl and also apply the same AbortController timeout
pattern in fetchArtistServer.ts and fetchArtist.ts, using a reasonable timeout
(about 5–8 seconds). Make sure the abort signal is wired into the fetch call and
that timeout-related failures are handled consistently.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: The id path parameter is interpolated directly into the upstream fetch URL without validation. Since Next.js URL-decodes dynamic route params, a crafted request like /api/spotify/artist/..%2F..%2Fadmin produces id = "../../admin", which after URL resolution by fetch() can reach arbitrary paths on the upstream API host (SSRF). Spotify artist IDs are strictly alphanumeric — add a regex check (e.g., /^[a-zA-Z0-9]+$/) to reject anything else before constructing the URL.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/api/spotify/artist/[id]/route.ts, line 20:

<comment>The `id` path parameter is interpolated directly into the upstream fetch URL without validation. Since Next.js URL-decodes dynamic route params, a crafted request like `/api/spotify/artist/..%2F..%2Fadmin` produces `id = "../../admin"`, which after URL resolution by `fetch()` can reach arbitrary paths on the upstream API host (SSRF). Spotify artist IDs are strictly alphanumeric — add a regex check (e.g., `/^[a-zA-Z0-9]+$/`) to reject anything else before constructing the URL.</comment>

<file context>
@@ -0,0 +1,36 @@
+  }
+
+  try {
+    const res = await fetch(`${siteConfig.apiUrl}/spotify/artist/${id}`);
+    if (!res.ok) {
+      return NextResponse.json({ error: "not found" }, { status: 404 });
</file context>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The upstream fetch has no timeout configured. If the upstream Spotify API hangs, this serverless function will block until the platform's execution timeout (potentially 30s+), wasting resources and degrading user experience. Consider adding an AbortController with a reasonable timeout (e.g., 5–8s) to fail fast.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/api/spotify/artist/[id]/route.ts, line 20:

<comment>The upstream `fetch` has no timeout configured. If the upstream Spotify API hangs, this serverless function will block until the platform's execution timeout (potentially 30s+), wasting resources and degrading user experience. Consider adding an `AbortController` with a reasonable timeout (e.g., 5–8s) to fail fast.</comment>

<file context>
@@ -0,0 +1,36 @@
+  }
+
+  try {
+    const res = await fetch(`${siteConfig.apiUrl}/spotify/artist/${id}`);
+    if (!res.ok) {
+      return NextResponse.json({ error: "not found" }, { status: 404 });
</file context>

if (!res.ok) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The non-ok upstream response is always mapped to 404 with a generic "not found" error. This loses signal from upstream 429 (rate-limit), 500, or 502 responses. The existing /api/spotify/search route preserves res.status instead of hardcoding a status code, which is the established convention in this codebase.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/api/spotify/artist/[id]/route.ts, line 21:

<comment>The non-ok upstream response is always mapped to 404 with a generic "not found" error. This loses signal from upstream 429 (rate-limit), 500, or 502 responses. The existing `/api/spotify/search` route preserves `res.status` instead of hardcoding a status code, which is the established convention in this codebase.</comment>

<file context>
@@ -0,0 +1,36 @@
+
+  try {
+    const res = await fetch(`${siteConfig.apiUrl}/spotify/artist/${id}`);
+    if (!res.ok) {
+      return NextResponse.json({ error: "not found" }, { status: 404 });
+    }
</file context>

return NextResponse.json({ error: "not found" }, { status: 404 });
}

const a = await res.json();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Custom agent: Enforce Clear Code Style and Maintainability Practices

The variable a is implicitly typed as any from res.json() and its name gives no indication of what it represents. This bypasses strict TypeScript checking and hurts readability. Consider renaming it to rawArtist and adding an inline type (or extracting an interface) so the expected Spotify API shape is explicit, matching the pattern already used in app/api/spotify/search/route.ts.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/api/spotify/artist/[id]/route.ts, line 25:

<comment>The variable `a` is implicitly typed as `any` from `res.json()` and its name gives no indication of what it represents. This bypasses strict TypeScript checking and hurts readability. Consider renaming it to `rawArtist` and adding an inline type (or extracting an interface) so the expected Spotify API shape is explicit, matching the pattern already used in `app/api/spotify/search/route.ts`.</comment>

<file context>
@@ -0,0 +1,36 @@
+      return NextResponse.json({ error: "not found" }, { status: 404 });
+    }
+
+    const a = await res.json();
+    return NextResponse.json({
+      id: a.id ?? id,
</file context>

return NextResponse.json({
id: a.id ?? id,
name: a.name ?? "Unknown Artist",
image: a.images?.[0]?.url ?? null,
genre: a.genres?.[0] ?? null,
followers: a.followers?.total ?? 0,
});
} catch {
return NextResponse.json({ error: "fetch failed" }, { status: 502 });
}
}
58 changes: 58 additions & 0 deletions app/valuation/[artistId]/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import type { Metadata } from "next";
import { fetchArtistServer } from "@/lib/spotify/fetchArtistServer";
import { CatalogValuation } from "@/components/valuation/CatalogValuation";

type Props = {
params: Promise<{ artistId: string }>;
};

export async function generateMetadata({ params }: Props): Promise<Metadata> {
const { artistId } = await params;
const artist = await fetchArtistServer(artistId);
const name = artist?.name ?? "this artist";

return {
title: `What's ${name}'s Catalog Worth? | Recoup`,
description: `Get a directional valuation of ${name}'s recorded catalog from live Spotify play counts — measured, not guessed.`,
openGraph: {
title: `What's ${name}'s Catalog Worth?`,
description: `Run a live catalog valuation for ${name} — one click, no uploads, no statements.`,
...(artist?.image ? { images: [{ url: artist.image, width: 640, height: 640 }] } : {}),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: When fetchArtistServer returns null or the artist has no image, the Open Graph metadata omits the images field entirely. For a summary_large_image Twitter card and rich link previews, a missing image can degrade the card to a smaller format or show an empty/generic preview. Consider providing a branded fallback image (e.g., a site-level default OG asset) so shared links always render a rich preview even when the artist image is unavailable.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/valuation/[artistId]/page.tsx, line 20:

<comment>When `fetchArtistServer` returns null or the artist has no image, the Open Graph metadata omits the `images` field entirely. For a `summary_large_image` Twitter card and rich link previews, a missing image can degrade the card to a smaller format or show an empty/generic preview. Consider providing a branded fallback image (e.g., a site-level default OG asset) so shared links always render a rich preview even when the artist image is unavailable.</comment>

<file context>
@@ -0,0 +1,58 @@
+    openGraph: {
+      title: `What's ${name}'s Catalog Worth?`,
+      description: `Run a live catalog valuation for ${name} — one click, no uploads, no statements.`,
+      ...(artist?.image ? { images: [{ url: artist.image, width: 640, height: 640 }] } : {}),
+    },
+    twitter: {
</file context>

},
twitter: {
card: "summary_large_image",
title: `What's ${name}'s Catalog Worth?`,
description: `Run a live catalog valuation for ${name} — one click, no uploads, no statements.`,
},
};
}

export default async function ArtistValuationPage({ params }: Props) {
const { artistId } = await params;

return (
<main className="relative pt-36 sm:pt-44 pb-24 min-h-screen">
<div className="mx-auto max-w-[800px] px-6 flex flex-col items-center text-center">
<span
className="inline-flex items-center gap-2.5 mb-5 px-4 py-2 rounded-full text-[12px] uppercase tracking-[0.16em] font-pixel text-(--foreground)/50"
style={{
boxShadow:
"0 0 0 1px color-mix(in srgb, var(--foreground) 15%, transparent)",
}}
>
<span className="w-2 h-2 rounded-full bg-green-500/70 animate-pulse" />
Live catalog valuation
</span>
<h1 className="font-pixel text-[clamp(2.5rem,7vw,4.5rem)] text-(--foreground) leading-[0.95] tracking-[-0.01em] mb-5">
What&apos;s your catalog worth?
</h1>
<p className="text-[clamp(1.0625rem,1.6vw,1.25rem)] text-(--foreground)/60 max-w-[560px] leading-relaxed">
Pick your artist. We measure live play counts across your entire
catalog and turn them into a valuation band — one click, no uploads,
no statements.
</p>
<CatalogValuation initialArtistId={artistId} />
</div>
</main>
);
}
9 changes: 7 additions & 2 deletions components/valuation/CatalogValuation.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,18 @@ import { ArtistSearchBox } from "@/components/search/ArtistSearchBox";
import { ValuationResult } from "@/components/valuation/ValuationResult";
import { toArtist } from "@/lib/valuation/toArtist";

type CatalogValuationProps = {
/** Spotify artist ID from a shareable URL — auto-selects and runs. */
initialArtistId?: string;
};

/**
* The one-click catalog valuation flow: search → run → shareable result card.
* Adapts the shared ArtistSearchBox to the valuation domain inline — Spotify→
* Artist mapping, the clear-while-running guard, and error chrome (chat#1814).
*/
export function CatalogValuation() {
const v = useCatalogValuation();
export function CatalogValuation({ initialArtistId }: CatalogValuationProps = {}) {
const v = useCatalogValuation(initialArtistId);
const running = v.phase === "running";

return (
Expand Down
87 changes: 87 additions & 0 deletions components/valuation/ShareValuation.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
"use client";

import { useState } from "react";
import { formatUsd } from "@/lib/valuation/formatUsd";

type ShareValuationProps = {
artistName: string | null;
artistId: string | null;
centralValue: number;
};

function CopyIcon() {
return (
<svg width="14" height="14" viewBox="0 0 24 24" fill="none" stroke="currentColor" strokeWidth="2" strokeLinecap="round" strokeLinejoin="round">
<rect x="9" y="9" width="13" height="13" rx="2" ry="2" />
<path d="M5 15H4a2 2 0 0 1-2-2V4a2 2 0 0 1 2-2h9a2 2 0 0 1 2 2v1" />
</svg>
);
}

function CheckIcon() {
return (
<svg width="14" height="14" viewBox="0 0 24 24" fill="none" stroke="currentColor" strokeWidth="2" strokeLinecap="round" strokeLinejoin="round">
<polyline points="20 6 9 17 4 12" />
</svg>
);
}

function XIcon() {
return (
<svg width="14" height="14" viewBox="0 0 24 24" fill="currentColor">
<path d="M18.244 2.25h3.308l-7.227 8.26 8.502 11.24H16.17l-5.214-6.817L4.99 21.75H1.68l7.73-8.835L1.254 2.25H8.08l4.713 6.231zm-1.161 17.52h1.833L7.084 4.126H5.117z" />
</svg>
);
}

function LinkedInIcon() {
return (
<svg width="14" height="14" viewBox="0 0 24 24" fill="currentColor">
<path d="M20.447 20.452h-3.554v-5.569c0-1.328-.027-3.037-1.852-3.037-1.853 0-2.136 1.445-2.136 2.939v5.667H9.351V9h3.414v1.561h.046c.477-.9 1.637-1.85 3.37-1.85 3.601 0 4.267 2.37 4.267 5.455v6.286zM5.337 7.433a2.062 2.062 0 0 1-2.063-2.065 2.064 2.064 0 1 1 2.063 2.065zm1.782 13.019H3.555V9h3.564v11.452zM22.225 0H1.771C.792 0 0 .774 0 1.729v20.542C0 23.227.792 24 1.771 24h20.451C23.2 24 24 23.227 24 22.271V1.729C24 .774 23.2 0 22.222 0h.003z" />
</svg>
);
}

export function ShareValuation({ artistName, artistId, centralValue }: ShareValuationProps) {
const [copied, setCopied] = useState(false);

const url = artistId
? `https://recoupable.dev/valuation/${artistId}`

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The domain recoupable.dev is hardcoded here. Share links will always point to production regardless of environment — dev and staging links will redirect users to production, making the share flow untestable locally and creating a maintenance risk if the domain changes. Consider deriving the base URL from an environment variable (e.g., NEXT_PUBLIC_SITE_URL) or a centralized config.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At components/valuation/ShareValuation.tsx, line 49:

<comment>The domain `recoupable.dev` is hardcoded here. Share links will always point to production regardless of environment — dev and staging links will redirect users to production, making the share flow untestable locally and creating a maintenance risk if the domain changes. Consider deriving the base URL from an environment variable (e.g., `NEXT_PUBLIC_SITE_URL`) or a centralized config.</comment>

<file context>
@@ -0,0 +1,87 @@
+  const [copied, setCopied] = useState(false);
+
+  const url = artistId
+    ? `https://recoupable.dev/valuation/${artistId}`
+    : "https://recoupable.dev/valuation";
+  const value = formatUsd(centralValue);
</file context>

: "https://recoupable.dev/valuation";
Comment on lines +48 to +50

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Hardcoded domain violates coding guidelines.

The domain recoupable.dev is hardcoded on lines 49-50. As per coding guidelines, brand values must be imported from lib/config.ts rather than hardcoded. This also means share links will point to production even in development/staging environments.

As per coding guidelines: "Never hardcode brand values; import from lib/config.ts"

♻️ Proposed fix
-  const url = artistId
-    ? `https://recoupable.dev/valuation/${artistId}`
-    : "https://recoupable.dev/valuation";
+  const baseUrl = process.env.NEXT_PUBLIC_SITE_URL ?? "https://recoupable.dev";
+  const url = artistId
+    ? `${baseUrl}/valuation/${artistId}`
+    : `${baseUrl}/valuation`;

Or, if lib/config.ts already exports a site URL constant:

+import { siteUrl } from "`@/lib/config`";
+
 // ...
-  const url = artistId
-    ? `https://recoupable.dev/valuation/${artistId}`
-    : "https://recoupable.dev/valuation";
+  const url = artistId
+    ? `${siteUrl}/valuation/${artistId}`
+    : `${siteUrl}/valuation`;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@components/valuation/ShareValuation.tsx` around lines 48 - 50, The share URL
in ShareValuation is hardcoded to the production domain, which breaks
environment-specific links. Update the URL construction in ShareValuation to use
the site URL value imported from lib/config.ts instead of embedding
recoupable.dev, and keep the artistId path logic unchanged so the share link
works correctly across development, staging, and production.

Source: Coding guidelines

const value = formatUsd(centralValue);
const name = artistName ?? "My artist";

const tweetText = `${name} catalog just got valued at ${value}. What's yours worth?\n\n${url}\n\n#recoup`;
const tweetUrl = `https://twitter.com/intent/tweet?text=${encodeURIComponent(tweetText)}`;
const linkedInUrl = `https://www.linkedin.com/sharing/share-offsite/?url=${encodeURIComponent(url)}`;

async function copyLink() {
try {
await navigator.clipboard.writeText(url);
setCopied(true);
setTimeout(() => setCopied(false), 2000);
} catch {
// Fallback — noop
}
}

const btnClass =
"inline-flex items-center gap-2 px-4 py-2.5 rounded-lg text-[12px] font-pixel uppercase tracking-[0.12em] text-(--foreground)/45 transition-all duration-200 hover:text-(--foreground)/70 hover:bg-(--foreground)/[0.04]";

return (
<div className="mt-6 flex items-center justify-center gap-1">
<button type="button" onClick={copyLink} className={btnClass}>
{copied ? <CheckIcon /> : <CopyIcon />}
{copied ? "Copied" : "Copy link"}
</button>
<a href={tweetUrl} target="_blank" rel="noopener noreferrer" className={btnClass}>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Both share links have the same accessible name ("Share"). Add aria-label="Share on X" and aria-label="Share on LinkedIn" so assistive tech users can distinguish them.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At components/valuation/ShareValuation.tsx, line 77:

<comment>Both share links have the same accessible name ("Share"). Add `aria-label="Share on X"` and `aria-label="Share on LinkedIn"` so assistive tech users can distinguish them.</comment>

<file context>
@@ -0,0 +1,87 @@
+        {copied ? <CheckIcon /> : <CopyIcon />}
+        {copied ? "Copied" : "Copy link"}
+      </button>
+      <a href={tweetUrl} target="_blank" rel="noopener noreferrer" className={btnClass}>
+        <XIcon />
+        Share
</file context>

<XIcon />
Share
</a>
<a href={linkedInUrl} target="_blank" rel="noopener noreferrer" className={btnClass}>
<LinkedInIcon />
Share
</a>
</div>
);
}
2 changes: 2 additions & 0 deletions components/valuation/ValuationResult.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { ArtistHeader } from "@/components/valuation/ArtistHeader";
import { ValuationStats } from "@/components/valuation/ValuationStats";
import { MeasuredCatalog } from "@/components/valuation/MeasuredCatalog";
import { GetFullReportCta } from "@/components/valuation/GetFullReportCta";
import { ShareValuation } from "@/components/valuation/ShareValuation";
import { formatUsd } from "@/lib/valuation/formatUsd";

type ValuationResultProps = {
Expand Down Expand Up @@ -49,6 +50,7 @@ export function ValuationResult({ artist, result, catalogAlbums }: ValuationResu
totalStreams={result.totalStreams}
/>
<GetFullReportCta snapshotId={result.snapshotId} artistName={artist?.name} />
<ShareValuation artistName={artist?.name ?? null} artistId={artist?.id ?? null} centralValue={result.valueBand.central} />
</div>
);
}
30 changes: 29 additions & 1 deletion hooks/useCatalogValuation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import type {
} from "@/components/valuation/types";
import { runValuationFlow } from "@/lib/valuation/runValuationFlow";
import { captureRunLead } from "@/lib/valuation/captureRunLead";
import { fetchSpotifyArtist } from "@/lib/spotify/fetchArtist";
import { toArtist } from "@/lib/valuation/toArtist";

type Phase = "idle" | "running" | "done" | "error";

Expand All @@ -28,10 +30,16 @@ export type CatalogValuationState = {
* Drives the catalog valuation behind the Privy sign-in gate (chat#1798). The
* run trigger opens Privy when signed out and, on login, auto-fires the run for
* the **originally** selected artist. Render inside `PrivyProvider`.
*
* When `initialArtistId` is provided (shareable URL), auto-fetches the artist
* and queues the valuation run on mount.
*/
export function useCatalogValuation(): CatalogValuationState {
export function useCatalogValuation(
initialArtistId?: string,
): CatalogValuationState {
const { authenticated, login, getAccessToken, user } = usePrivy();
const [picked, setPicked] = useState<Artist | null>(null);
const initialFetched = useRef(false);

const [catalogAlbums, setCatalogAlbums] = useState<StartedAlbum[]>([]);
const [phase, setPhase] = useState<Phase>("idle");
Expand Down Expand Up @@ -83,6 +91,26 @@ export function useCatalogValuation(): CatalogValuationState {
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [authenticated]);

// Shareable URL: fetch artist by Spotify ID and auto-trigger the run.
useEffect(() => {
if (!initialArtistId || initialFetched.current) return;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: The initialFetched boolean ref never resets, so if the component instance is reused during client-side navigation (e.g., from /valuation/artistA to /valuation/artistB), the new artist will never be fetched — the page metadata shows artist B but the component still shows artist A's data or an empty state. Track the fetched artist ID instead of a boolean so the effect re-runs when initialArtistId changes.

Additionally, when fetchSpotifyArtist returns null (not found / network error), the effect silently returns with no feedback. Users arriving from a shared link see an empty search box with no indication of what went wrong.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At hooks/useCatalogValuation.ts, line 96:

<comment>The `initialFetched` boolean ref never resets, so if the component instance is reused during client-side navigation (e.g., from `/valuation/artistA` to `/valuation/artistB`), the new artist will never be fetched — the page metadata shows artist B but the component still shows artist A's data or an empty state. Track the fetched artist ID instead of a boolean so the effect re-runs when `initialArtistId` changes.

Additionally, when `fetchSpotifyArtist` returns `null` (not found / network error), the effect silently returns with no feedback. Users arriving from a shared link see an empty search box with no indication of what went wrong.</comment>

<file context>
@@ -83,6 +91,26 @@ export function useCatalogValuation(): CatalogValuationState {
 
+  // Shareable URL: fetch artist by Spotify ID and auto-trigger the run.
+  useEffect(() => {
+    if (!initialArtistId || initialFetched.current) return;
+    initialFetched.current = true;
+    void (async () => {
</file context>

initialFetched.current = true;
void (async () => {
const spotify = await fetchSpotifyArtist(initialArtistId);
if (!spotify) return;
const artist = toArtist(spotify);
setPicked(artist);
// If already authenticated, run immediately; otherwise queue for login.
if (authenticated) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Already-signed-in recipients can land on /valuation/[artistId] and never get the automatic valuation if Privy finishes initializing while the artist fetch is in flight. This effect relies on a captured authenticated value without checking Privy's ready state, so consider waiting for ready and/or using current auth state when queuing the deferred run.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At hooks/useCatalogValuation.ts, line 104:

<comment>Already-signed-in recipients can land on `/valuation/[artistId]` and never get the automatic valuation if Privy finishes initializing while the artist fetch is in flight. This effect relies on a captured `authenticated` value without checking Privy's `ready` state, so consider waiting for `ready` and/or using current auth state when queuing the deferred run.</comment>

<file context>
@@ -83,6 +91,26 @@ export function useCatalogValuation(): CatalogValuationState {
+      const artist = toArtist(spotify);
+      setPicked(artist);
+      // If already authenticated, run immediately; otherwise queue for login.
+      if (authenticated) {
+        void doRun(artist);
+      } else {
</file context>

void doRun(artist);
} else {
pendingRun.current = artist;
login();
}
})();
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [initialArtistId]);
Comment on lines +94 to +112

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

initialFetched boolean ref blocks re-fetch on initialArtistId change, and fetch failures are silent.

Two issues in this effect:

  1. Stale ref guard: initialFetched is a boolean that never resets. If a user navigates client-side from /valuation/artistA to /valuation/artistB, React reuses the component instance (same tree position), so initialFetched.current stays true and artist B is never fetched. The page metadata shows artist B but the component shows artist A's data (or an empty search box). Fix: track the fetched ID, not a boolean.

  2. Silent failure: When fetchSpotifyArtist returns null (not found, network error, or SSRF rejection), the effect returns with no user feedback. Users arriving from a shareable link see an empty search box with no indication the artist wasn't found. Set an error state so the user knows the link didn't resolve.

🐛 Proposed fix: track fetched ID and surface fetch failures
-  const initialFetched = useRef(false);
+  const fetchedArtistId = useRef<string | undefined>(undefined);
   useEffect(() => {
-    if (!initialArtistId || initialFetched.current) return;
-    initialFetched.current = true;
+    if (!initialArtistId || fetchedArtistId.current === initialArtistId) return;
+    fetchedArtistId.current = initialArtistId;
     void (async () => {
       const spotify = await fetchSpotifyArtist(initialArtistId);
-      if (!spotify) return;
+      if (!spotify) {
+        setError("Could not load this artist. Try searching manually.");
+        return;
+      }
       const artist = toArtist(spotify);
       setPicked(artist);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Shareable URL: fetch artist by Spotify ID and auto-trigger the run.
useEffect(() => {
if (!initialArtistId || initialFetched.current) return;
initialFetched.current = true;
void (async () => {
const spotify = await fetchSpotifyArtist(initialArtistId);
if (!spotify) return;
const artist = toArtist(spotify);
setPicked(artist);
// If already authenticated, run immediately; otherwise queue for login.
if (authenticated) {
void doRun(artist);
} else {
pendingRun.current = artist;
login();
}
})();
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [initialArtistId]);
const fetchedArtistId = useRef<string | undefined>(undefined);
// Shareable URL: fetch artist by Spotify ID and auto-trigger the run.
useEffect(() => {
if (!initialArtistId || fetchedArtistId.current === initialArtistId) return;
fetchedArtistId.current = initialArtistId;
void (async () => {
const spotify = await fetchSpotifyArtist(initialArtistId);
if (!spotify) {
setError("Could not load this artist. Try searching manually.");
return;
}
const artist = toArtist(spotify);
setPicked(artist);
// If already authenticated, run immediately; otherwise queue for login.
if (authenticated) {
void doRun(artist);
} else {
pendingRun.current = artist;
login();
}
})();
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [initialArtistId]);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@hooks/useCatalogValuation.ts` around lines 94 - 112, The useEffect in
useCatalogValuation is guarding with initialFetched as a boolean, which prevents
refetching when initialArtistId changes; switch this to track the last fetched
artist ID so a new shareable link can trigger a fresh fetch. Also handle the
null result from fetchSpotifyArtist by setting an error state (instead of
returning silently) so users get feedback when the Spotify artist lookup fails.
Keep the logic around toArtist, setPicked, doRun, and login, but make the effect
depend on the current initialArtistId and the fetched-ID guard.


return {
picked,
catalogAlbums,
Expand Down
17 changes: 17 additions & 0 deletions lib/spotify/fetchArtist.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import type { SpotifyArtist } from "@/lib/spotify/types";

/**
* Fetch a single Spotify artist by ID via the internal API proxy.
* Returns null if not found or on error.
*/
export async function fetchSpotifyArtist(
id: string,
): Promise<SpotifyArtist | null> {
try {
const res = await fetch(`/api/spotify/artist/${id}`);
if (!res.ok) return null;
return (await res.json()) as SpotifyArtist;
} catch {
return null;
}
}
25 changes: 25 additions & 0 deletions lib/spotify/fetchArtistServer.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { siteConfig } from "@/lib/config";

type ArtistInfo = { name: string; image: string | null };

/**
* Server-side fetch of a Spotify artist for metadata generation.
* Calls the Recoup API directly (no internal proxy needed at build time).
*/
export async function fetchArtistServer(
id: string,
): Promise<ArtistInfo | null> {
try {
const res = await fetch(`${siteConfig.apiUrl}/spotify/artist/${id}`, {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Malformed shared URLs can make metadata fetch the wrong Spotify artist because the raw artistId is interpolated directly into the Recoup API path. Encoding the path segment keeps route params from being interpreted as URL syntax.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/spotify/fetchArtistServer.ts, line 13:

<comment>Malformed shared URLs can make metadata fetch the wrong Spotify artist because the raw `artistId` is interpolated directly into the Recoup API path. Encoding the path segment keeps route params from being interpreted as URL syntax.</comment>

<file context>
@@ -0,0 +1,25 @@
+  id: string,
+): Promise<ArtistInfo | null> {
+  try {
+    const res = await fetch(`${siteConfig.apiUrl}/spotify/artist/${id}`, {
+      next: { revalidate: 86400 }, // cache 24h
+    });
</file context>

next: { revalidate: 86400 }, // cache 24h
});
if (!res.ok) return null;
const data = await res.json();
return {
name: data.name ?? "Unknown Artist",
image: data.images?.[0]?.url ?? null,
};
} catch {
return null;
}
}
Comment on lines +9 to +25

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win

Same SSRF vulnerability as the API route — validate id here too.

id arrives from params.artistId (URL-decoded by Next.js) and is interpolated directly into the fetch URL. The same path-traversal exploit applies. Add the same Zod validation as in the route handler. As per coding guidelines, use Zod for all validation in **/*.{ts,tsx,js,jsx} files.

🔒 Proposed fix
+import { z } from "zod";
+
 export async function fetchArtistServer(
   id: string,
 ): Promise<ArtistInfo | null> {
+  const parsed = z.string().regex(/^[a-zA-Z0-9]+$/).safeParse(id);
+  if (!parsed.success) return null;
+  const safeId = parsed.data;
   try {
-    const res = await fetch(`${siteConfig.apiUrl}/spotify/artist/${id}`, {
+    const res = await fetch(`${siteConfig.apiUrl}/spotify/artist/${safeId}`, {
       next: { revalidate: 86400 },
     });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export async function fetchArtistServer(
id: string,
): Promise<ArtistInfo | null> {
try {
const res = await fetch(`${siteConfig.apiUrl}/spotify/artist/${id}`, {
next: { revalidate: 86400 }, // cache 24h
});
if (!res.ok) return null;
const data = await res.json();
return {
name: data.name ?? "Unknown Artist",
image: data.images?.[0]?.url ?? null,
};
} catch {
return null;
}
}
import { z } from "zod";
export async function fetchArtistServer(
id: string,
): Promise<ArtistInfo | null> {
const parsed = z.string().regex(/^[a-zA-Z0-9]+$/).safeParse(id);
if (!parsed.success) return null;
const safeId = parsed.data;
try {
const res = await fetch(`${siteConfig.apiUrl}/spotify/artist/${safeId}`, {
next: { revalidate: 86400 }, // cache 24h
});
if (!res.ok) return null;
const data = await res.json();
return {
name: data.name ?? "Unknown Artist",
image: data.images?.[0]?.url ?? null,
};
} catch {
return null;
}
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/spotify/fetchArtistServer.ts` around lines 9 - 25, The fetchArtistServer
function interpolates id directly into the Spotify fetch URL, so add the same
Zod-based validation used in the API route before building the request. Validate
the params.artistId-derived id at the start of fetchArtistServer, return null
for invalid values, and only proceed to fetch when the Zod check passes to
prevent path-traversal/SSRF-style inputs.

Source: Coding guidelines