feat(valuation): shareable result URLs — /valuation/[artistId]#48
feat(valuation): shareable result URLs — /valuation/[artistId]#48sidneyswift wants to merge 3 commits into
Conversation
Adds copy-link, X, and LinkedIn share buttons below the 'Get full report' CTA on valuation result cards. Each share pre-fills with the artist name and valuation figure, linking back to /valuation to drive organic traffic. Design matches existing system: font-pixel labels, shadow-as-border, foreground opacity modifiers, muted hover states.
Each valuation result now has a unique URL: /valuation/[artistId] - Dynamic route with server-side OG metadata (title, description, image) - Auto-fetches artist info and triggers valuation on page load - Share buttons now link to artist-specific URLs instead of generic /valuation - Spotify artist lookup API route for server + client artist fetching - Existing /valuation route unchanged — zero regressions When a label exec shares their $4.7M valuation on LinkedIn or X, the link takes recipients directly to that artist's valuation — not a blank search. Every share becomes a complete viral loop.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds a Spotify artist proxy API route and server/client fetch helpers, a dynamic artist valuation landing page with SEO metadata, hook logic to auto-fetch and auto-run valuation from a URL-provided artist id, and a new ShareValuation component wired into ValuationResult for sharing via link/Twitter/LinkedIn. ChangesShareable Artist Valuation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Page as ArtistValuationPage
participant CV as CatalogValuation
participant Hook as useCatalogValuation
participant API as fetchSpotifyArtist
participant Auth as login/authenticated
Page->>CV: initialArtistId
CV->>Hook: useCatalogValuation(initialArtistId)
Hook->>API: fetchSpotifyArtist(initialArtistId)
API-->>Hook: SpotifyArtist or null
Hook->>Hook: set picked artist
alt authenticated
Hook->>Hook: doRun()
else not authenticated
Hook->>Auth: login()
Auth-->>Hook: pendingRun executed
end
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
components/valuation/ShareValuation.tsx (2)
77-84: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDisambiguate share link labels for accessibility.
Both social links use the same "Share" label, making them indistinguishable for screen reader users. Use platform-specific labels like "Share on X" and "Share on LinkedIn".
♿ Proposed fix
<a href={tweetUrl} target="_blank" rel="noopener noreferrer" className={btnClass}> <XIcon /> - Share + Share on X </a> <a href={linkedInUrl} target="_blank" rel="noopener noreferrer" className={btnClass}> <LinkedInIcon /> - Share + Share on LinkedIn </a>🤖 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 77 - 84, The share links in ShareValuation.tsx use the same generic label, which makes the two anchors indistinguishable to assistive tech. Update the anchor text or add accessible labels for the XIcon and LinkedInIcon links so they are clearly identified as platform-specific actions, using distinct wording like “Share on X” and “Share on LinkedIn” in the ShareValuation component.
58-66: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winClipboard copy silently fails with no user feedback.
The catch block is a noop — if
navigator.clipboardis unavailable (non-secure context, older browser) the user gets no indication the copy failed. Consider showing a fallback state or using a temporary input element for broader compatibility.♿ Proposed fallback
async function copyLink() { try { await navigator.clipboard.writeText(url); setCopied(true); setTimeout(() => setCopied(false), 2000); } catch { - // Fallback — noop + // Fallback for non-secure contexts + const input = document.createElement("input"); + input.value = url; + document.body.appendChild(input); + input.select(); + document.execCommand("copy"); + document.body.removeChild(input); + setCopied(true); + setTimeout(() => setCopied(false), 2000); } }🤖 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 58 - 66, The copyLink() handler currently swallows clipboard failures with a noop catch, so ShareValuation gives no feedback when navigator.clipboard.writeText(url) is unavailable. Update copyLink() to provide a visible fallback or failure state in the catch block (for example by using a temporary input-based copy fallback and/or setting a user-facing error/copied state) so the user can tell the copy action did not succeed.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@app/api/spotify/artist/`[id]/route.ts:
- Around line 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.
- Around line 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.
In `@components/valuation/ShareValuation.tsx`:
- Around line 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.
In `@hooks/useCatalogValuation.ts`:
- Around line 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.
In `@lib/spotify/fetchArtistServer.ts`:
- Around line 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.
---
Nitpick comments:
In `@components/valuation/ShareValuation.tsx`:
- Around line 77-84: The share links in ShareValuation.tsx use the same generic
label, which makes the two anchors indistinguishable to assistive tech. Update
the anchor text or add accessible labels for the XIcon and LinkedInIcon links so
they are clearly identified as platform-specific actions, using distinct wording
like “Share on X” and “Share on LinkedIn” in the ShareValuation component.
- Around line 58-66: The copyLink() handler currently swallows clipboard
failures with a noop catch, so ShareValuation gives no feedback when
navigator.clipboard.writeText(url) is unavailable. Update copyLink() to provide
a visible fallback or failure state in the catch block (for example by using a
temporary input-based copy fallback and/or setting a user-facing error/copied
state) so the user can tell the copy action did not succeed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8e464f0d-d7c7-4856-ae13-8e9d16c15fec
📒 Files selected for processing (8)
app/api/spotify/artist/[id]/route.tsapp/valuation/[artistId]/page.tsxcomponents/valuation/CatalogValuation.tsxcomponents/valuation/ShareValuation.tsxcomponents/valuation/ValuationResult.tsxhooks/useCatalogValuation.tslib/spotify/fetchArtist.tslib/spotify/fetchArtistServer.ts
| const { id } = await params; | ||
| if (!id) { | ||
| return NextResponse.json({ error: "missing id" }, { status: 400 }); | ||
| } | ||
|
|
||
| try { | ||
| const res = await fetch(`${siteConfig.apiUrl}/spotify/artist/${id}`); |
There was a problem hiding this comment.
🔒 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.
| 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
| try { | ||
| const res = await fetch(`${siteConfig.apiUrl}/spotify/artist/${id}`); |
There was a problem hiding this comment.
🩺 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.
| const url = artistId | ||
| ? `https://recoupable.dev/valuation/${artistId}` | ||
| : "https://recoupable.dev/valuation"; |
There was a problem hiding this comment.
📐 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
| // 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]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
initialFetched boolean ref blocks re-fetch on initialArtistId change, and fetch failures are silent.
Two issues in this effect:
-
Stale ref guard:
initialFetchedis a boolean that never resets. If a user navigates client-side from/valuation/artistAto/valuation/artistB, React reuses the component instance (same tree position), soinitialFetched.currentstaystrueand 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. -
Silent failure: When
fetchSpotifyArtistreturnsnull(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.
| // 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.
| 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; | ||
| } | ||
| } |
There was a problem hiding this comment.
🔒 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.
| 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
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
10 issues found across 8 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/api/spotify/artist/[id]/route.ts">
<violation number="1" location="app/api/spotify/artist/[id]/route.ts:20">
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.</violation>
<violation number="2" location="app/api/spotify/artist/[id]/route.ts:20">
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.</violation>
<violation number="3" location="app/api/spotify/artist/[id]/route.ts:21">
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.</violation>
<violation number="4" location="app/api/spotify/artist/[id]/route.ts:25">
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`.</violation>
</file>
<file name="lib/spotify/fetchArtistServer.ts">
<violation number="1" location="lib/spotify/fetchArtistServer.ts:13">
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.</violation>
</file>
<file name="hooks/useCatalogValuation.ts">
<violation number="1" location="hooks/useCatalogValuation.ts:96">
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.</violation>
<violation number="2" location="hooks/useCatalogValuation.ts:104">
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.</violation>
</file>
<file name="app/valuation/[artistId]/page.tsx">
<violation number="1" location="app/valuation/[artistId]/page.tsx:20">
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.</violation>
</file>
<file name="components/valuation/ShareValuation.tsx">
<violation number="1" location="components/valuation/ShareValuation.tsx:49">
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.</violation>
<violation number="2" location="components/valuation/ShareValuation.tsx:77">
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.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| } | ||
|
|
||
| try { | ||
| const res = await fetch(`${siteConfig.apiUrl}/spotify/artist/${id}`); |
There was a problem hiding this comment.
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>
| const artist = toArtist(spotify); | ||
| setPicked(artist); | ||
| // If already authenticated, run immediately; otherwise queue for login. | ||
| if (authenticated) { |
There was a problem hiding this comment.
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>
|
|
||
| // Shareable URL: fetch artist by Spotify ID and auto-trigger the run. | ||
| useEffect(() => { | ||
| if (!initialArtistId || initialFetched.current) return; |
There was a problem hiding this comment.
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>
| } | ||
|
|
||
| try { | ||
| const res = await fetch(`${siteConfig.apiUrl}/spotify/artist/${id}`); |
There was a problem hiding this comment.
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>
| id: string, | ||
| ): Promise<ArtistInfo | null> { | ||
| try { | ||
| const res = await fetch(`${siteConfig.apiUrl}/spotify/artist/${id}`, { |
There was a problem hiding this comment.
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>
|
|
||
| try { | ||
| const res = await fetch(`${siteConfig.apiUrl}/spotify/artist/${id}`); | ||
| if (!res.ok) { |
There was a problem hiding this comment.
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>
| const [copied, setCopied] = useState(false); | ||
|
|
||
| const url = artistId | ||
| ? `https://recoupable.dev/valuation/${artistId}` |
There was a problem hiding this comment.
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>
| return NextResponse.json({ error: "not found" }, { status: 404 }); | ||
| } | ||
|
|
||
| const a = await res.json(); |
There was a problem hiding this comment.
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>
| 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 }] } : {}), |
There was a problem hiding this comment.
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>
| {copied ? <CheckIcon /> : <CopyIcon />} | ||
| {copied ? "Copied" : "Copy link"} | ||
| </button> | ||
| <a href={tweetUrl} target="_blank" rel="noopener noreferrer" className={btnClass}> |
There was a problem hiding this comment.
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>
1 participant
What
Every valuation result now has a unique, shareable URL:
/valuation/[artistId]When someone shares their catalog valuation on X or LinkedIn, the link takes recipients directly to that artist's valuation page — with OG metadata (title, description, artist image) for rich social previews.
Why
Last night's share buttons (PR #47) link to generic
/valuation. Recipients land on a blank search page. The viral loop is broken at the most critical moment — right when someone is impressed enough to share.Changes
app/valuation/[artistId]/page.tsx— Dynamic route withgenerateMetadatafor artist-specific OG tagsapp/api/spotify/artist/[id]/route.ts— Server route to fetch single Spotify artist by IDlib/spotify/fetchArtist.ts— Client-side artist fetch for auto-select on loadlib/spotify/fetchArtistServer.ts— Server-side artist fetch for metadata generation (24h cache)hooks/useCatalogValuation.ts— AcceptsinitialArtistId, auto-selects and auto-runs on mountcomponents/valuation/CatalogValuation.tsx— PassesinitialArtistIdprop throughcomponents/valuation/ShareValuation.tsx— Share URLs now point to/valuation/{artistId}components/valuation/ValuationResult.tsx— PassesartistIdto share componentHow it works
/valuation/[artistId]→ auto-fetches artist → Privy auth gate → auto-runs valuationTesting
@privy-io/react-auth+@vercel/analytics/nextmissing dep (not from this PR)/valuationroute unchangedSummary by cubic
Add shareable valuation result URLs at /valuation/[artistId] with artist-specific OG previews. Shared links now open directly to that artist’s valuation and auto-run after the Privy gate.
Written for commit 1343534. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes