Skip to content

fix: remove useAccessToken hook, use getAccessToken() directly#1628

Merged
sweetmantech merged 3 commits into
testfrom
fix/REC-45-remove-useAccessToken
Apr 2, 2026
Merged

fix: remove useAccessToken hook, use getAccessToken() directly#1628
sweetmantech merged 3 commits into
testfrom
fix/REC-45-remove-useAccessToken

Conversation

@recoup-coding-agent

@recoup-coding-agent recoup-coding-agent commented Apr 2, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Removed the useAccessToken hook which cached a stale Privy access token in React state
  • Updated all 12 consumer hooks to call getAccessToken() from usePrivy() at the point of each API call
  • This ensures a fresh, valid token is used for every request — fixing 401 errors when tokens expire in long-lived browser sessions

Files changed

  • Deleted: hooks/useAccessToken.ts
  • Updated: useConnectors, useAccountOrganizations, useChatTransport, useCreateChat, useRenameModal, useVercelChat, usePulseToggle, useArtists, useTaskRunStatus, useConversations, useMessageLoader, useCreateArtistTool

Key pattern changes

  • useQuery/useMutation hooks: getAccessToken() called inside queryFn/mutationFn, enabled uses authenticated flag
  • Async callbacks: getAccessToken() called at start of each operation
  • useChatTransport: returns async getHeaders() function instead of static headers
  • useMessageLoader: accepts getAccessToken function parameter instead of static token

Test plan

  • Verify login and chat functionality work normally
  • Leave browser open until token would expire (~60min), verify API calls still succeed
  • Verify artist loading, conversations, connectors, pulse toggle, task runs all function correctly

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Refactor

    • Centralized async authentication/token retrieval across the app for more reliable, on-demand auth.
    • Chat, messaging, and connector flows now fetch credentials at request time to reduce stale-token errors.
  • New Features

    • Artist lists now show pinned artists first, then alphabetically, for easier discovery.

The useAccessToken hook cached a Privy access token in React state via
useEffect. When the token expired (e.g. browser left open), the stale
cached value caused all subsequent API calls to fail with 401 errors.

Replace all usages with direct getAccessToken() calls from usePrivy()
at the point of each API call, ensuring a fresh token is always used.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
@vercel

vercel Bot commented Apr 2, 2026

Copy link
Copy Markdown
Contributor

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

Project Deployment Actions Updated (UTC)
recoup-chat Ready Ready Preview Apr 2, 2026 6:25pm

Request Review

@coderabbitai

coderabbitai Bot commented Apr 2, 2026

Copy link
Copy Markdown
Contributor

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 15faef88-4e7b-464f-ac9a-f123841a5077

📥 Commits

Reviewing files that changed from the base of the PR and between db5ebc3 and f9c658d.

📒 Files selected for processing (4)
  • hooks/useArtists.tsx
  • hooks/useMessageLoader.ts
  • hooks/useVercelChat.ts
  • lib/artists/sortArtistsWithPinnedFirst.ts

📝 Walkthrough

Walkthrough

Deleted the synchronous useAccessToken hook and migrated ~13 hooks to call Privy’s usePrivy().getAccessToken() (async) at runtime; queries now often gate on Privy’s authenticated; added sortArtistsWithPinnedFirst utility.

Changes

