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 52 minutes and 8 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)
📝 WalkthroughWalkthroughThis PR integrates API key-based authentication throughout the guardrails components by extracting the active API key from the auth context and threading it through all Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/components/guardrails/BanListField.tsx (1)
55-88:⚠️ Potential issue | 🟠 Major
apiKeymissing from effect dependencies — ban list/word fetches won't retry after auth hydrates.
fetchBanListsis invoked in a mount-only effect ([]on Line 80), andfetchBannedWordsdepends only on[value](Line 88). Both close overapiKey, which is""on the first render whenactiveKeyhasn't populated yet. OnceuseAuth()hydrates andapiKeychanges, neither effect re-runs, so the requests are permanently sent withoutX-API-KEYand the user sees "Failed to load ban lists" until a remount.This is the most impactful issue in this PR — unlike
guardrails/page.tsx, which correctly includesapiKeyin its dep arrays, this component will be broken for users whose auth context isn't ready by mount.Proposed fix
useEffect(() => { + if (!apiKey) return; fetchBanLists(); -}, []); + // eslint-disable-next-line react-hooks/exhaustive-deps +}, [apiKey]); useEffect(() => { - if (value) { + if (value && apiKey) { fetchBannedWords(value); } else { setBannedWords([]); } -}, [value]); + // eslint-disable-next-line react-hooks/exhaustive-deps +}, [value, apiKey]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/guardrails/BanListField.tsx` around lines 55 - 88, The effects that call fetchBanLists and fetchBannedWords close over apiKey but do not include it in their dependency arrays, so add apiKey to both useEffect deps (the mount effect that currently calls fetchBanLists and the effect that calls fetchBannedWords when value changes) so they re-run when auth hydrates; ensure fetchBanLists and fetchBannedWords continue to read the apiKey param (or accept apiKey as an explicit argument) so the requests include the correct X-API-KEY after hydration.app/(main)/guardrails/page.tsx (1)
60-71:⚠️ Potential issue | 🟡 MinorValidators load fires before auth is hydrated.
Unlike the verify effect above (Line 41), this effect has no
isHydratedguard, so on first mountapiKeyis""and/api/guardrailsis called withoutX-API-KEY. It will re-run onceapiKeyis set (dep array is correct), but the initial unauthenticated request is wasted and may toast"Failed to load validators"transiently. Consider gating onisHydratedand a non-emptyapiKey:Proposed fix
useEffect(() => { + if (!isHydrated || !apiKey) return; setValidatorsLoading(true); guardrailsFetch<{ validators?: Validator[] }>("/api/guardrails", apiKey) ... -}, [apiKey]); +}, [isHydrated, apiKey]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/`(main)/guardrails/page.tsx around lines 60 - 71, The effect that loads validators is firing before auth is hydrated; update the useEffect (the effect that calls guardrailsFetch and uses setValidatorsLoading, setValidators) to bail out unless isHydrated is true and apiKey is non-empty: add a guard at the top of the effect that returns early when !isHydrated or apiKey === "" so the initial unauthenticated request (and transient toast from guardrailsFetch .catch) is avoided, keeping the rest of the logic (then/catch/finally) unchanged.app/components/prompt-editor/ConfigEditorPane.tsx (1)
74-89:⚠️ Potential issue | 🟡 MinorVerify request may fire with an empty
apiKey.When the component mounts before auth is hydrated (or when the parent hasn't passed
apiKeyyet), this effect calls/api/apikeys/verifywithapiKey="", resulting in an unauthenticated request. The effect will re-run whenapiKeybecomes non-empty, but the initial call is wasted. Consider early-returning whenapiKeyis empty:Proposed fix
useEffect(() => { + if (!apiKey) return; guardrailsFetch<{ data?: { organization_id: number; project_id: number } }>( "/api/apikeys/verify", apiKey, ) ... }, [apiKey]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/prompt-editor/ConfigEditorPane.tsx` around lines 74 - 89, The effect in ConfigEditorPane calls guardrailsFetch("/api/apikeys/verify", apiKey) even when apiKey is empty; update the useEffect callback to early-return if apiKey is falsy (e.g., empty string) before calling guardrailsFetch so no unauthenticated request is sent, preserving the existing logic that setsGuardrailsQueryString from the response once a valid apiKey is available.
🧹 Nitpick comments (1)
app/lib/guardrailsClient.ts (1)
63-72: Consider makingapiKeyoptional for ergonomics.Changing
apiKeyto a required positionalstringforces every caller to explicitly pass""when no key is available (and already several call sites doactiveKey?.key ?? ""). Making it optional (apiKey?: string) would be equivalent at runtime (theif (apiKey)guard already handles falsy values) and reduce noise at call sites. Non-blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/guardrailsClient.ts` around lines 63 - 72, The function guardrailsFetch currently requires apiKey: string which forces callers to pass empty strings; change the signature to apiKey?: string so the parameter is optional (the existing if (apiKey) guard and headers.set("X-API-KEY", apiKey) behavior remain correct), update any TypeScript usages/types accordingly, and run a quick grep for guardrailsFetch call sites to remove redundant ?? "" where present.
🤖 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/`(main)/guardrails/page.tsx:
- Around line 60-71: The effect that loads validators is firing before auth is
hydrated; update the useEffect (the effect that calls guardrailsFetch and uses
setValidatorsLoading, setValidators) to bail out unless isHydrated is true and
apiKey is non-empty: add a guard at the top of the effect that returns early
when !isHydrated or apiKey === "" so the initial unauthenticated request (and
transient toast from guardrailsFetch .catch) is avoided, keeping the rest of the
logic (then/catch/finally) unchanged.
In `@app/components/guardrails/BanListField.tsx`:
- Around line 55-88: The effects that call fetchBanLists and fetchBannedWords
close over apiKey but do not include it in their dependency arrays, so add
apiKey to both useEffect deps (the mount effect that currently calls
fetchBanLists and the effect that calls fetchBannedWords when value changes) so
they re-run when auth hydrates; ensure fetchBanLists and fetchBannedWords
continue to read the apiKey param (or accept apiKey as an explicit argument) so
the requests include the correct X-API-KEY after hydration.
In `@app/components/prompt-editor/ConfigEditorPane.tsx`:
- Around line 74-89: The effect in ConfigEditorPane calls
guardrailsFetch("/api/apikeys/verify", apiKey) even when apiKey is empty; update
the useEffect callback to early-return if apiKey is falsy (e.g., empty string)
before calling guardrailsFetch so no unauthenticated request is sent, preserving
the existing logic that setsGuardrailsQueryString from the response once a valid
apiKey is available.
---
Nitpick comments:
In `@app/lib/guardrailsClient.ts`:
- Around line 63-72: The function guardrailsFetch currently requires apiKey:
string which forces callers to pass empty strings; change the signature to
apiKey?: string so the parameter is optional (the existing if (apiKey) guard and
headers.set("X-API-KEY", apiKey) behavior remain correct), update any TypeScript
usages/types accordingly, and run a quick grep for guardrailsFetch call sites to
remove redundant ?? "" where present.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b0468bf2-93db-48db-8e44-4ed1e967b960
📒 Files selected for processing (6)
app/(main)/guardrails/page.tsxapp/components/guardrails/BanListField.tsxapp/components/guardrails/BanListModal.tsxapp/components/guardrails/SavedConfigsList.tsxapp/components/prompt-editor/ConfigEditorPane.tsxapp/lib/guardrailsClient.ts
2 participants
Previous PR Reference: #116
Summary
X-API-KEY+ access token forwarding inguardrailsFetch.