Skip to content

feat(referrals): invite links via GET /api/v1/referrals and dashboard UX#420

Open
odilitime wants to merge 3 commits intodevfrom
odi-ref
Open

feat(referrals): invite links via GET /api/v1/referrals and dashboard UX#420
odilitime wants to merge 3 commits intodevfrom
odi-ref

Conversation

@odilitime
Copy link
Copy Markdown
Contributor

@odilitime odilitime commented Mar 30, 2026

Add authenticated GET endpoint that idempotently ensures a referral code and returns flat JSON for clients. Surface copy-to-clipboard invite links in the dashboard header and on the Affiliates page, with distinct styling and copy from affiliates.

Return 403 for ForbiddenError, block sharing inactive codes, and hide the header Invite control during auth grace. Extend e2e coverage for the new GET route.

Document flows in docs/referrals.md and docs/affiliate-referral-comparison.md, update README, ROADMAP, and changelog, and add WHY-oriented code comments.

Made-with: Cursor


Note

Medium Risk
Adds a new authenticated v1 endpoint that can create DB rows on GET and is consumed by new dashboard UI surfaces, so failures could affect signup attribution/sharing flows. Additional changes touch shared URL construction/clipboard utilities and test infrastructure, but are otherwise contained.

Overview
Adds referral invite link surfacing end-to-end. Introduces GET /api/v1/referrals (CORS + rate-limited, force-dynamic) that idempotently ensures a user has a referral_codes row and returns flat { code, total_referrals, is_active }, with explicit 401 vs 403 handling.

Updates dashboard UX to expose referral links without conflating with affiliates. Adds a header Invite button (hidden during authGraceActive) that fetches on click, blocks copying inactive codes, and uses a shared clipboard helper; adds an “Invite friends” card on /dashboard/affiliates, renames the page header to “Affiliates & Referrals,” and updates affiliate copy/cURL snippet to use the same clipboard fallback.

Adds shared helpers + tests/docs. Introduces ReferralMeResponse parsing/coercion, fetchReferralMe, buildReferralInviteLoginUrl, copyTextToClipboard, and a shared copy-feedback constant; expands e2e/unit coverage for the new route and parsing; pins bun test concurrency to 1 to avoid global mock bleed; updates docs/README/roadmap/changelog and tweaks getMiladyAgentPublicWebUiUrl call sites/options handling.

Written by Cursor Bugbot for commit 24d7200. This will update automatically on new commits. Configure here.

Summary by CodeRabbit

Release Notes

  • New Features

    • Added referral invite links—users can copy and share personal invitation URLs from the dashboard "Invite" button and the Affiliates page.
    • Inactive referral codes display warning states and prevent copying.
  • Documentation

    • Comprehensive referral program documentation added, including API behavior, referral vs. affiliate program comparison, and UI implementation details.

Add authenticated GET endpoint that idempotently ensures a referral code and returns
flat JSON for clients. Surface copy-to-clipboard invite links in the dashboard
header and on the Affiliates page, with distinct styling and copy from affiliates.

Return 403 for ForbiddenError, block sharing inactive codes, and hide the header
Invite control during auth grace. Extend e2e coverage for the new GET route.

Document flows in docs/referrals.md and docs/affiliate-referral-comparison.md,
update README, ROADMAP, and changelog, and add WHY-oriented code comments.

Made-with: Cursor
Copilot AI review requested due to automatic review settings March 30, 2026 04:16
@vercel
Copy link
Copy Markdown

vercel bot commented Mar 30, 2026

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

Project Deployment Actions Updated (UTC)
eliza-cloud-v2 Ready Ready Preview, Comment Mar 30, 2026 6:12am

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 30, 2026

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and specifically summarizes the main change: introducing invite links via a new GET /api/v1/referrals endpoint and corresponding dashboard UI components.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch odi-ref

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@claude
Copy link
Copy Markdown

claude bot commented Mar 30, 2026

Code Review — PR #420: Referral Invite Links

Overall this is clean and well-documented. The idempotent GET, rate limiting, CORS, and in-flight dedupe patterns are solid. A few issues worth addressing before merge:


Bug: Missing try/catch for network errors in affiliates-page-client.tsx

The loadReferral async function only handles !res.ok (HTTP errors) but has no try/catch for network-level failures (DNS error, connection refused, etc.). A network throw will leave loadingReferral as true indefinitely since the finally path is never reached.