Cohort / File(s) Summary
Deleted Hook
hooks/useAccessToken.ts
Removed the synchronous access-token hook.
Query & Listing Hooks
hooks/useAccountOrganizations.ts, hooks/useArtists.tsx, hooks/useConversations.tsx, hooks/useTaskRunStatus.ts
Replaced captured token usage with async getAccessToken() in query functions; gate queries on Privy authenticated; removed token from query keys where applicable.
Callback / Effect Hooks
hooks/useConnectors.ts, hooks/useCreateArtistTool.ts, hooks/useCreateChat.tsx, hooks/useRenameModal.ts
Deferred token acquisition into callbacks/effects via getAccessToken(); added early-returns when token unavailable; updated dependencies to getAccessToken.
Transport & Chat Flow
hooks/useChatTransport.ts, hooks/useVercelChat.ts
Replaced memoized headers with async getHeaders(); transport initialization no longer depends on token; sends now await headers at call time; message loader no longer requires external token.
Message Loader Signature
hooks/useMessageLoader.ts
Removed accessToken parameter; loadMessages now obtains token via getAccessToken() internally and aborts if unavailable.
Pulse / Mutation Hooks
hooks/usePulseToggle.ts
Switched query/mutation functions to fetch tokens at call time and gate initial fetch on authenticated.
Misc Hooks
hooks/useCreateChat.tsx, hooks/useRenameModal.ts, hooks/useArtists.tsx
Updated effect/callback dependency lists to use getAccessToken and adjusted early-exit logic to obtain tokens on demand.
New Utility
lib/artists/sortArtistsWithPinnedFirst.ts
Added sortArtistsWithPinnedFirst(artists) to order pinned artists first, then by case-insensitive name.

Sequence Diagram(s)

sequenceDiagram
  participant UI as Client UI / Hook
  participant Privy as Privy (auth)
  participant API as Backend API
  participant Transport as Chat Transport

  UI->>Privy: call getAccessToken()
  Privy-->>UI: access token (or null)
  alt token returned
    UI->>API: request with Authorization: Bearer <token>
    API-->>UI: response (data)
    UI->>Transport: send/receive chat messages (await getHeaders -> Privy)
    Transport-->>API: proxied request with headers
    API-->>Transport: message response
    Transport-->>UI: message delivered
  else no token
    UI-->>UI: abort action / early return
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

Poem

Tokens wake when called, not kept in sleep,
Privy whispers keys to flows now running deep.
Hooks pivot to async steps in time,
Pinned artists rise, sorted and sublime.
Small shifts, clean lines — the codebase hums in rhyme. 🎵

🚥 Pre-merge checks | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Solid & Clean Code ⚠️ Warning PR violates DRY principle with 5+ inconsistent error handling patterns across 12+ hooks, uses unsafe non-null assertions bypassing error handling, and leaves state inconsistent in useArtists. Extract token validation into shared utility function; standardize all hooks to use it; fix useArtists state management; replace non-null assertions with proper validation.

✏️ 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 fix/REC-45-remove-useAccessToken

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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: db5ebc35b5

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

() => ["conversations", accessToken] as const,
[accessToken]
);
const queryKey = useMemo(() => ["conversations"] as const, []);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Scope conversations cache key to current user

useConversations now uses a static React Query key (["conversations"]), so cached data is shared across auth sessions. Because the app uses a singleton QueryClient (providers/Providers.tsx), logging out and back in as a different account can show the previous user’s conversations until a refetch succeeds (and can persist if that refetch fails). The key should include user identity (as it previously did via token) or be explicitly cleared on auth changes to avoid cross-account data leakage.

Useful? React with 👍 / 👎.

