feat: migrate data fetching to React Query hooks#105
Merged
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 9 minutes and 34 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughReplaces manual useState/useEffect fetching with React Query across core hooks and the project settings page; adds six query hooks, refactors useProjectViewer and useRemoteSync to use queries/mutations/cache ops, and updates tests to run under a QueryClientProvider and assert hook-level behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as "Settings Page / Component"
participant Hook as "useRemoteSync / useProjectViewer"
participant API as "remoteSyncApi / other APIs"
participant QC as "QueryClient (cache)"
UI->>Hook: invoke useQuery / mutate
Hook->>QC: read cached data (queryKey)
alt cache miss or refetch
Hook->>API: fetch data or perform mutation
API-->>Hook: return result
Hook->>QC: setQueryData(queryKey, updatedData) or invalidateQueries(...)
end
QC-->>UI: updated cached data exposed via hooks
Hook-->>UI: expose isLoading / isSuccess / error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
60 tasks
JohnRDOrazio
force-pushed
the
feat/react-query-migration
branch
from
April 10, 2026 23:37
256271a to
47ad81f
Compare
Create standalone React Query hooks for reusable data fetching: - useMembers: project member list with add/update/remove mutations - useLintSummary: lint status summary - useNormalizationStatus: ontology normalization status - useIndexStatus: ontology search index status - useOpenPRCount: open pull request count - usePendingSuggestionCount: pending suggestion count Migrate settings page from manual useState/useEffect data fetching to React Query hooks with proper cache invalidation after mutations (update project, add/remove members, transfer ownership, reindex, normalization). Convert useRemoteSync from useState/useEffect/setInterval to React Query useQuery for config and history, with useMutation for save and delete operations. Polling for background jobs is retained. Simplify useProjectViewer by replacing manual useEffect fetch calls for secondary data with the new standalone query hooks. Update tests to provide QueryClientProvider wrapper and mock the new hooks instead of raw API modules. Addresses #89 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
JohnRDOrazio
force-pushed
the
feat/react-query-migration
branch
from
April 11, 2026 01:29
47ad81f to
6cf1c7c
Compare
damienriehl
previously requested changes
Apr 12, 2026
damienriehl
left a comment
damienriehl
left a comment
Contributor
There was a problem hiding this comment.
Good work on Phase 2 of the React Query migration — the new standalone hooks (useOpenPRCount, useLintSummary, useNormalizationStatus, useIndexStatus, usePendingSuggestionCount, useMembers) follow clean conventions, and the useProjectViewer cleanup is solid. CI is green (build, lint, test, type-check all pass).
A few issues to address:
1. Settings page: React Query + local state sync is an anti-pattern (major)
The settings page fetches via React Query (useProject, useMembers, useNormalizationStatus, useIndexStatus) then immediately syncs to local useState via useEffect:
const [project, setProject] = useState<Project | null>(null);
useEffect(() => {
if (projectData) setProject(projectData);
}, [projectData]);This creates two sources of truth that can diverge. If React Query refetches in the background (e.g., on window focus), the useEffect fires and overwrites any local mutations the user may have triggered (like handleSaveSettings calling setProject(updated)). Going the other direction, local setProject() calls don't update the React Query cache, so any component elsewhere reading from the query sees stale data.
Recommendation: Either:
- (a) Idiomatic React Query — remove local state mirrors for
projectandmembers; read directly from query data; usequeryClient.setQueryData()for optimistic updates in mutation handlers. FornormalizationStatusandindexStatus, the complex polling/job-tracking flows make local state reasonable — those can stay as-is. - (b) Pragmatic — if (a) is too much churn for this PR, acknowledge the pattern as a known compromise and file a follow-up issue. But don't ship the dual state without at least documenting why.
2. useMembers: mutation hooks defined but never used
useAddMember, useUpdateMemberRole, and useRemoveMember are defined in lib/hooks/useMembers.ts (lines 20–89) but the settings page never imports them — it still calls projectApi directly plus manual queryClient.invalidateQueries. This is dead code that also creates a confusing parallel path.
Either wire these mutations into the settings page or remove them until they're needed.
3. handleApproveJoinRequest: redundant invalidation + manual fetch
queryClient.invalidateQueries({ queryKey: memberQueryKeys.list(projectId) });
const refreshedMembers = await projectApi.listMembers(project.id, session.accessToken);
setMembers(refreshedMembers.items);The invalidation triggers a background refetch, and the manual fetch does the same thing immediately after. These will race. Either rely on React Query's refetch (and read from the query) or do the manual fetch only (and skip the invalidation).
4. useOpenPRCount / useLintSummary: missing accessToken guard in enabled
These hooks only check !!projectId for enabled, unlike the other hooks which also check !!accessToken. If these API endpoints require auth, this will fire 401 errors on every unauthenticated page load:
// Should be:
enabled: !!projectId && !!accessToken,5. projectQueryKeys.detail(projectId, true) — hardcoded boolean
All cache invalidation calls in settings use the hardcoded true:
queryClient.invalidateQueries({ queryKey: projectQueryKeys.detail(projectId, true) });The second param is isAuthenticated (derived from !!accessToken in useProject). This works because settings requires auth, but it's brittle. Consider !!session?.accessToken for clarity and safety.
6. Removed test coverage: accessToken guard for suggestions
The old suite had "does not fetch suggestions when canEdit is true but accessToken is missing". The new hook handles this via enabled: !!accessToken, but the behavioral assertion is lost. Worth adding a unit test for usePendingSuggestionCount that verifies the enabled guard.
7. Minor: useRemoteSync wrapper callbacks recreate on mutation state changes
The useCallback wrappers around saveConfigMutation.mutateAsync list the mutation object in their dependency array. Since useMutation returns a new reference on state transitions (idle → loading → success), these callbacks get recreated on every mutation. Not a correctness bug, but consumers depending on referential stability could see unexpected re-fires. Consider extracting just .mutateAsync into a stable ref.
Bottom line: The new hooks and useProjectViewer refactor are clean. The main blocker is #1 (dual source of truth in settings) — resolve it or explicitly defer it. Issues #2–4 should ship with this PR. #5–7 are nits.
Remove local state mirrors for project/members data — read directly from React Query cache and use setQueryData for optimistic updates in mutation handlers. Also: remove unused mutation hooks from useMembers, add accessToken guard to useOpenPRCount/useLintSummary enabled flags, and fix handleApproveJoinRequest redundant fetch. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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/projects/`[id]/settings/page.tsx:
- Around line 137-140: The status hooks currently enable based only on
project?.source_file_path which can fire before session is hydrated; update the
enabled predicates to also require a valid token (e.g. enabled:
!!project?.source_file_path && !!session?.accessToken) for
useNormalizationStatus and the other status hook (the second hook at 166-167),
and ensure you're passing session?.accessToken as the token argument to those
hooks (useNormalizationStatus and the other status hook) so API calls never run
with an undefined token.
- Around line 101-118: The composed error value should always be a string or
null; change the assignment that builds error so it converts projectError to a
string before use (e.g., use projectError?.message or String(projectError) when
projectError is present) so that the derived variable error (currently built
from localError and projectError) is renderable; update the code around the
useProject usage where projectError is referenced and replace the current "const
error = localError || projectError || null" with a form that yields a string or
null (preserving localError priority).
In `@lib/hooks/useIndexStatus.ts`:
- Around line 13-16: The query in useIndexStatus is currently only keyed by
projectId and only gated on projectId, which lets it run once with accessToken
undefined; update useIndexStatus so the queryKey includes accessToken (e.g.,
indexQueryKeys.status(projectId, accessToken)) and gate the query by both
projectId and accessToken (enabled: (options?.enabled ?? true) && !!projectId &&
!!accessToken), and ensure the queryFn still calls
projectOntologyApi.getIndexStatus(projectId, accessToken) so the authenticated
token is used for the request.
In `@lib/hooks/useNormalizationStatus.ts`:
- Around line 13-16: The query in useNormalizationStatus currently enables based
only on projectId and options, so normalizationApi.getStatus(projectId,
accessToken) can be called with an undefined accessToken; update the enabled
condition to also require a valid access token (e.g., change enabled to
(options?.enabled ?? true) && !!projectId && !!accessToken) so the query is
blocked until session.auth provides session.accessToken, or alternatively
short-circuit in the queryFn of useNormalizationStatus to throw/return early
when accessToken is missing before calling normalizationApi.getStatus.
In `@lib/hooks/useRemoteSync.ts`:
- Around line 72-80: The hook's loading state is wrong because
configQuery.isLoading alone can become false before historyQuery has started;
update the history query to enable only when configQuery is settled (use
configQuery.isSuccess or !!configQuery.data) and expose/compute a combined
loading flag (e.g., const isLoading = configQuery.isLoading ||
historyQuery.isLoading) so callers see loading true until both config and
history are fetched; change the history useQuery's enabled to depend on
configQuery.isSuccess (instead of just !!configQuery.data) and return or export
the combined isLoading from the hook (referencing historyQuery, configQuery,
useQuery, and remoteSyncApi.getHistory).
- Around line 65-80: The queries are starting before the session token exists,
causing unauthenticated cached results; modify both useQuery calls so they only
run when accessToken is present and include accessToken in the query keys to
avoid stale unauthenticated cache hits: update the queryKey calls to
remoteSyncQueryKeys.config(projectId, accessToken) and
remoteSyncQueryKeys.history(projectId, accessToken), and change the enabled
conditions to include !!accessToken (e.g. enabled: !!projectId && enabled &&
!!accessToken, and for history also && !!configQuery.data).
- Around line 194-197: The returned error currently ignores failures from
historyQuery, causing consumers to see history: [] with error: null; update the
error combination logic in useRemoteSync (referencing checkError, historyQuery,
and configQuery) to include historyQuery.error (prefer checkError or mutation
errors first, then historyQuery.error, then configQuery.error/message) so any
history fetch failure surfaces as the returned error; ensure the final error
variable reflects this ordering and that callers receive a non-null error when
historyQuery failed.
🪄 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: 1e3a9a51-b14c-4cc2-859b-7303f82ebbce
📒 Files selected for processing (11)
__tests__/lib/hooks/useProjectViewer.test.ts__tests__/lib/hooks/useRemoteSync.test.tsapp/projects/[id]/settings/page.tsxlib/hooks/useIndexStatus.tslib/hooks/useLintSummary.tslib/hooks/useMembers.tslib/hooks/useNormalizationStatus.tslib/hooks/useOpenPRCount.tslib/hooks/usePendingSuggestionCount.tslib/hooks/useProjectViewer.tslib/hooks/useRemoteSync.ts
Comment on lines
+101
to
+118
| const { | ||
| project, | ||
| isLoading: isProjectLoading, | ||
| error: projectError, | ||
| } = useProject(projectId, session?.accessToken); | ||
| const { data: membersData } = useMembers(projectId, session?.accessToken); | ||
| const members = membersData?.items ?? []; | ||
|
|
||
| // Stable query key for optimistic updates via setQueryData | ||
| const projectKey = projectQueryKeys.detail(projectId, !!session?.accessToken); | ||
|
|
||
| const [isSaving, setIsSaving] = useState(false); | ||
| const [isDeleting, setIsDeleting] = useState(false); | ||
| const [error, setError] = useState<string | null>(null); | ||
| const [localError, setLocalError] = useState<string | null>(null); | ||
| const [successMessage, setSuccessMessage] = useState<string | null>(null); | ||
|
|
||
| // Derive displayed error: local mutation errors take priority, then query errors | ||
| const error = localError || projectError || null; |
There was a problem hiding this comment.
Convert projectError to a string before rendering it.
useProject() returns an error object, so localError || projectError makes error non-renderable on the failure path. The 403/404/network fallback will throw instead of showing the error state.
Suggested fix
const {
project,
isLoading: isProjectLoading,
error: projectError,
} = useProject(projectId, session?.accessToken);
+ const projectErrorMessage =
+ projectError instanceof Error
+ ? projectError.message
+ : projectError
+ ? String(projectError)
+ : null;
const { data: membersData } = useMembers(projectId, session?.accessToken);
const members = membersData?.items ?? [];
@@
- const error = localError || projectError || null;
+ const error = localError || projectErrorMessage;📝 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 { | |
| project, | |
| isLoading: isProjectLoading, | |
| error: projectError, | |
| } = useProject(projectId, session?.accessToken); | |
| const { data: membersData } = useMembers(projectId, session?.accessToken); | |
| const members = membersData?.items ?? []; | |
| // Stable query key for optimistic updates via setQueryData | |
| const projectKey = projectQueryKeys.detail(projectId, !!session?.accessToken); | |
| const [isSaving, setIsSaving] = useState(false); | |
| const [isDeleting, setIsDeleting] = useState(false); | |
| const [error, setError] = useState<string | null>(null); | |
| const [localError, setLocalError] = useState<string | null>(null); | |
| const [successMessage, setSuccessMessage] = useState<string | null>(null); | |
| // Derive displayed error: local mutation errors take priority, then query errors | |
| const error = localError || projectError || null; | |
| const { | |
| project, | |
| isLoading: isProjectLoading, | |
| error: projectError, | |
| } = useProject(projectId, session?.accessToken); | |
| const projectErrorMessage = | |
| projectError instanceof Error | |
| ? projectError.message | |
| : projectError | |
| ? String(projectError) | |
| : null; | |
| const { data: membersData } = useMembers(projectId, session?.accessToken); | |
| const members = membersData?.items ?? []; | |
| // Stable query key for optimistic updates via setQueryData | |
| const projectKey = projectQueryKeys.detail(projectId, !!session?.accessToken); | |
| const [isSaving, setIsSaving] = useState(false); | |
| const [isDeleting, setIsDeleting] = useState(false); | |
| const [localError, setLocalError] = useState<string | null>(null); | |
| const [successMessage, setSuccessMessage] = useState<string | null>(null); | |
| // Derive displayed error: local mutation errors take priority, then query errors | |
| const error = localError || projectErrorMessage; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/projects/`[id]/settings/page.tsx around lines 101 - 118, The composed
error value should always be a string or null; change the assignment that builds
error so it converts projectError to a string before use (e.g., use
projectError?.message or String(projectError) when projectError is present) so
that the derived variable error (currently built from localError and
projectError) is renderable; update the code around the useProject usage where
projectError is referenced and replace the current "const error = localError ||
projectError || null" with a form that yields a string or null (preserving
localError priority).
Tests for useOpenPRCount, useLintSummary, useMembers, useNormalizationStatus, useIndexStatus, and usePendingSuggestionCount. Each test verifies data fetching, enabled guards on missing params, and the enabled option where applicable. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@__tests__/lib/hooks/useQueryHooks.test.ts`:
- Around line 61-64: The anonymous wrapper returned by createWrapper causes a
react/display-name lint error; change createWrapper so it returns a named
component (e.g., function Wrapper({ children }: { children: ReactNode }) {
return React.createElement(QueryClientProvider, { client: qc }, children); } )
instead of an anonymous arrow, or assign a displayName (Wrapper.displayName =
'QueryWrapper') after defining it. Ensure the function name (Wrapper) is used as
the returned value from createWrapper and still provides the QueryClient (qc) to
QueryClientProvider.
🪄 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: 0d76e4ee-7b13-4c53-81b2-b91cd7dc803a
📒 Files selected for processing (1)
__tests__/lib/hooks/useQueryHooks.test.ts
Prevent unauthenticated API calls by requiring accessToken in the enabled condition and including it in the queryKey to avoid stale cache entries from before session hydration. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ry key Prevent the normalization status query from firing before session hydration by requiring accessToken in both the enabled predicate and the queryKey. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add accessToken check to the enabled option passed to useNormalizationStatus and useIndexStatus in the settings page for defense-in-depth, ensuring queries never fire before session hydration even at the call site. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…and error surfacing - Include accessToken in query keys to prevent stale unauthenticated cache hits. - Gate both config and history queries on !!accessToken so they don't fire before session hydration. - Gate history query on configQuery.isSuccess (not just !!configQuery.data) so it waits until config is fully settled. - Expose combined isLoading (config + history) so callers see loading true until both queries complete. - Include historyQuery.error in the error combination logic so history fetch failures surface to consumers. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ay-name Replace the anonymous arrow function returned by createWrapper with a named Wrapper function so ESLint's react/display-name rule is satisfied. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…e changes Use refs for mutateAsync functions so saveConfig and deleteConfig callbacks don't recreate on every mutation state transition (idle → loading → success), preventing unnecessary re-renders in consumers that depend on referential stability. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move mutateAsync ref updates into useEffect blocks instead of assigning during render, which the react-hooks/refs lint rule prohibits. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
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)
lib/hooks/useRemoteSync.ts (1)
93-132:⚠️ Potential issue | 🟠 MajorConvert the polling loop to single-flight with fresh token reads.
setInterval(async ...)can fire overlappinggetJobStatuscalls if a request exceeds 2 seconds, and the callback capturesaccessTokenfrom the momentstartPolling()ran. If the session token refreshes mid-poll, the active interval continues using the stale token, causing repeated 401s whileisCheckingnever clears. Use a recursivesetTimeout-based loop that reads the latest token from a ref (updated via the dependency array or passed on each call) to ensure single-flight polling and fresh authentication.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/hooks/useRemoteSync.ts` around lines 93 - 132, startPolling currently uses setInterval which can fire overlapping async getJobStatus calls and captures the accessToken at invocation; replace the interval with a single-flight recursive setTimeout loop that awaits each remoteSyncApi.getJobStatus call before scheduling the next tick, read the latest token from a mutable ref (e.g., accessTokenRef.current) inside the loop instead of the closed-over accessToken, keep pollTimerRef to store/clear the timeout id, stop the loop and clear pollTimerRef when jobIdRef or status becomes complete/failed (and still call setIsChecking(false), setCheckError(...) and await refetchAll()), and ensure cancellations clear the pending timeout to avoid stray calls.app/projects/[id]/settings/page.tsx (1)
748-785:⚠️ Potential issue | 🟠 MajorThe normalization poller is never cleaned up on unmount.
The cleanup returned at line 784 is inside an event handler (
handleRunNormalization), so React ignores it. Leaving this page while a normalization job is running keeps the interval alive and it continues calling state setters on an unmounted component, causing memory leaks.Suggested fix
+ // add near `reindexPollRef` + const normalizationPollRef = useRef<ReturnType<typeof setInterval> | null>(null); + + useEffect(() => { + return () => { + if (normalizationPollRef.current) { + clearInterval(normalizationPollRef.current); + } + }; + }, []); ... - const pollInterval = setInterval(async () => { + normalizationPollRef.current = setInterval(async () => { try { const jobStatus = await normalizationApi.getJobStatus( project.id, queueResult.job_id, session.accessToken ); setNormalizationJobStatus(jobStatus.status); if (jobStatus.status === "complete") { - clearInterval(pollInterval); + clearInterval(normalizationPollRef.current!); + normalizationPollRef.current = null; setNormalizationJobId(null); setNormalizationJobStatus(null); setIsRunningNormalization(false); ... } else if (jobStatus.status === "failed" || jobStatus.status === "not_found") { - clearInterval(pollInterval); + clearInterval(normalizationPollRef.current!); + normalizationPollRef.current = null; setNormalizationJobId(null); setNormalizationJobStatus(null); setIsRunningNormalization(false); ... } } catch (pollErr) { console.error("Error polling job status:", pollErr); } }, 2000); - - return () => clearInterval(pollInterval);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/projects/`[id]/settings/page.tsx around lines 748 - 785, The polling interval created inside the event handler is never cleaned up on component unmount because the return cleanup is inside the handler rather than a useEffect; fix by moving interval management to component scope: create a ref (e.g., normalizationPollerRef) to hold the interval id, start the interval in handleRunNormalization by assigning normalizationPollerRef.current = setInterval(...), clear the interval whenever the job finishes/fails inside that same polling callback (clearInterval(normalizationPollerRef.current) and set normalizationPollerRef.current = null), and add a useEffect cleanup that clears normalizationPollerRef.current on unmount; ensure functions referenced (normalizationApi.getJobStatus, setNormalizationJobStatus, setIsRunningNormalization, setNormalizationJobId, setNormalizationStatus, queryClient.invalidateQueries, setSuccessMessage, setLocalError) still operate the same.
♻️ Duplicate comments (1)
app/projects/[id]/settings/page.tsx (1)
117-118:⚠️ Potential issue | 🔴 CriticalRender
projectErrorto a string before composingerror.Line 118 can still propagate the query error object into JSX. When the project query fails before
localErroris set, both{error}render sites can throw instead of showing the failure state.Suggested fix
+ const projectErrorMessage = + projectError instanceof Error + ? projectError.message + : projectError + ? String(projectError) + : null; - const error = localError || projectError || null; + const error = localError || projectErrorMessage;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/projects/`[id]/settings/page.tsx around lines 117 - 118, The composite error assignment currently lets projectError (a non-serializable Error/object) flow into JSX via const error = localError || projectError || null; fix by coercing projectError to a safe string before composing error (e.g., derive const projectErrorString = projectError ? String(projectError) : null and then const error = localError || projectErrorString || null) so both places that render {error} always receive a string; update references to localError, projectError and error in page.tsx accordingly.
🤖 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/projects/`[id]/settings/page.tsx:
- Around line 598-599: The state updates close over stale joinRequests and
joinRequestCount; switch to functional state updates in the approve/decline
handlers so updates use the latest state — replace usages of
setJoinRequests(joinRequests.filter(...)) with setJoinRequests(prev =>
prev.filter(jr => jr.id !== requestId)) and setJoinRequestCount(Math.max(0,
joinRequestCount - 1)) with setJoinRequestCount(prev => Math.max(0, prev - 1));
apply the same change in the other handler(s) that update these states (the
approve/decline handlers around the second occurrence).
- Around line 223-252: The current useEffect using prSettingsApi.get,
joinRequestApi.list, and userSettingsApi.getGitHubTokenStatus can produce race
conditions and should be replaced with React Query useQuery hooks: create three
queries (prSettingsQuery, joinRequestsQuery, githubTokenStatusQuery) that use
projectId and session?.accessToken as query keys and set enabled to
Boolean(canManage && session?.accessToken) for PR/GitHub queries and
Boolean(isPublic && canManage && session?.accessToken) for join requests; move
state updates done by setPrSettings, setGithubIntegration,
setPrApprovalRequired, setJoinRequests, setJoinRequestCount, and
setHasGithubToken into the queries’ onSuccess handlers (or derive UI from
query.data directly) so React Query handles cancellation, deduping and stale
responses instead of the useEffect promise chains.
---
Outside diff comments:
In `@app/projects/`[id]/settings/page.tsx:
- Around line 748-785: The polling interval created inside the event handler is
never cleaned up on component unmount because the return cleanup is inside the
handler rather than a useEffect; fix by moving interval management to component
scope: create a ref (e.g., normalizationPollerRef) to hold the interval id,
start the interval in handleRunNormalization by assigning
normalizationPollerRef.current = setInterval(...), clear the interval whenever
the job finishes/fails inside that same polling callback
(clearInterval(normalizationPollerRef.current) and set
normalizationPollerRef.current = null), and add a useEffect cleanup that clears
normalizationPollerRef.current on unmount; ensure functions referenced
(normalizationApi.getJobStatus, setNormalizationJobStatus,
setIsRunningNormalization, setNormalizationJobId, setNormalizationStatus,
queryClient.invalidateQueries, setSuccessMessage, setLocalError) still operate
the same.
In `@lib/hooks/useRemoteSync.ts`:
- Around line 93-132: startPolling currently uses setInterval which can fire
overlapping async getJobStatus calls and captures the accessToken at invocation;
replace the interval with a single-flight recursive setTimeout loop that awaits
each remoteSyncApi.getJobStatus call before scheduling the next tick, read the
latest token from a mutable ref (e.g., accessTokenRef.current) inside the loop
instead of the closed-over accessToken, keep pollTimerRef to store/clear the
timeout id, stop the loop and clear pollTimerRef when jobIdRef or status becomes
complete/failed (and still call setIsChecking(false), setCheckError(...) and
await refetchAll()), and ensure cancellations clear the pending timeout to avoid
stray calls.
---
Duplicate comments:
In `@app/projects/`[id]/settings/page.tsx:
- Around line 117-118: The composite error assignment currently lets
projectError (a non-serializable Error/object) flow into JSX via const error =
localError || projectError || null; fix by coercing projectError to a safe
string before composing error (e.g., derive const projectErrorString =
projectError ? String(projectError) : null and then const error = localError ||
projectErrorString || null) so both places that render {error} always receive a
string; update references to localError, projectError and error in page.tsx
accordingly.
🪄 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: b094e574-65ed-48ef-91f5-5bac97888b5e
📒 Files selected for processing (5)
__tests__/lib/hooks/useQueryHooks.test.tsapp/projects/[id]/settings/page.tsxlib/hooks/useIndexStatus.tslib/hooks/useNormalizationStatus.tslib/hooks/useRemoteSync.ts
✅ Files skipped from review due to trivial changes (1)
- tests/lib/hooks/useQueryHooks.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/hooks/useIndexStatus.ts
- lib/hooks/useNormalizationStatus.ts
…tale closures Replace direct references to joinRequests and joinRequestCount with functional updaters (prev => ...) so the handlers always operate on the latest state, avoiding stale closure bugs when multiple requests are processed in quick succession. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The polling interval created in handleRunNormalization was stored in a local variable, so the return cleanup inside the handler had no effect. Move the interval ID to a normalizationPollRef and clear it in the existing unmount useEffect alongside reindexPollRef. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…olling setInterval can fire overlapping async getJobStatus calls if a request takes longer than the poll interval. Switch to recursive setTimeout that only schedules the next tick after the current one completes. Also read accessToken from a ref to avoid capturing a stale token at invocation time. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…tings data Replace the manual useEffect with three inline useQuery calls for PR settings, join requests, and GitHub token status. React Query handles cancellation, deduping, and stale response management, eliminating race conditions from the previous promise-chain pattern. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
JohnRDOrazio
dismissed
damienriehl’s stale review
April 12, 2026 22:45
completed, Claude Code posted a comment with issues that needed to addressed and they have been addressed
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
app/projects/[id]/settings/page.tsx (3)
3015-3029:⚠️ Potential issue | 🟡 MinorPolling callbacks capture
accessTokenfrom closure.The polling intervals in
EmbeddingSettingsSectioncaptureaccessTokenfrom the closure. If the parent component re-renders with a refreshed token, the polling callback will continue using the stale value. Consider using a ref pattern similar to the main component's polling fix.Also applies to: 3089-3106
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/projects/`[id]/settings/page.tsx around lines 3015 - 3029, The polling callback created in EmbeddingSettingsSection captures accessToken from closure, causing it to use a stale token after re-renders; fix by mirroring the main component's pattern: create an accessTokenRef and update it whenever accessToken changes, then use accessTokenRef.current inside the setInterval callback (and guard for null) when calling embeddingsApi.getStatus; ensure pollTimerRef and interval lifecycle remain the same so intervals are cleared/reset correctly when stopping or restarting polling.
765-800:⚠️ Potential issue | 🟠 MajorSame stale token issue applies to normalization polling.
The normalization job polling interval at line 765 also captures
session.accessTokenfrom the closure. Apply the same ref-based fix as suggested for the reindex polling to avoid stale token issues during long-running normalization jobs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/projects/`[id]/settings/page.tsx around lines 765 - 800, The normalization polling closure captures session.accessToken and can become stale; update the code to read the token from a ref instead of the closure (create/update accessTokenRef.current whenever session.accessToken changes) and use accessTokenRef.current in all calls inside the interval (normalizationApi.getJobStatus and normalizationApi.getStatus and queryClient.invalidateQueries usage), ensuring normalizationPollRef, setNormalizationJobId, setNormalizationJobStatus, and setIsRunningNormalization logic remains the same while the interval always uses the latest token.
668-686:⚠️ Potential issue | 🟠 MajorPolling callback captures
session.accessTokenfrom closure, risking stale token usage.The
setIntervalcallback at line 668 capturessession.accessTokenat creation time. If the NextAuth session refreshes during the polling period, subsequent API calls will use the stale token, potentially causing 401 errors.Store the token in a ref and read from it inside the callback, similar to the pattern mentioned in the PR commits for
useRemoteSync.Suggested fix
+ const accessTokenRef = useRef(session?.accessToken); + useEffect(() => { + accessTokenRef.current = session?.accessToken; + }, [session?.accessToken]); // In handleReindex: reindexPollRef.current = setInterval(async () => { try { + const token = accessTokenRef.current; + if (!token) return; const status = await projectOntologyApi.getIndexStatus( project.id, - session.accessToken + token ); // ...🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/projects/`[id]/settings/page.tsx around lines 668 - 686, The interval callback captures session.accessToken by closure and may use a stale token; change to store the latest token in a ref (e.g., tokenRef) and update tokenRef.current whenever session.accessToken changes, then inside the reindexPollRef setInterval callback read projectOntologyApi.getIndexStatus(project.id, tokenRef.current) so each poll uses the freshest token; keep clearing reindexPollRef, setIsReindexing, and queryClient.invalidateQueries(indexQueryKeys.status(projectId, tokenRef.current)) behavior but reference tokenRef.current instead of session.accessToken.
🧹 Nitpick comments (3)
app/projects/[id]/settings/page.tsx (3)
230-235: Query keys include the full accessToken, which may cause unnecessary refetches.Including
session?.accessTokendirectly in query keys means these queries will refetch whenever the token is refreshed (even if the user/project hasn't changed). Consider using a more stable identifier likeprojectIdalone, or a boolean!!session?.accessToken, since the token is only used for authorization, not data identity.Also applies to: 244-249, 257-262
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/projects/`[id]/settings/page.tsx around lines 230 - 235, The query keys currently embed the full access token (e.g., in prSettingsQuery defined with queryKey: ["prSettings", projectId, session?.accessToken]) which causes needless refetches on token rotation; change the queryKey to a stable identifier such as ["prSettings", projectId] or ["prSettings", projectId, !!session?.accessToken] and keep the queryFn using session!.accessToken! for authorization; apply the same change to the other queries mentioned (the similar query definitions at the blocks around lines 244-249 and 257-262) so none include the raw token in their queryKey.
2110-2127: Optional: Add cleanup for the delayed webhook setup call.The
setTimeoutat line 2112 can cause state updates on an unmounted component if the user navigates away quickly after toggling webhooks. Consider using a ref to track mounted state, or move this to a useEffect with cleanup.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/projects/`[id]/settings/page.tsx around lines 2110 - 2127, The delayed setTimeout that calls githubIntegrationApi.setupWebhook after setIsToggling may update state after the component unmounts; store the timeout id (returned by setTimeout) in a ref or move the delayed logic into a useEffect that depends on the toggling state (e.g., isToggling/projectId/accessToken) and clearTimeout in the cleanup or check an isMounted ref before calling setWebhookStatus, setWebhookMessage, onIntegrationUpdate, etc.; ensure you clear the timeout on unmount and before scheduling a new one so githubIntegrationApi.setupWebhook and the subsequent state updates won’t run on an unmounted component.
143-146: Consider deriving local state directly from query data instead of syncing via useEffect.The useEffect that copies
normalizationQueryDatatonormalizationStatus(and similarly forindexQueryData) adds an extra render cycle. Since the local state is used for optimistic updates during polling, consider usingqueryClient.setQueryData()for those updates instead, eliminating the need for separate local state.Also applies to: 171-174
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/projects/`[id]/settings/page.tsx around lines 143 - 146, The effect copies normalizationQueryData into local state (normalizationStatus) causing an extra render; remove the useState/useEffect pair for normalizationStatus (and the analogous indexStatus state) and derive UI values directly from normalizationQueryData and indexQueryData, or perform optimistic updates directly on the React Query cache via queryClient.setQueryData() in your polling/update handlers; update any references to normalizationStatus/indexStatus to use the query data or the cached value you set with queryClient.setQueryData() and delete setNormalizationStatus/setIndexStatus usages.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@app/projects/`[id]/settings/page.tsx:
- Around line 3015-3029: The polling callback created in
EmbeddingSettingsSection captures accessToken from closure, causing it to use a
stale token after re-renders; fix by mirroring the main component's pattern:
create an accessTokenRef and update it whenever accessToken changes, then use
accessTokenRef.current inside the setInterval callback (and guard for null) when
calling embeddingsApi.getStatus; ensure pollTimerRef and interval lifecycle
remain the same so intervals are cleared/reset correctly when stopping or
restarting polling.
- Around line 765-800: The normalization polling closure captures
session.accessToken and can become stale; update the code to read the token from
a ref instead of the closure (create/update accessTokenRef.current whenever
session.accessToken changes) and use accessTokenRef.current in all calls inside
the interval (normalizationApi.getJobStatus and normalizationApi.getStatus and
queryClient.invalidateQueries usage), ensuring normalizationPollRef,
setNormalizationJobId, setNormalizationJobStatus, and setIsRunningNormalization
logic remains the same while the interval always uses the latest token.
- Around line 668-686: The interval callback captures session.accessToken by
closure and may use a stale token; change to store the latest token in a ref
(e.g., tokenRef) and update tokenRef.current whenever session.accessToken
changes, then inside the reindexPollRef setInterval callback read
projectOntologyApi.getIndexStatus(project.id, tokenRef.current) so each poll
uses the freshest token; keep clearing reindexPollRef, setIsReindexing, and
queryClient.invalidateQueries(indexQueryKeys.status(projectId,
tokenRef.current)) behavior but reference tokenRef.current instead of
session.accessToken.
---
Nitpick comments:
In `@app/projects/`[id]/settings/page.tsx:
- Around line 230-235: The query keys currently embed the full access token
(e.g., in prSettingsQuery defined with queryKey: ["prSettings", projectId,
session?.accessToken]) which causes needless refetches on token rotation; change
the queryKey to a stable identifier such as ["prSettings", projectId] or
["prSettings", projectId, !!session?.accessToken] and keep the queryFn using
session!.accessToken! for authorization; apply the same change to the other
queries mentioned (the similar query definitions at the blocks around lines
244-249 and 257-262) so none include the raw token in their queryKey.
- Around line 2110-2127: The delayed setTimeout that calls
githubIntegrationApi.setupWebhook after setIsToggling may update state after the
component unmounts; store the timeout id (returned by setTimeout) in a ref or
move the delayed logic into a useEffect that depends on the toggling state
(e.g., isToggling/projectId/accessToken) and clearTimeout in the cleanup or
check an isMounted ref before calling setWebhookStatus, setWebhookMessage,
onIntegrationUpdate, etc.; ensure you clear the timeout on unmount and before
scheduling a new one so githubIntegrationApi.setupWebhook and the subsequent
state updates won’t run on an unmounted component.
- Around line 143-146: The effect copies normalizationQueryData into local state
(normalizationStatus) causing an extra render; remove the useState/useEffect
pair for normalizationStatus (and the analogous indexStatus state) and derive UI
values directly from normalizationQueryData and indexQueryData, or perform
optimistic updates directly on the React Query cache via
queryClient.setQueryData() in your polling/update handlers; update any
references to normalizationStatus/indexStatus to use the query data or the
cached value you set with queryClient.setQueryData() and delete
setNormalizationStatus/setIndexStatus usages.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d5d4ffd6-b8e3-4d60-aa79-18b085f46897
📒 Files selected for processing (2)
app/projects/[id]/settings/page.tsxlib/hooks/useRemoteSync.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/hooks/useRemoteSync.ts
All setInterval polling callbacks in the settings page (reindex, normalization, embeddings) captured session.accessToken by closure, which goes stale after token rotation. Store the latest token in a ref and read accessTokenRef.current inside each polling callback. Also: - Remove raw access tokens from React Query keys to avoid needless refetches on token rotation; the enabled guard already prevents queries from running unauthenticated. - Store webhook setup setTimeout in a ref and clear it on unmount to prevent state updates on unmounted components. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
app/projects/[id]/settings/page.tsx (1)
123-124:⚠️ Potential issue | 🔴 CriticalConvert
projectErrorto a string before composingerror.
localError || projectError || nullcan still produce a non-renderable object, which will break error rendering paths.Suggested fix
+ const projectErrorMessage = + projectError instanceof Error + ? projectError.message + : projectError + ? String(projectError) + : null; - const error = localError || projectError || null; + const error = localError || projectErrorMessage;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/projects/`[id]/settings/page.tsx around lines 123 - 124, The composed error value may be a non-renderable object because projectError can be an object; update the logic that sets error (the variable derived from localError || projectError || null) to convert projectError to a string before composing (e.g., use projectError?.message or String(projectError)) so error is always a renderable string when localError is falsy; adjust the assignment where localError, projectError, and error are defined to perform this conversion.
🤖 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/projects/`[id]/settings/page.tsx:
- Around line 107-112: The current code calls useProject and useMembers while
session.status may be "loading", causing unauthenticated requests; fix this by
ensuring the hooks only run after the session is hydrated: pass
session?.accessToken to useProject/useMembers only when session.status !==
"loading" (or add an explicit enabled/skip parameter to useProject and
useMembers and set enabled = session.status === "authenticated"), and update the
hooks to early-return / skip fetching when the token is not provided; reference
the useProject, useMembers calls and the session?.accessToken / session.status
checks and projectId so the hook invocations wait for token hydration before
issuing API requests.
- Around line 263-267: The githubTokenStatusQuery uses a global cache key
["githubTokenStatus"] which can be shared across different users; update the
queryKey passed to useQuery (githubTokenStatusQuery) to include a per-user
identifier (e.g., session?.user?.id or similar userId from session) such as
["githubTokenStatus", session?.user?.id] and keep the existing enabled check
(enabled: !!canManage && !!session?.accessToken) so the query is properly scoped
per user and avoids cross-account cache reuse.
---
Duplicate comments:
In `@app/projects/`[id]/settings/page.tsx:
- Around line 123-124: The composed error value may be a non-renderable object
because projectError can be an object; update the logic that sets error (the
variable derived from localError || projectError || null) to convert
projectError to a string before composing (e.g., use projectError?.message or
String(projectError)) so error is always a renderable string when localError is
falsy; adjust the assignment where localError, projectError, and error are
defined to perform this conversion.
🪄 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: 4e4b83c6-0322-43ff-8a77-fc9513ffbd09
📒 Files selected for processing (1)
app/projects/[id]/settings/page.tsx
…unt cache reuse Include session user ID in the query key so the GitHub token status cache is not shared across different user sessions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/projects/[id]/settings/page.tsx (1)
2121-2145:⚠️ Potential issue | 🟠 MajorCancel any pending auto-setup timer before disabling webhooks.
A quick enable → disable sequence still leaves the delayed
setupWebhook()callback armed. If that 100ms timer fires after the disable request succeeds, it can recreate/configure the webhook and writegithub_hook_idback into state even though webhooks are off.Suggested fix
const handleToggleWebhooks = async () => { if (!accessToken) return; + if (webhookSetupTimerRef.current) { + clearTimeout(webhookSetupTimerRef.current); + webhookSetupTimerRef.current = null; + } setIsToggling(true); setError(null); try { const updated = await githubIntegrationApi.update( @@ if (!updated.webhooks_enabled) { setWebhookSecret(null); setWebhookUrl(null); setWebhookStatus("unknown"); setWebhookMessage(null); } else { // When enabling, attempt auto-setup setIsToggling(false); // Small delay to let state settle (stored in ref for unmount cleanup) webhookSetupTimerRef.current = setTimeout(async () => { try { const result = await githubIntegrationApi.setupWebhook(projectId, accessToken); + webhookSetupTimerRef.current = null; setWebhookStatus(result.status as WebhookStatus); setWebhookMessage(result.message); if (result.github_hook_id) { onIntegrationUpdate({ ...updated, github_hook_id: result.github_hook_id, }); } } catch { + webhookSetupTimerRef.current = null; setWebhookStatus("unknown"); } }, 100); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/projects/`[id]/settings/page.tsx around lines 2121 - 2145, When handling the case where updated.webhooks_enabled is false, clear any pending auto-setup timer before mutating state: if webhookSetupTimerRef.current exists, call clearTimeout(webhookSetupTimerRef.current) and set webhookSetupTimerRef.current = null, then proceed to setWebhookSecret(null), setWebhookUrl(null), setWebhookStatus("unknown") and setWebhookMessage(null). This prevents the delayed callback that calls githubIntegrationApi.setupWebhook (the async function started in the setTimeout stored in webhookSetupTimerRef) from running after webhooks have been disabled.
♻️ Duplicate comments (1)
app/projects/[id]/settings/page.tsx (1)
123-124:⚠️ Potential issue | 🟠 MajorConvert
projectErrorto a renderable string before using it inerror.This is still composing a potentially non-renderable query error into the JSX path. When
localErroris empty, the failure UI can throw instead of showing the actual error message.Suggested fix
+ const projectErrorMessage = + projectError instanceof Error + ? projectError.message + : projectError + ? String(projectError) + : null; // Derive displayed error: local mutation errors take priority, then query errors - const error = localError || projectError || null; + const error = localError || projectErrorMessage;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/projects/`[id]/settings/page.tsx around lines 123 - 124, The computed error variable may include a non-renderable GraphQL/query error (projectError); update the assignment so projectError is converted to a plain string before use (e.g., derive a message from projectError.message or safely stringify its relevant fields) and then set const error = localError || projectErrorMessage || null so the JSX always receives a renderable string; update references to projectError in this file (the localError, projectError, error variables near the displayed error derivation) accordingly.
🤖 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/projects/`[id]/settings/page.tsx:
- Around line 256-261: The local state updates in handleApproveJoinRequest and
handleDeclineJoinRequest are being overwritten by stale react-query cache; after
you update local state in each handler, call queryClient.invalidateQueries({
queryKey: ["joinRequests", projectId] }) to invalidate the cached
["joinRequests", projectId] query so it refetches and won’t resurrect removed
requests—add this invalidateQueries call in both handleApproveJoinRequest and
handleDeclineJoinRequest immediately after the state mutations.
---
Outside diff comments:
In `@app/projects/`[id]/settings/page.tsx:
- Around line 2121-2145: When handling the case where updated.webhooks_enabled
is false, clear any pending auto-setup timer before mutating state: if
webhookSetupTimerRef.current exists, call
clearTimeout(webhookSetupTimerRef.current) and set webhookSetupTimerRef.current
= null, then proceed to setWebhookSecret(null), setWebhookUrl(null),
setWebhookStatus("unknown") and setWebhookMessage(null). This prevents the
delayed callback that calls githubIntegrationApi.setupWebhook (the async
function started in the setTimeout stored in webhookSetupTimerRef) from running
after webhooks have been disabled.
---
Duplicate comments:
In `@app/projects/`[id]/settings/page.tsx:
- Around line 123-124: The computed error variable may include a non-renderable
GraphQL/query error (projectError); update the assignment so projectError is
converted to a plain string before use (e.g., derive a message from
projectError.message or safely stringify its relevant fields) and then set const
error = localError || projectErrorMessage || null so the JSX always receives a
renderable string; update references to projectError in this file (the
localError, projectError, error variables near the displayed error derivation)
accordingly.
🪄 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: 2c0eb1c4-2bf5-4621-893d-48f17c20fdfe
📒 Files selected for processing (1)
app/projects/[id]/settings/page.tsx
…ebhook timer on disable - Invalidate the ["joinRequests", projectId] query after approving or declining a join request so background refetches don't resurrect removed items via the useEffect sync. - Clear the pending webhook auto-setup timer when disabling webhooks to prevent the delayed setupWebhook call from firing after webhooks have been turned off. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
6 tasks
This was referenced Apr 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
useMembers,useLintSummary,useNormalizationStatus,useIndexStatus,useOpenPRCount,usePendingSuggestionCount— each with query key factories following theuseProjectconventionuseEffect/Promise.allfetch with React Query hooks +invalidateQueriesafter mutations (update project, add/remove/update members, transfer ownership, reindex, normalization)useState/useEffecttouseQueryfor config/history +useMutationfor save/delete; retain imperative interval polling for background jobsuseState+ 2useEffectfetch blocks with the new standalone query hooksAddresses #89
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Refactor
Tests