// packages/ui/src/components/affiliates/affiliates-page-client.tsx
const loadReferral = async () => {
  setLoadingReferral(true);
  setReferralFetchFailed(false);
  // ❌ No try/catch — a network error leaves loadingReferral=true forever
  const res = await fetch("/api/v1/referrals", { method: "GET", credentials: "include" });

Fix: wrap in try/catch mirroring the error-path logic.


Fragile: isAuthError string-matching instead of instanceof

app/api/v1/referrals/route.ts detects auth errors by matching error message strings:

error.message.includes("Unauthorized") ||
error.message.includes("Authentication required") ||
...

This silently breaks if any upstream message changes wording (e.g. during a dependency update). The existing codebase uses typed error classes — check if AuthenticationError (or equivalent) is exported from @/lib/api/errors. If so, error instanceof AuthenticationError is more robust and consistent with how ForbiddenError is handled in the same catch block.


REST semantics: GET that creates a DB row

GET /api/v1/referrals calls getOrCreateCode which inserts a row on first use. This violates the HTTP safe-method contract (GET must have no side effects) and means browser prefetching, CDN warming, or health checks could silently create referral codes. force-dynamic prevents caching but does not prevent external callers from triggering creation.

If creation-on-read is intentional (acknowledged in comments), the risk is low given rate limiting and auth requirements. Worth a note that automated scanners or link-preview services with auth cookies could trigger this. Alternatively, a POST /api/v1/referrals with GET for read-only retrieval avoids the semantic issue entirely — but understand this is a deliberate design trade-off.


Potential race condition in getOrCreateCode

If two concurrent authenticated requests arrive for a new user before either completes the insert:

const existing = await referralCodesRepository.findByUserId(userId);
if (existing) return existing;
// ← gap: second request also hits here before first insert commits
return await referralCodesRepository.create(...);

Both could attempt to insert. Whether this is safe depends on whether referral_codes has a UNIQUE constraint on user_id and whether the repository handles ON CONFLICT. Worth verifying — if not, a duplicate-key error could surface as a 500.


Minor: Duplicate URL construction

${window.location.origin}/login?ref=${encodeURIComponent(referralMe.code)} is built in three places in affiliates-page-client.tsx (inactive display, active display, copy handler) and once in header-invite-button.tsx. Consider a shared helper buildReferralUrl(code: string): string to avoid drift if the URL shape changes.


Minor: Stale is_active in header button cache

HeaderInviteButton caches the ReferralMeResponse in cachedRef indefinitely. If an admin deactivates a user's code mid-session, the header will continue showing the button and only block on copy (toast). This is acceptable UX, but worth documenting as a known limitation.


Positive notes

  • The inFlightRef dedupe pattern in HeaderInviteButton is a good call.
  • parseReferralMeResponse failing closed on bad shapes is the right approach for untrusted fetch JSON.
  • authGraceActive guard in the header correctly prevents 401s during session settlement.
  • E2E test verifying idempotency (two GETs return the same code) is exactly the right thing to test here.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a first-class “invite link” (referral) surface to the dashboard by introducing an authenticated GET /api/v1/referrals endpoint (idempotently ensuring a referral code) and wiring it into the header + Affiliates page UI, with supporting docs and e2e coverage.

Changes:

  • Add GET /api/v1/referrals route returning flat { code, total_referrals, is_active } and mapping ForbiddenError to 403.
  • Add dashboard UX for copying invite links (header Invite button + “Invite friends” card on Affiliates page) and differentiate referral vs affiliate messaging/styling.
  • Add shared client response type/parser and extend e2e tests + documentation/changelog/roadmap/readme updates.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
packages/ui/src/components/layout/header.tsx Adds header Invite control gated on authGraceActive.
packages/ui/src/components/layout/header-invite-button.tsx New client component to lazy-fetch referral “me” payload and copy invite URL.
packages/ui/src/components/affiliates/affiliates-page-wrapper.tsx Updates page header copy to include referrals.
packages/ui/src/components/affiliates/affiliates-page-client.tsx Adds “Invite friends” referral card with its own fetch + copy UX.
packages/tests/e2e/v1/affiliates.test.ts Adds e2e coverage for auth + shape/idempotency of GET /api/v1/referrals.
packages/lib/types/referral-me.ts Introduces ReferralMeResponse and parseReferralMeResponse for client-side validation.
packages/lib/services/referrals.ts Documents getOrCreateCode as the backing behavior for the new GET endpoint.
packages/content/changelog.mdx Changelog entry describing the new referral invite links API + UX.
docs/referrals.md Full referral/invite link flow + API documentation and rationale.
docs/affiliate-referral-comparison.md Quick comparison doc to reduce confusion between programs.
docs/ROADMAP.md Marks referral invite links work as done and notes follow-ups.
app/api/v1/referrals/route.ts Implements the new authenticated GET endpoint (+ OPTIONS, rate-limit, CORS).
README.md Updates product documentation around invite links and the new endpoint.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +84 to +91
await navigator.clipboard.writeText(url).then(
() => {
toast.success("Invite link copied!");
},
() => {
toast.error("Could not copy to clipboard");
},
);
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

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

navigator.clipboard.writeText can be undefined or throw synchronously in unsupported/non-secure contexts. Right now the call is not guarded, so clicking Invite could throw before the .then(...) handlers run. Consider checking for navigator.clipboard?.writeText (and possibly window.isSecureContext) and surfacing a toast error/fallback when unavailable (similar to CodeBlockCopyButton’s guard pattern).

Suggested change
await navigator.clipboard.writeText(url).then(
() => {
toast.success("Invite link copied!");
},
() => {
toast.error("Could not copy to clipboard");
},
);
if (
!window.isSecureContext ||
typeof navigator === "undefined" ||
!navigator.clipboard ||
typeof navigator.clipboard.writeText !== "function"
) {
toast.error("Clipboard copy is not available in this context");
return;
}
try {
await navigator.clipboard.writeText(url);
toast.success("Invite link copied!");
} catch {
toast.error("Could not copy to clipboard");
}

Copilot uses AI. Check for mistakes.
Comment on lines +82 to +99
const res = await fetch("/api/v1/referrals", { method: "GET", credentials: "include" });
if (cancelled) return;
if (!res.ok) {
setReferralFetchFailed(true);
setReferralMe(null);
setLoadingReferral(false);
return;
}
const json: unknown = await res.json();
const parsed = parseReferralMeResponse(json);
if (cancelled) return;
if (!parsed) {
setReferralFetchFailed(true);
setReferralMe(null);
} else {
setReferralMe(parsed);
}
setLoadingReferral(false);
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

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

loadReferral doesn’t have a try/catch/finally. If fetch fails (network) or res.json() throws (invalid JSON), the promise will reject and loadingReferral will never be set back to false, leaving the UI stuck in the skeleton state. Wrap the whole body in try/catch/finally and ensure setLoadingReferral(false) runs in finally (respecting cancelled).

Suggested change
const res = await fetch("/api/v1/referrals", { method: "GET", credentials: "include" });
if (cancelled) return;
if (!res.ok) {
setReferralFetchFailed(true);
setReferralMe(null);
setLoadingReferral(false);
return;
}
const json: unknown = await res.json();
const parsed = parseReferralMeResponse(json);
if (cancelled) return;
if (!parsed) {
setReferralFetchFailed(true);
setReferralMe(null);
} else {
setReferralMe(parsed);
}
setLoadingReferral(false);
try {
const res = await fetch("/api/v1/referrals", {
method: "GET",
credentials: "include",
});
if (cancelled) return;
if (!res.ok) {
setReferralFetchFailed(true);
setReferralMe(null);
return;
}
const json: unknown = await res.json();
const parsed = parseReferralMeResponse(json);
if (cancelled) return;
if (!parsed) {
setReferralFetchFailed(true);
setReferralMe(null);
} else {
setReferralMe(parsed);
}
} catch (error) {
if (cancelled) return;
setReferralFetchFailed(true);
setReferralMe(null);
} finally {
if (!cancelled) {
setLoadingReferral(false);
}
}

Copilot uses AI. Check for mistakes.
toast.error("Could not build invite link");
return;
}
const url = `${origin}/login?ref=${encodeURIComponent(referralMe.code)}`;
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

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