Comment thread hooks/useVercelChat.ts
const handleSubmit = async (e: React.FormEvent<HTMLFormElement>) => {
e.preventDefault();

const headers = await getHeaders();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Capture message state before awaiting auth headers

handleSubmit now awaits getHeaders() before reading input, creating an async gap where the textarea state can change. If token retrieval is slow (e.g., refresh), the sent payload may include newer text than what was submitted, and setInput("") can clear text the user typed after pressing send. Snapshot input/attachments before awaiting header retrieval (or block edits while awaiting) to keep send semantics deterministic.

Useful? React with 👍 / 👎.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
hooks/useVercelChat.ts (1)

197-239: ⚠️ Potential issue | 🟡 Minor

Await handleSubmit() before optimistic navigation.

Line 305 now kicks off an async handleSubmit() and immediately updates the URL / optimistic conversations. If getHeaders() rejects, the request never starts but the UI has already navigated as if it did.

💡 Proposed fix
-    handleSubmit(event);
+    await handleSubmit(event);

As per coding guidelines, custom hooks should "Handle edge cases and errors".

Also applies to: 294-311

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

In `@hooks/useVercelChat.ts` around lines 197 - 239, The submit handler
handleSubmit is async but callers perform optimistic navigation immediately
without awaiting it, so failures in getHeaders() or sendMessage() can leave the
UI in an incorrect state; update the places that call handleSubmit (the
event/trigger that currently invokes it around lines mentioned) to await
handleSubmit() and catch errors, and inside handleSubmit ensure sendMessage(...)
is awaited or its returned promise is propagated so callers can detect failures,
and on error revert/avoid optimistic navigation and surface an error state.
Ensure references: handleSubmit, getHeaders, sendMessage, textAttachments,
selectedFileAttachments are used when locating and adjusting the code.
hooks/useConversations.tsx (1)

24-37: ⚠️ Potential issue | 🟠 Major

Partition the conversations cache by user account identifier.

TanStack Query uses queryKey as the cache identity. With a static ["conversations"] key, conversations from one authenticated user remain cached under the same key when another user logs in, creating a privacy regression. This is particularly risky because the enabled guard prevents fetches while unauthenticated, but doesn't clear stale cached data.

💡 Proposed fix
-  const queryKey = useMemo(() => ["conversations"] as const, []);
+  const queryKey = useMemo(
+    () =>
+      [
+        "conversations",
+        userData?.account_id ?? userData?.id ?? null,
+      ] as const,
+    [userData?.account_id, userData?.id],
+  );
@@
-    enabled: authenticated,
+    enabled: authenticated && Boolean(userData?.account_id ?? userData?.id),

This ensures each user's conversations are cached separately per their account identifier, aligning with the guideline to "implement built-in security practices for authentication and data handling."

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

In `@hooks/useConversations.tsx` around lines 24 - 37, The query currently uses a
static queryKey (queryKey defined in useMemo as ["conversations"]) which causes
conversation caches to be shared across users; update queryKey to include the
authenticated user's unique account identifier (e.g., ["conversations",
accountId]) so each user has a separate cache entry. Obtain accountId
synchronously from your auth state/context (not via async getAccessToken call)
and make useMemo depend on accountId so the key updates when users change; also
change the useQuery enabled condition to authenticated && !!accountId to avoid
fetching without a user. Use the existing symbols queryKey, useQuery,
authenticated, getAccessToken/getConversations only in the queryFn (keep async
token retrieval there) and ensure the new accountId is used solely for the key
and enabled guard.
🧹 Nitpick comments (5)
hooks/useConnectors.ts (1)

46-46: Initial loading state may cause brief flash of incorrect UI.

The initial isLoading state is set to !accountId, which means when accountId is undefined (common case when using useConnectors() without config), isLoading starts as true. This is correct.

However, when accountId is explicitly passed but initially empty (e.g., artistAccountId not yet loaded), the check !accountId evaluates to true, setting isLoading to true. But when fetchConnectors runs and hits line 50-54, it immediately sets isLoading to false with an empty connectors array before the actual data loads.

Consider initializing loading state more defensively:

💡 Suggested initialization
-  const [isLoading, setIsLoading] = useState(!accountId);
+  const [isLoading, setIsLoading] = useState(true);

This ensures consumers always see a loading state until the first successful fetch completes.

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

In `@hooks/useConnectors.ts` at line 46, The initial isLoading initialization in
useConnectors leads to a premature false when fetchConnectors sets connectors to
[] before data arrives; update the logic so isLoading starts true whenever a
real fetch is required and remains true until the first successful response:
adjust the useState initialization for isLoading and the fetchConnectors flow in
the useConnectors hook (referencing isLoading, setIsLoading, fetchConnectors and
connectors) so that setIsLoading(false) is only called after a successful data
load (or explicit empty-result acknowledgement), and ensure the initial state is
conservative (true) when an accountId may be empty but intended to trigger a
fetch.
hooks/useMessageLoader.ts (1)

40-42: LGTM - Correct defensive token retrieval.

The pattern correctly handles potential null token:

const accessToken = await getAccessToken();
if (!accessToken) return;

Note: The early return when accessToken is null will leave isLoading as true (set on line 36) since finally block isn't reached when returning early from try. Consider whether this is the intended behavior or if you should set isLoading(false) before returning.

