Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughModified Changes
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (1 warning, 2 inconclusive)
✅ 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/hooks/useClerkAttachAuthInterceptorsToAxios.tsx`:
- Around line 69-72: The code computing direct in
useClerkAttachAuthInterceptorsToAxios.tsx uses
parseBoolean(proxyDisableProperty[0].value) which can throw on malformed backend
values; wrap the parse call in a safe non-throwing guard (e.g., try/catch) or
replace with a non-throwing parser helper so that if parseBoolean throws or
returns invalid, you fall back to orgDirect, ensuring render never throws;
update the expression that sets direct (and any helper around parseBoolean) to
catch parsing errors and return orgDirect as the default.
- Around line 69-72: The computed "direct" value in
useClerkAttachAuthInterceptorsToAxios currently falls back to orgDirect while
the proxyDisableProperty query may still be loading, which can prematurely
enable direct interceptors; change the logic in the direct calculation
(referencing proxyDisableProperty, orgDirect, and the direct variable) to only
use orgDirect when the proxyDisableProperty result is resolved (e.g., check that
proxyDisableProperty is defined and not loading) and otherwise keep direct in a
safe default state (not enabling direct mode) until the override property is
available so interceptors are only attached after the query result is known.
- Around line 56-58: The queryKey used in useQuery (currently
["properties","proxy.disable"]) is too broad and allows cached proxy.disable
values to be reused across different backend/org contexts; update the queryKey
to include the per-org/backend context (for example include backendUrl or orgId)
so it becomes something like ["properties", backendUrl, "proxy.disable"] or
["properties", orgId, "proxy.disable"], and ensure the queryFn continues to use
the same backendUrl variable so the cache is correctly scoped to the current
org/backend context.
| const { data: proxyDisableProperty } = useQuery({ | ||
| queryKey: ["properties", "proxy.disable"], | ||
| queryFn: async () => { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/hooks/useClerkAttachAuthInterceptorsToAxios.tsx | head -100Repository: flanksource/flanksource-ui
Length of output: 3716
🏁 Script executed:
cat -n src/hooks/useClerkAttachAuthInterceptorsToAxios.tsx | tail -50Repository: flanksource/flanksource-ui
Length of output: 2150
🏁 Script executed:
rg -A 10 "export.*apiBase" --type ts --type tsxRepository: flanksource/flanksource-ui
Length of output: 97
🏁 Script executed:
rg -A 10 "export.*apiBase" --type tsRepository: flanksource/flanksource-ui
Length of output: 527
🏁 Script executed:
rg "queryKey.*properties" --type ts -B 2 -A 5Repository: flanksource/flanksource-ui
Length of output: 2590
🏁 Script executed:
rg -i "proxy.disable\|properties.*api" --type ts -B 3 -A 3 | head -50Repository: flanksource/flanksource-ui
Length of output: 52
🏁 Script executed:
cd src && find . -name "*.ts" -o -name "*.tsx" | xargs grep -l "useQuery" | head -5 | while read f; do echo "=== $f ==="; rg "useQuery\({" "$f" -A 8 | head -30; doneRepository: flanksource/flanksource-ui
Length of output: 731
🏁 Script executed:
rg "queryKey.*\[" src/hooks/ReactQuery/Properties.tsx src/api/query-hooks/useFeatureFlags.tsx -A 2 -B 2Repository: flanksource/flanksource-ui
Length of output: 1627
🏁 Script executed:
cat -n src/api/axios.ts | head -50Repository: flanksource/flanksource-ui
Length of output: 1654
🏁 Script executed:
rg "useOrganization\|organization\?.id" src/hooks/useClerkAttachAuthInterceptorsToAxios.tsx -B 2 -A 2Repository: flanksource/flanksource-ui
Length of output: 52
🏁 Script executed:
rg "QueryClientProvider\|queryClient\|invalidateQueries" src --type ts -B 2 -A 5 | head -60Repository: flanksource/flanksource-ui
Length of output: 52
🏁 Script executed:
rg "enabled.*backendUrl\|staleTime\|gcTime" src/hooks/useClerkAttachAuthInterceptorsToAxios.tsx -B 3 -A 3Repository: flanksource/flanksource-ui
Length of output: 52
🏁 Script executed:
rg "featureFlag" src/api/query-hooks/useFeatureFlags.tsx -B 5 -A 5 | head -40Repository: flanksource/flanksource-ui
Length of output: 462
Scope the React Query cache key by backend/org context to prevent stale data reuse across org switches.
The static queryKey at line 57 causes React Query to reuse cached proxy.disable data from a previous organization when switching orgs, since the queryKey remains unchanged even though backendUrl differs per organization. With the default 5-minute cache time, this stale data persists.
Suggested fix
const { data: proxyDisableProperty } = useQuery({
- queryKey: ["properties", "proxy.disable"],
+ queryKey: ["properties", organization?.id, backendUrl, "proxy.disable"],
queryFn: async () => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { data: proxyDisableProperty } = useQuery({ | |
| queryKey: ["properties", "proxy.disable"], | |
| queryFn: async () => { | |
| const { data: proxyDisableProperty } = useQuery({ | |
| queryKey: ["properties", organization?.id, backendUrl, "proxy.disable"], | |
| queryFn: async () => { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/useClerkAttachAuthInterceptorsToAxios.tsx` around lines 56 - 58,
The queryKey used in useQuery (currently ["properties","proxy.disable"]) is too
broad and allows cached proxy.disable values to be reused across different
backend/org contexts; update the queryKey to include the per-org/backend context
(for example include backendUrl or orgId) so it becomes something like
["properties", backendUrl, "proxy.disable"] or ["properties", orgId,
"proxy.disable"], and ensure the queryFn continues to use the same backendUrl
variable so the cache is correctly scoped to the current org/backend context.
| const direct = | ||
| proxyDisableProperty && proxyDisableProperty.length > 0 | ||
| ? parseBoolean(proxyDisableProperty[0].value) | ||
| : orgDirect; |
There was a problem hiding this comment.
Handle malformed proxy.disable values without throwing in render.
Line 71 parses a backend-provided value with a throwing parser. A bad property value (e.g. whitespace/typo) will throw during render and break this hook.
Suggested fix
function parseBoolean(value: unknown): boolean {
if (typeof value === "string") {
- return value.toLowerCase() === "true";
+ const normalized = value.trim().toLowerCase();
+ if (normalized === "true") return true;
+ if (normalized === "false") return false;
+ return false;
}
@@
- throw new Error("Invalid boolean value");
+ return false;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/useClerkAttachAuthInterceptorsToAxios.tsx` around lines 69 - 72,
The code computing direct in useClerkAttachAuthInterceptorsToAxios.tsx uses
parseBoolean(proxyDisableProperty[0].value) which can throw on malformed backend
values; wrap the parse call in a safe non-throwing guard (e.g., try/catch) or
replace with a non-throwing parser helper so that if parseBoolean throws or
returns invalid, you fall back to orgDirect, ensuring render never throws;
update the expression that sets direct (and any helper around parseBoolean) to
catch parsing errors and return orgDirect as the default.
Avoid enabling direct mode before the override property is resolved.
Lines 69-72 default to orgDirect while the query is still loading. That can briefly attach direct interceptors and route requests to the backend before proxy.disable=false arrives.
Suggested fix
- const { data: proxyDisableProperty } = useQuery({
+ const { data: proxyDisableProperty, isLoading: isProxyDisableLoading } = useQuery({
@@
- const direct =
- proxyDisableProperty && proxyDisableProperty.length > 0
+ const direct =
+ backendUrl && isProxyDisableLoading
+ ? false
+ : proxyDisableProperty && proxyDisableProperty.length > 0
? parseBoolean(proxyDisableProperty[0].value)
: orgDirect;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/useClerkAttachAuthInterceptorsToAxios.tsx` around lines 69 - 72,
The computed "direct" value in useClerkAttachAuthInterceptorsToAxios currently
falls back to orgDirect while the proxyDisableProperty query may still be
loading, which can prematurely enable direct interceptors; change the logic in
the direct calculation (referencing proxyDisableProperty, orgDirect, and the
direct variable) to only use orgDirect when the proxyDisableProperty result is
resolved (e.g., check that proxyDisableProperty is defined and not loading) and
otherwise keep direct in a safe default state (not enabling direct mode) until
the override property is available so interceptors are only attached after the
query result is known.
Fixes: #2832
Summary by CodeRabbit