The referral Copy button uses navigator.clipboard.writeText without checking API availability, so it can throw synchronously in unsupported/non-secure contexts. Add a guard (e.g. if (!navigator.clipboard?.writeText)) and show a toast error or fallback copy method.

Suggested change
const url = `${origin}/login?ref=${encodeURIComponent(referralMe.code)}`;
const url = `${origin}/login?ref=${encodeURIComponent(referralMe.code)}`;
if (typeof navigator === "undefined" || !navigator.clipboard?.writeText) {
toast.error("Copy to clipboard is not supported in this browser");
return;
}

Copilot uses AI. Check for mistakes.
Comment on lines +20 to +21
const totalReferrals = typeof tr === "number" ? tr : Number(tr);
if (!Number.isFinite(totalReferrals)) return null;
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

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

parseReferralMeResponse currently coerces total_referrals with Number(tr), which will accept some non-numeric shapes as valid (e.g. an empty string becomes 0). If the goal is to fail closed on wrong shapes, consider only accepting a number or a non-empty numeric string (and optionally enforcing an integer >= 0).

Suggested change
const totalReferrals = typeof tr === "number" ? tr : Number(tr);
if (!Number.isFinite(totalReferrals)) return null;
let totalReferrals: number;
if (typeof tr === "number") {
if (!Number.isFinite(tr) || !Number.isInteger(tr) || tr < 0) return null;
totalReferrals = tr;
} else if (typeof tr === "string") {
const trimmed = tr.trim();
if (trimmed.length === 0 || !/^\d+$/.test(trimmed)) return null;
const parsed = Number(trimmed);
if (!Number.isFinite(parsed) || !Number.isInteger(parsed) || parsed < 0) return null;
totalReferrals = parsed;
} else {
return null;
}