💡 Consider setting loading state on early return
       const accessToken = await getAccessToken();
-      if (!accessToken) return;
+      if (!accessToken) {
+        setIsLoading(false);
+        return;
+      }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@hooks/useMessageLoader.ts` around lines 40 - 42, In useMessageLoader, the
early return after checking const accessToken = await getAccessToken(); if
(!accessToken) return; leaves the isLoading state (set earlier) stuck true;
update the control flow in the load function (or surrounding try/catch) to
ensure isLoading is set to false on all exit paths — either set isLoading(false)
just before the early return or restructure to use a finally block so isLoading
is cleared regardless; reference the getAccessToken call and the isLoading
setter used in the load/message-fetching function to make this change.
hooks/useAccountOrganizations.ts (1)

44-47: Same non-null assertion pattern - consider adding guard for consistency.

Like useTaskRunStatus, this uses accessToken! without a guard. While authenticated should prevent execution when unauthenticated, adding explicit token validation provides defense-in-depth and clearer error messages when token retrieval fails unexpectedly.

🛡️ Consistent defensive pattern
     queryFn: async () => {
       const accessToken = await getAccessToken();
+      if (!accessToken) {
+        throw new Error("Failed to retrieve access token");
+      }
-      return fetchAccountOrganizations(accessToken!);
+      return fetchAccountOrganizations(accessToken);
     },
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@hooks/useAccountOrganizations.ts` around lines 44 - 47, The queryFn in
useAccountOrganizations uses a non-null assertion on accessToken (accessToken!)
which risks runtime errors; replace this with an explicit guard: call
getAccessToken(), check if the returned token is truthy, and if not throw a
clear Error (or return a rejected Promise) with a descriptive message, otherwise
pass the token into fetchAccountOrganizations(accessToken); follow the same
defensive pattern used in useTaskRunStatus to ensure consistent error handling
and clearer diagnostics.
hooks/usePulseToggle.ts (1)

15-18: Consistent token retrieval pattern, same guard recommendation applies.

Both queryFn and mutationFn use the accessToken! pattern. The mutation is particularly important to guard since failed token retrieval during a toggle would leave the UI in an inconsistent optimistic state until onSettled triggers invalidation.

🛡️ Add guards for both query and mutation
     queryFn: async () => {
       const accessToken = await getAccessToken();
+      if (!accessToken) throw new Error("Authentication required");
-      return getPulse({ accessToken: accessToken! });
+      return getPulse({ accessToken });
     },
     mutationFn: async (active: boolean) => {
       const accessToken = await getAccessToken();
+      if (!accessToken) throw new Error("Authentication required");
-      return updatePulse({ accessToken: accessToken!, active });
+      return updatePulse({ accessToken, active });
     },

Also applies to: 23-26

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

In `@hooks/usePulseToggle.ts` around lines 15 - 18, Replace the non-null
assertions on accessToken in both queryFn and mutationFn in
hooks/usePulseToggle.ts with explicit guards: call getAccessToken(), check if
the returned token is present and if not throw or return a rejected Promise (so
React Query treats it as an error) before calling getPulse or togglePulse;
ensure the mutationFn similarly rejects when no token is available so optimistic
updates are reverted and onSettled/invalidation behavior still runs. Use the
existing function names (queryFn and mutationFn) and
getAccessToken/getPulse/togglePulse to locate and update the logic.
hooks/useVercelChat.ts (1)

169-178: Please verify reload still sends auth and chat context.

This hook now attaches headers and chatRequestBody only inside the explicit sendMessage wrappers. The AI SDK documents sendMessage and regenerate as separate requests, each with their own ChatRequestOptions, so I think the raw regenerate returned as reload is now missing roomId / artistId / organizationId / model and the auth header unless it is wrapped too. (ai-sdk.dev)

💡 One way to make the behavior explicit
+  const reload = useCallback(async () => {
+    const headers = await getHeaders();
+    await regenerate({ body: chatRequestBody, headers });
+  }, [regenerate, chatRequestBody, getHeaders]);
@@
-    reload: regenerate,
+    reload,

As per coding guidelines, custom hooks should "Return consistent interface" and "Handle edge cases and errors".

Also applies to: 197-245, 315-321

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

In `@hooks/useVercelChat.ts` around lines 169 - 178, The returned reload (the raw
regenerate from the AI SDK) no longer includes the auth headers and chat context
(roomId/artistId/organizationId/model) because headers and chatRequestBody are
only attached inside the sendMessage wrappers; fix by wrapping the regenerate
call the same way sendMessage is wrapped so it always merges headers and the
memoized chatRequestBody (respecting the optional organizationId spread) into
its ChatRequestOptions, reusing the existing headers and chatRequestBody
variables and returning a reload function with the same signature and error
handling as sendMessage to keep a consistent interface and handle edge cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@hooks/useArtists.tsx`:
- Around line 83-94: In getArtists, the early-return branches that run when
!userData?.id or when getAccessToken() returns falsy only clear artists; update
those branches to also reset the rest of hook state so the hook returns a
consistent interface: clear selectedArtist (e.g.,
setSelectedArtist(null/undefined)), setIsLoading(false), and clear any error
state if present (e.g., setError(null)), before returning; locate the early-exit
logic inside the useCallback for getArtists and ensure you call
setSelectedArtist and setIsLoading alongside setArtists.

In `@hooks/useTaskRunStatus.ts`:
- Around line 28-31: The queryFn inside useTaskRunStatus currently uses a
non-null assertion on accessToken (accessToken!) which can be null; update
queryFn to check the result of getAccessToken() and if it is null/undefined
throw an Error (or return a rejected promise) so React Query's retry logic
(retry: 3) can run instead of calling getTaskRunStatus with a null token;
specifically, modify the queryFn that calls getAccessToken() and only call
getTaskRunStatus(runId, accessToken) when accessToken is truthy, otherwise throw
a clear error to trigger retry/refresh.

---

Outside diff comments:
In `@hooks/useConversations.tsx`:
- Around line 24-37: The query currently uses a static queryKey (queryKey
defined in useMemo as ["conversations"]) which causes conversation caches to be
shared across users; update queryKey to include the authenticated user's unique
account identifier (e.g., ["conversations", accountId]) so each user has a
separate cache entry. Obtain accountId synchronously from your auth
state/context (not via async getAccessToken call) and make useMemo depend on
accountId so the key updates when users change; also change the useQuery enabled
condition to authenticated && !!accountId to avoid fetching without a user. Use
the existing symbols queryKey, useQuery, authenticated,
getAccessToken/getConversations only in the queryFn (keep async token retrieval
there) and ensure the new accountId is used solely for the key and enabled
guard.

In `@hooks/useVercelChat.ts`:
- Around line 197-239: The submit handler handleSubmit is async but callers
perform optimistic navigation immediately without awaiting it, so failures in
getHeaders() or sendMessage() can leave the UI in an incorrect state; update the
places that call handleSubmit (the event/trigger that currently invokes it
around lines mentioned) to await handleSubmit() and catch errors, and inside
handleSubmit ensure sendMessage(...) is awaited or its returned promise is
propagated so callers can detect failures, and on error revert/avoid optimistic
navigation and surface an error state. Ensure references: handleSubmit,
getHeaders, sendMessage, textAttachments, selectedFileAttachments are used when
locating and adjusting the code.

---

Nitpick comments:
In `@hooks/useAccountOrganizations.ts`:
- Around line 44-47: The queryFn in useAccountOrganizations uses a non-null
assertion on accessToken (accessToken!) which risks runtime errors; replace this
with an explicit guard: call getAccessToken(), check if the returned token is
truthy, and if not throw a clear Error (or return a rejected Promise) with a
descriptive message, otherwise pass the token into
fetchAccountOrganizations(accessToken); follow the same defensive pattern used
in useTaskRunStatus to ensure consistent error handling and clearer diagnostics.

In `@hooks/useConnectors.ts`:
- Line 46: The initial isLoading initialization in useConnectors leads to a
premature false when fetchConnectors sets connectors to [] before data arrives;
update the logic so isLoading starts true whenever a real fetch is required and
remains true until the first successful response: adjust the useState
initialization for isLoading and the fetchConnectors flow in the useConnectors
hook (referencing isLoading, setIsLoading, fetchConnectors and connectors) so
that setIsLoading(false) is only called after a successful data load (or
explicit empty-result acknowledgement), and ensure the initial state is
conservative (true) when an accountId may be empty but intended to trigger a
fetch.

In `@hooks/useMessageLoader.ts`:
- Around line 40-42: In useMessageLoader, the early return after checking const
accessToken = await getAccessToken(); if (!accessToken) return; leaves the
isLoading state (set earlier) stuck true; update the control flow in the load
function (or surrounding try/catch) to ensure isLoading is set to false on all
exit paths — either set isLoading(false) just before the early return or
restructure to use a finally block so isLoading is cleared regardless; reference
the getAccessToken call and the isLoading setter used in the
load/message-fetching function to make this change.

In `@hooks/usePulseToggle.ts`:
- Around line 15-18: Replace the non-null assertions on accessToken in both
queryFn and mutationFn in hooks/usePulseToggle.ts with explicit guards: call
getAccessToken(), check if the returned token is present and if not throw or
return a rejected Promise (so React Query treats it as an error) before calling
getPulse or togglePulse; ensure the mutationFn similarly rejects when no token
is available so optimistic updates are reverted and onSettled/invalidation
behavior still runs. Use the existing function names (queryFn and mutationFn)
and getAccessToken/getPulse/togglePulse to locate and update the logic.

In `@hooks/useVercelChat.ts`:
- Around line 169-178: The returned reload (the raw regenerate from the AI SDK)
no longer includes the auth headers and chat context
(roomId/artistId/organizationId/model) because headers and chatRequestBody are
only attached inside the sendMessage wrappers; fix by wrapping the regenerate
call the same way sendMessage is wrapped so it always merges headers and the
memoized chatRequestBody (respecting the optional organizationId spread) into
its ChatRequestOptions, reusing the existing headers and chatRequestBody
variables and returning a reload function with the same signature and error
handling as sendMessage to keep a consistent interface and handle edge cases.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 9986a880-cc59-47b7-8e82-7420d50c233c

📥 Commits

Reviewing files that changed from the base of the PR and between 98b67a9 and db5ebc3.

📒 Files selected for processing (13)
  • hooks/useAccessToken.ts
  • hooks/useAccountOrganizations.ts
  • hooks/useArtists.tsx
  • hooks/useChatTransport.ts
  • hooks/useConnectors.ts
  • hooks/useConversations.tsx
  • hooks/useCreateArtistTool.ts
  • hooks/useCreateChat.tsx
  • hooks/useMessageLoader.ts
  • hooks/usePulseToggle.ts
  • hooks/useRenameModal.ts
  • hooks/useTaskRunStatus.ts
  • hooks/useVercelChat.ts
💤 Files with no reviewable changes (1)
  • hooks/useAccessToken.ts

Comment thread hooks/useArtists.tsx
Comment on lines 83 to 94
const getArtists = useCallback(
async (artistId?: string) => {
if (!userData?.id || !accessToken) {
if (!userData?.id) {
setArtists([]);
return;
}

const accessToken = await getAccessToken();
if (!accessToken) {
setArtists([]);
return;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Reset the rest of the hook state on auth early exits.

These branches only clear artists. If userData is missing or getAccessToken() returns nothing, the hook can still expose a stale selectedArtist and leave isLoading stuck true, which will make the artists UI look empty-but-loading forever on the very token-expiry path this PR is targeting.

💡 Proposed fix
       if (!userData?.id) {
         setArtists([]);
+        setSelectedArtist(null);
+        setIsLoading(false);
         return;
       }
 
       const accessToken = await getAccessToken();
       if (!accessToken) {
         setArtists([]);
+        setSelectedArtist(null);
+        setIsLoading(false);
         return;
       }

As per coding guidelines, custom hooks should "Handle edge cases and errors" and "Return consistent interface".

📝 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 getArtists = useCallback(
async (artistId?: string) => {
if (!userData?.id || !accessToken) {
if (!userData?.id) {
setArtists([]);
return;
}
const accessToken = await getAccessToken();
if (!accessToken) {
setArtists([]);
return;
}
const getArtists = useCallback(
async (artistId?: string) => {
if (!userData?.id) {
setArtists([]);
setSelectedArtist(null);
setIsLoading(false);
return;
}
const accessToken = await getAccessToken();
if (!accessToken) {
setArtists([]);
setSelectedArtist(null);
setIsLoading(false);
return;
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@hooks/useArtists.tsx` around lines 83 - 94, In getArtists, the early-return
branches that run when !userData?.id or when getAccessToken() returns falsy only
clear artists; update those branches to also reset the rest of hook state so the
hook returns a consistent interface: clear selectedArtist (e.g.,
setSelectedArtist(null/undefined)), setIsLoading(false), and clear any error
state if present (e.g., setError(null)), before returning; locate the early-exit
logic inside the useCallback for getArtists and ensure you call
setSelectedArtist and setIsLoading alongside setArtists.

Comment thread hooks/useTaskRunStatus.ts
Comment on lines +28 to +31
queryFn: async () => {
const accessToken = await getAccessToken();
return getTaskRunStatus(runId, accessToken!);
},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Non-null assertion may pass null token to API if token retrieval fails.

The queryFn uses accessToken! (non-null assertion), but getAccessToken() can return null even when authenticated is true (e.g., if token just expired or refresh fails). This would result in an invalid Bearer null header being sent to the API.

Consider adding a guard or throwing to trigger React Query's retry mechanism:

🛡️ Safer token handling
     queryFn: async () => {
       const accessToken = await getAccessToken();
+      if (!accessToken) {
+        throw new Error("Failed to retrieve access token");
+      }
-      return getTaskRunStatus(runId, accessToken!);
+      return getTaskRunStatus(runId, accessToken);
     },

This allows React Query's retry: 3 to handle transient token retrieval failures gracefully.

📝 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
queryFn: async () => {
const accessToken = await getAccessToken();
return getTaskRunStatus(runId, accessToken!);
},
queryFn: async () => {
const accessToken = await getAccessToken();
if (!accessToken) {
throw new Error("Failed to retrieve access token");
}
return getTaskRunStatus(runId, accessToken);
},
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@hooks/useTaskRunStatus.ts` around lines 28 - 31, The queryFn inside
useTaskRunStatus currently uses a non-null assertion on accessToken
(accessToken!) which can be null; update queryFn to check the result of
getAccessToken() and if it is null/undefined throw an Error (or return a
rejected promise) so React Query's retry logic (retry: 3) can run instead of
calling getTaskRunStatus with a null token; specifically, modify the queryFn
that calls getAccessToken() and only call getTaskRunStatus(runId, accessToken)
when accessToken is truthy, otherwise throw a clear error to trigger
retry/refresh.

Comment thread hooks/useArtists.tsx
Comment thread hooks/useMessageLoader.ts Outdated
roomId: string | undefined,
userId: string | undefined,
accessToken: string | null,
getAccessToken: () => Promise<string | null>,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

DRY

  • actual: passing getAccessToken as a param for the function.
  • required: directly import the usePrivy hook in useMessageLoader.

Instead of passing getAccessToken as a parameter from the caller,
useMessageLoader now imports usePrivy and calls getAccessToken itself.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Comment thread hooks/useArtists.tsx Outdated

// Helper function to sort artists with pinned first, then alphabetically
const sortArtistsWithPinnedFirst = (artists: ArtistRecord[]): ArtistRecord[] => {
const sortArtistsWithPinnedFirst = (

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

SRP

  • move sortArtistsWithPinnedFirst to a standalone lib file.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@sweetmantech sweetmantech merged commit f31c2f4 into test Apr 2, 2026
2 checks passed
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