Copilot uses AI. Check for mistakes.
Comment on lines +26 to +36
/** WHY broad message match: Session + API key auth throw varied `AuthenticationError` messages; all should map to 401 for this route. */
function isAuthError(error: unknown): boolean {
if (!(error instanceof Error)) return false;
return (
error.message.includes("Unauthorized") ||
error.message.includes("Authentication required") ||
error.message.includes("Invalid or expired token") ||
error.message.includes("Invalid or expired API key") ||
error.message.includes("Invalid wallet signature") ||
error.message.includes("Wallet authentication failed")
);
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

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

isAuthError relies on matching a few message substrings, but requireAuthOrApiKeyWithOrg can throw AuthenticationError with other messages (e.g. "Invalid authorization header", "User not found"). Those will currently fall through to the 500 handler instead of returning 401. Consider handling AuthenticationError via instanceof (or expanding the matcher) so all auth failures consistently map to 401 as documented.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🧹 Nitpick comments (1)
packages/ui/src/components/affiliates/affiliates-page-client.tsx (1)

90-91: Remove the explicit unknown type annotation.

The parseReferralMeResponse(json) function already declares unknown as its input parameter type and validates the structure. The explicit type annotation on line 90 is redundant and contradicts the repo's TypeScript guidelines.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/ui/src/components/affiliates/affiliates-page-client.tsx` around
lines 90 - 91, Remove the redundant explicit "unknown" annotation on the fetched
response: change the declaration of the local variable "json" (currently "const
json: unknown = await res.json()") to let TypeScript infer the type (e.g.,
"const json = await res.json()"); keep the subsequent call to
parseReferralMeResponse(json) which already accepts and validates unknown, so no
other changes to parseReferralMeResponse are required.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@app/api/v1/referrals/route.ts`:
- Around line 59-67: The code currently masks malformed row.total_referrals by
replacing non-finite values with 0 when building the ReferralMeResponse;
instead, change the logic in the block that computes totalReferrals (and where
the body is created for ReferralMeResponse) to validate row.total_referrals and
throw a clear error if it cannot be converted to a finite number (e.g., when
Number.isFinite(totalReferrals) is false) so the existing catch will return a
500 rather than silently returning 0; reference the totalReferrals variable, the
row.total_referrals source, and the ReferralMeResponse construction to locate
and update the code.
- Around line 26-37: Replace fragile string-based checks in isAuthError with a
proper type check for the concrete AuthenticationError thrown by
requireAuthOrApiKeyWithOrg: have isAuthError accept Error (not unknown) and
return true only when error instanceof AuthenticationError (import or reference
the AuthenticationError type used by requireAuthOrApiKeyWithOrg). Also remove
the silent fallback to 0 for total_referrals: where total_referrals is validated
(the code that currently uses total_referrals ?? 0), validate that
Number.isFinite(total_referrals) and throw a descriptive Error if it is not
finite so the route fails fast on corrupted upstream data. Ensure you update
imports/usages to reference AuthenticationError and adjust any callers expecting
the old boolean behavior.

In `@packages/lib/types/referral-me.ts`:
- Around line 18-26: The current conversion uses Number(tr) which coerces values
like "", null, true, and [] into numbers; tighten validation in the block that
computes totalReferrals (where tr = o.total_referrals and totalReferrals is set
via Number(tr)) so malformed payloads are rejected: accept only a numeric type
or a numeric-string (e.g., match /^\d+$/ or /^\d+(\.\d+)?$/ as appropriate),
parse with Number/parseInt after that check, verify Number.isFinite and (if
integers expected) Number.isInteger and non-negative, and return null for any
other types (including boolean, null, empty string, or arrays). Keep the
existing checks for o.code and o.is_active but replace the loose Number(tr)
coercion with the stricter validation and assignment to totalReferrals.

In `@packages/ui/src/components/affiliates/affiliates-page-client.tsx`:
- Line 101: The call to void loadReferral() starts an async fetch that can throw
(fetch, res.json, or parseReferralMeResponse) and currently has no rejection
handling so loadingReferral may never be set to false; modify loadReferral (and
any async helpers like parseReferralMeResponse) to use try/catch/finally (or
attach .catch/.finally) so that on any error you call setLoadingReferral(false)
in the finally block and optionally set an error state or log the error; ensure
the function name loadReferral and state setter setLoadingReferral (and
parseReferralMeResponse if applicable) are the focal points for this change.

In `@packages/ui/src/components/layout/header-invite-button.tsx`:
- Around line 24-25: The cachedRef persistence causes stale is_active decisions;
before performing the copy action, always revalidate current referral state
instead of relying solely on cachedRef: if an in-flight fetch (inFlightRef)
exists await it, otherwise trigger a fresh fetch of the referral status, update
cachedRef with the fresh ReferralMeResponse, then check is_active and proceed to
copy only if active (or abort if inactive); ensure any copy handler (where
cachedRef and inFlightRef are used) clears/updates cachedRef on error or after
explicit refresh so future clicks use the latest state.

---

Nitpick comments:
In `@packages/ui/src/components/affiliates/affiliates-page-client.tsx`:
- Around line 90-91: Remove the redundant explicit "unknown" annotation on the
fetched response: change the declaration of the local variable "json" (currently
"const json: unknown = await res.json()") to let TypeScript infer the type
(e.g., "const json = await res.json()"); keep the subsequent call to
parseReferralMeResponse(json) which already accepts and validates unknown, so no
other changes to parseReferralMeResponse are required.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9ea55a1d-fd90-4141-a053-05762ac6d206

📥 Commits

Reviewing files that changed from the base of the PR and between e7c31f1 and b4be8ca.

📒 Files selected for processing (13)
  • README.md
  • app/api/v1/referrals/route.ts
  • docs/ROADMAP.md
  • docs/affiliate-referral-comparison.md
  • docs/referrals.md
  • packages/content/changelog.mdx
  • packages/lib/services/referrals.ts
  • packages/lib/types/referral-me.ts
  • packages/tests/e2e/v1/affiliates.test.ts
  • packages/ui/src/components/affiliates/affiliates-page-client.tsx
  • packages/ui/src/components/affiliates/affiliates-page-wrapper.tsx
  • packages/ui/src/components/layout/header-invite-button.tsx
  • packages/ui/src/components/layout/header.tsx

Comment on lines +26 to +37
/** WHY broad message match: Session + API key auth throw varied `AuthenticationError` messages; all should map to 401 for this route. */
function isAuthError(error: unknown): boolean {
if (!(error instanceof Error)) return false;
return (
error.message.includes("Unauthorized") ||
error.message.includes("Authentication required") ||
error.message.includes("Invalid or expired token") ||
error.message.includes("Invalid or expired API key") ||
error.message.includes("Invalid wallet signature") ||
error.message.includes("Wallet authentication failed")
);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect the auth helper and auth-specific error types thrown today.
rg -n -C3 '\brequireAuthOrApiKeyWithOrg\b'
rg -n -C3 'class\s+\w*(Auth|Authentication)\w*Error\b|throw new \w*(Auth|Authentication)\w*Error\(|class ForbiddenError\b'

Repository: elizaOS/cloud

Length of output: 50370


🏁 Script executed:

# Find auth error definitions
find . -name "*.ts" -path "*/lib/*" \( -name "*error*" -o -name "*auth*" \) | head -20

Repository: elizaOS/cloud

Length of output: 955


🏁 Script executed:

# Get error class definitions
rg -A3 'class.*Error\b' packages/lib/auth.ts packages/lib/api/errors.ts 2>/dev/null | head -100

Repository: elizaOS/cloud

Length of output: 2151


🏁 Script executed:

# View the full referrals route file
cat -n app/api/v1/referrals/route.ts

Repository: elizaOS/cloud

Length of output: 4121


Replace message matching with a typed error check, and validate data instead of falling back to defaults.

Lines 26–37: isAuthError() relies on message fragments and accepts unknown, which violates the guideline to always use specific types. Since requireAuthOrApiKeyWithOrg throws AuthenticationError (a concrete class), use error instanceof AuthenticationError instead.

Lines 59–67: Silently falling back to 0 when total_referrals is not finite hides upstream data corruption. Throw an error instead to fail fast.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/api/v1/referrals/route.ts` around lines 26 - 37, Replace fragile
string-based checks in isAuthError with a proper type check for the concrete
AuthenticationError thrown by requireAuthOrApiKeyWithOrg: have isAuthError
accept Error (not unknown) and return true only when error instanceof
AuthenticationError (import or reference the AuthenticationError type used by
requireAuthOrApiKeyWithOrg). Also remove the silent fallback to 0 for
total_referrals: where total_referrals is validated (the code that currently
uses total_referrals ?? 0), validate that Number.isFinite(total_referrals) and
throw a descriptive Error if it is not finite so the route fails fast on
corrupted upstream data. Ensure you update imports/usages to reference
AuthenticationError and adjust any callers expecting the old boolean behavior.

Comment on lines +59 to +67
const totalReferrals =
typeof row.total_referrals === "number"
? row.total_referrals
: Number(row.total_referrals);

const body: ReferralMeResponse = {
code: row.code,
total_referrals: Number.isFinite(totalReferrals) ? totalReferrals : 0,
is_active: row.is_active,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Don't mask bad referral counts as zero.

Returning 0 for a malformed row.total_referrals under-reports real referrals and hides the upstream bug. Let this throw so the existing catch block returns a 500 instead of bad business data.

Suggested fix
-    const totalReferrals =
-      typeof row.total_referrals === "number"
-        ? row.total_referrals
-        : Number(row.total_referrals);
+    const totalReferrals = Number(row.total_referrals);
+    if (!Number.isFinite(totalReferrals)) {
+      throw new Error(`Invalid total_referrals for user ${user.id}`);
+    }
@@
-      total_referrals: Number.isFinite(totalReferrals) ? totalReferrals : 0,
+      total_referrals: totalReferrals,
As per coding guidelines, "`**/*.{ts,tsx}`: Use expect/throw pattern for error handling; always aim to catch errors and throw instead of hiding errors and falling back`."
📝 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 totalReferrals =
typeof row.total_referrals === "number"
? row.total_referrals
: Number(row.total_referrals);
const body: ReferralMeResponse = {
code: row.code,
total_referrals: Number.isFinite(totalReferrals) ? totalReferrals : 0,
is_active: row.is_active,
const totalReferrals = Number(row.total_referrals);
if (!Number.isFinite(totalReferrals)) {
throw new Error(`Invalid total_referrals for user ${user.id}`);
}
const body: ReferralMeResponse = {
code: row.code,
total_referrals: totalReferrals,
is_active: row.is_active,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/api/v1/referrals/route.ts` around lines 59 - 67, The code currently masks
malformed row.total_referrals by replacing non-finite values with 0 when
building the ReferralMeResponse; instead, change the logic in the block that
computes totalReferrals (and where the body is created for ReferralMeResponse)
to validate row.total_referrals and throw a clear error if it cannot be
converted to a finite number (e.g., when Number.isFinite(totalReferrals) is
false) so the existing catch will return a 500 rather than silently returning 0;
reference the totalReferrals variable, the row.total_referrals source, and the
ReferralMeResponse construction to locate and update the code.

Comment on lines +18 to +26
if (typeof o.code !== "string" || o.code.length === 0) return null;
const tr = o.total_referrals;
const totalReferrals = typeof tr === "number" ? tr : Number(tr);
if (!Number.isFinite(totalReferrals)) return null;
if (typeof o.is_active !== "boolean") return null;
return {
code: o.code,
total_referrals: totalReferrals,
is_active: o.is_active,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fail closed on malformed payloads.

Number(tr) accepts "", null, true, and [], so bad responses get coerced into 0/1 instead of being rejected. That makes the client silently tolerate contract drift and can show bogus referral counts.

Suggested tightening
 export function parseReferralMeResponse(data: unknown): ReferralMeResponse | null {
   if (typeof data !== "object" || data === null) return null;
   const o = data as Record<string, unknown>;
-  if (typeof o.code !== "string" || o.code.length === 0) return null;
-  const tr = o.total_referrals;
-  const totalReferrals = typeof tr === "number" ? tr : Number(tr);
-  if (!Number.isFinite(totalReferrals)) return null;
+  if (typeof o.code !== "string" || o.code.trim().length === 0) return null;
+  if (
+    typeof o.total_referrals !== "number" ||
+    !Number.isInteger(o.total_referrals) ||
+    o.total_referrals < 0
+  ) {
+    return null;
+  }
   if (typeof o.is_active !== "boolean") return null;
   return {
     code: o.code,
-    total_referrals: totalReferrals,
+    total_referrals: o.total_referrals,
     is_active: o.is_active,
   };
 }
📝 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
if (typeof o.code !== "string" || o.code.length === 0) return null;
const tr = o.total_referrals;
const totalReferrals = typeof tr === "number" ? tr : Number(tr);
if (!Number.isFinite(totalReferrals)) return null;
if (typeof o.is_active !== "boolean") return null;
return {
code: o.code,
total_referrals: totalReferrals,
is_active: o.is_active,
export function parseReferralMeResponse(data: unknown): ReferralMeResponse | null {
if (typeof data !== "object" || data === null) return null;
const o = data as Record<string, unknown>;
if (typeof o.code !== "string" || o.code.trim().length === 0) return null;
if (
typeof o.total_referrals !== "number" ||
!Number.isInteger(o.total_referrals) ||
o.total_referrals < 0
) {
return null;
}
if (typeof o.is_active !== "boolean") return null;
return {
code: o.code,
total_referrals: o.total_referrals,
is_active: o.is_active,
};
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/lib/types/referral-me.ts` around lines 18 - 26, The current
conversion uses Number(tr) which coerces values like "", null, true, and [] into
numbers; tighten validation in the block that computes totalReferrals (where tr
= o.total_referrals and totalReferrals is set via Number(tr)) so malformed
payloads are rejected: accept only a numeric type or a numeric-string (e.g.,
match /^\d+$/ or /^\d+(\.\d+)?$/ as appropriate), parse with Number/parseInt
after that check, verify Number.isFinite and (if integers expected)
Number.isInteger and non-negative, and return null for any other types
(including boolean, null, empty string, or arrays). Keep the existing checks for
o.code and o.is_active but replace the loose Number(tr) coercion with the
stricter validation and assignment to totalReferrals.

}
setLoadingReferral(false);
};
void loadReferral();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Handle the rejected referral fetch path.

Line 101 kicks off loadReferral() without a rejection handler. If fetch(), res.json(), or parseReferralMeResponse() throws, loadingReferral never flips to false, so this card can stay on the skeleton forever.

Suggested fix
-    void loadReferral();
+    void loadReferral().catch(() => {
+      if (cancelled) return;
+      setReferralFetchFailed(true);
+      setReferralMe(null);
+      setLoadingReferral(false);
+    });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/ui/src/components/affiliates/affiliates-page-client.tsx` at line
101, The call to void loadReferral() starts an async fetch that can throw
(fetch, res.json, or parseReferralMeResponse) and currently has no rejection
handling so loadingReferral may never be set to false; modify loadReferral (and
any async helpers like parseReferralMeResponse) to use try/catch/finally (or
attach .catch/.finally) so that on any error you call setLoadingReferral(false)
in the finally block and optionally set an error state or log the error; ensure
the function name loadReferral and state setter setLoadingReferral (and
parseReferralMeResponse if applicable) are the focal points for this change.

Comment on lines +24 to +25
const cachedRef = useRef<ReferralMeResponse | null>(null);
const inFlightRef = useRef<Promise<ReferralMeResponse> | null>(null);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Revalidate before copying.

cachedRef persists is_active for the life of the header. If a code is deactivated or reactivated after the first click, later copies use stale state until a full refresh, which undermines the inactive-link safeguard added in this PR.

Suggested change
 export function HeaderInviteButton() {
   const [loading, setLoading] = useState(false);
-  const cachedRef = useRef<ReferralMeResponse | null>(null);
   const inFlightRef = useRef<Promise<ReferralMeResponse> | null>(null);
 
   const resolveMe = useCallback(async (): Promise<ReferralMeResponse> => {
-    if (cachedRef.current) {
-      return cachedRef.current;
-    }
     if (inFlightRef.current) {
       return inFlightRef.current;
     }
 
     const promise = (async (): Promise<ReferralMeResponse> => {
@@
       const parsed = parseReferralMeResponse(json);
       if (!parsed) {
         throw new Error("Invalid response from server");
       }
-      cachedRef.current = parsed;
       return parsed;
     })();

Also applies to: 27-50

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/ui/src/components/layout/header-invite-button.tsx` around lines 24 -
25, The cachedRef persistence causes stale is_active decisions; before
performing the copy action, always revalidate current referral state instead of
relying solely on cachedRef: if an in-flight fetch (inFlightRef) exists await
it, otherwise trigger a fresh fetch of the referral status, update cachedRef
with the fresh ReferralMeResponse, then check is_active and proceed to copy only
if active (or abort if inactive); ensure any copy handler (where cachedRef and
inFlightRef are used) clears/updates cachedRef on error or after explicit
refresh so future clicks use the latest state.

- Add GET /api/v1/referrals with auth, CORS, rate limit, and strict
  total_referrals validation (fail 500 on corrupt DB values). Map auth
  failures with AuthenticationError and existing 401 handling.
- Tighten parseReferralMeResponse for total_referrals; add shared
  buildReferralInviteLoginUrl helper and wire Affiliates card + header Invite.
- Revalidate invite on each header click (no stale cache); pairing-token unit
  tests use real milady-web-ui + env instead of global mock.
- Coalesce empty ELIZA_CLOUD_AGENT_BASE_DOMAIN in getMiladyAgentPublicWebUiUrl;
  simplify compat-envelope Web UI URL call. Update docs and affiliates e2e.

Made-with: Cursor
@claude
Copy link
Copy Markdown

claude bot commented Mar 30, 2026

Code Review — PR #420: Referral Invite Links

Overall: Well-executed feature with thoughtful API design, good security posture, and excellent documentation. A few issues worth addressing before merge.


Bugs / Issues

1. navigator.clipboard has no HTTP fallback

navigator.clipboard.writeText() requires a secure context (HTTPS or localhost). On plain HTTP the Promise rejects silently until the toast fires. The header button likely has the same gap. Either add document.execCommand('copy') as a fallback or document the HTTPS-only requirement explicitly.

2. Missing null-check after getOrCreateCode

There is no server-side guard if getOrCreateCode returns null/undefined. The response builder would produce { code: undefined, ... }. Add an explicit null-check and return 500 rather than a malformed 200.

3. Inactive-code UX on Affiliates page

When is_active === false, the card shows the URL but blocks copy. Good. However the warning message does not tell the user why the code is inactive or how to activate it. Consider linking to support or clarifying that an admin must enable it.


Security

Stale is_active in header button cache

The header button caches the last successful GET in-memory until reload. If an admin deactivates a code after the user has fetched it, the user can still copy a stale active URL until their next page load. No real security impact (code fails at apply-time), but worth flagging for support awareness.


Test Coverage Gaps

Current E2E tests cover unauthenticated 401/403 and happy-path create + idempotency. Missing:

  • is_active: false case — confirm response shape and that copy is blocked
  • 403 case (authenticated but missing org/feature gate) — verify 401 vs 403 distinction is correct
  • Concurrent first-GET requests hitting the UNIQUE constraint + retry path in getOrCreateCode
  • total_referrals with bigint from DB (coercion handles it but no test exercises that branch)

Code Quality

total_referrals coercion duplicated

The same number-coercion logic appears in both the API route handler and parseReferralMeResponse. Extract to a shared coerceCount(val: unknown): number | null utility called from both.

Four useState hooks for one domain

The Affiliates page uses four separate hooks (referralMe, loadingReferral, referralFetchFailed, referralCopied). A useReferralMe() custom hook would clean this up and make the fetch + cancellation logic reusable for the header button, which likely duplicates the copy handler too.

Magic 2000 timeout

The setTimeout reset uses a raw 2000 value. Worth checking for a shared COPY_FEEDBACK_DURATION_MS constant in the codebase for consistency.


Minor Nits

  • The milady Web UI URL unit-test refactor (removing getMiladyAgentPublicWebUiUrl mock) is unrelated cleanup — consider splitting to a separate PR to keep this one focused.
  • docs/affiliate-referral-comparison.md is a great addition; the comparison table will save future contributors from confusing ?ref= with ?affiliate= URLs.

Summary

Category Verdict
Auth / Security Solid — proper 401/403 split, rate-limited, force-dynamic
Idempotency Good — UNIQUE constraint + retry in getOrCreateCode
Response shape Flat JSON with defensive parser
UI patterns Proper cleanup, cancellation tokens, loading states
Clipboard No fallback for non-HTTPS — should fix
Null-check post-getOrCreateCode Missing — should fix
Test coverage Happy path only; inactive/403/concurrent paths missing
Code duplication Coercion logic and copy handler duplicated
Docs Excellent WHY comments + reference docs

Blocking: clipboard fallback for non-HTTPS, null-check after getOrCreateCode.
Follow-up: test coverage for inactive/403/concurrent cases, deduplicate coercion + copy logic.

- Add copyTextToClipboard (Clipboard API + execCommand fallback), COPY_FEEDBACK_DURATION_MS,
  fetchReferralMe(), useDashboardReferralMe hook; wire header Invite + Affiliates copies.
- Referrals GET: validate row/code/is_active; share coerceNonNegativeIntegerCount with parser;
  stricter digit strings (no leading-zero ambiguity).
- Docs: refresh Invite freshness + clipboard/HTTPS notes; remove stale header-cache text.
- bunfig: default test maxConcurrency=1 to avoid mock.module cross-talk.
- Mock @/db/repositories/* submodules (not barrel) in admin pricing, privy-sync, proxy-pricing,
  milady-billing; afterAll mock.restore on admin pricing + mcp-tools.
- Unit tests: referral-me coercion/parser; e2e inactive-shape check for GET /api/v1/referrals.

Made-with: Cursor
@claude
Copy link
Copy Markdown

claude bot commented Mar 30, 2026

Code Review — PR #420

Overall this is well-structured and well-commented. The WHY-oriented comments are genuinely useful. A few issues worth addressing before merging.


Issues

1. Behavior change in getMiladyAgentPublicWebUiUrl is a silent regression

packages/lib/milady-web-ui.ts — the refactor changes the function's contract:

Before: if any domain source normalized to null (invalid baseDomain option, invalid env var, all fallbacks exhausted), the function returned null. Callers could check null to mean "no URL available."

After:

const normalizedDomain =
  normalizeAgentBaseDomain(options.baseDomain) ??
  normalizeAgentBaseDomain(process.env.ELIZA_CLOUD_AGENT_BASE_DOMAIN) ??
  DEFAULT_AGENT_BASE_DOMAIN;

DEFAULT_AGENT_BASE_DOMAIN is used as a final fallback, so the function can never return null anymore. This is fine in the happy path but changes the semantics — if a caller passes an explicit baseDomain that fails normalization, the function silently falls through to the default instead of returning null to signal failure. The return type still says string | null but the null path is now unreachable.

This behavioral change is unrelated to referrals and should be a deliberate, separately documented decision. At minimum, callers should know they can remove their null checks.

2. maxConcurrency = 1 serializes the entire test suite

bunfig.toml sets maxConcurrency = 1 to prevent global mock bleed. This fixes the symptom but at a high cost — all tests now run serially by default.

The PR also adds afterAll(() => mock.restore()) to the affected test files, which directly addresses the root cause. With proper mock.restore() in place, the concurrency restriction may be unnecessary. If it stays, it should at minimum be documented as a temporary measure and tracked for removal.

3. Fragile string matching in isUnauthorizedError

app/api/v1/referrals/route.ts lines 65–70:

error.message.includes("Invalid wallet signature") ||
error.message.includes("Wallet authentication failed")

These hardcoded strings will silently produce 500 instead of 401 if the wallet auth error messages ever change. This is noted in the comments as a known issue until those paths throw typed errors — just flagging it should be tracked and resolved rather than left indefinitely.

4. Inline URL computation duplicated in JSX

In affiliates-page-client.tsx, typeof window !== "undefined" ? window.location.origin : getAppUrl() appears twice in the render tree (inactive state and active state). It's "use client" so window is always defined at runtime — getAppUrl() is dead code in the active branch. Minor, but extracting to a local variable would be cleaner:

const origin = typeof window !== "undefined" ? window.location.origin : getAppUrl();

Minor observations

header-invite-button.tsx in-flight dedupe: The inFlightRef.current stores the original promise but the return value is promise.finally(...). Concurrent callers awaiting inFlightRef.current directly get the un-finally-chained promise — functionally identical (same resolved value), just worth noting the reference asymmetry.

inFlightRef doesn't cache results — acknowledged in comments. Every click re-fetches. For an inactive code, this means a network request on every click, which is fine for correctness but could be noisy. The roadmap item for SWR is the right long-term fix.

use-dashboard-referral-me.ts: Clean implementation. The cancelled flag and finally cleanup are correct.

copyViaExecCommand: Correct pattern. execCommand('copy') is deprecated but the fallback path is appropriate until Clipboard API adoption is universal.


What's good

  • Defensive parseReferralMeResponse / coerceNonNegativeIntegerCount with solid unit coverage
  • force-dynamic + rate limit + CORS on the new endpoint
  • Proper 401 vs 403 distinction
  • buildReferralInviteLoginUrl centralizes the URL shape
  • useDashboardReferralMe cancel-on-unmount is correct
  • Idempotency of getOrCreateCode is well-documented including the concurrent INSERT / unique-violation edge case

Summary: The milady-web-ui.ts behavioral change (item 1) is the main concern — it's an unrelated semantic change bundled into a feature PR that could affect existing callers. The maxConcurrency = 1 (item 2) deserves a follow-up issue. Items 3 and 4 are minor.

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

const s = val.trim();
if (!/^(0|[1-9]\d*)$/.test(s)) return null;
return parseInt(s, 10);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

String branch missing safe integer check unlike bigint branch

Low Severity

coerceNonNegativeIntegerCount applies a Number.isSafeInteger guard in the bigint branch but omits it in the string branch. For large numeric strings (e.g. "9007199254740993"), parseInt silently loses precision and returns a different number (9007199254740992), whereas the equivalent bigint input would correctly return null. This inconsistency means the function can silently return a corrupted count for string inputs above Number.MAX_SAFE_INTEGER.

Additional Locations (1)
Fix in Cursor Fix in Web

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants