Guardrails: Prevent Editing and Deleting Configs#185
Merged
Conversation
📝 WalkthroughWalkthroughThis PR removes config-deletion functionality and simplifies the save flow to POST-only. Saved configs display in read-only mode within the ValidatorConfigPanel. Field components gain validation (BanListModal description check), enhanced UI (BanListField clipboard copy and textarea), and read-only details display (TopicRelevanceField). Supporting types and utility functions are updated and cleaned up. ChangesConfig Save-Only and Read-Only View Mode
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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.
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/components/ui/MultiSelect.tsx (1)
67-78:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDisabled state can still mutate selections when dropdown is already open.
When
disabledflips totruewhile open, option buttons remain interactive andtoggle/removestill callonChange. This breaks read-only guarantees.Suggested fix
@@ useEffect(() => { + if (disabled) { + setOpen(false); + } + }, [disabled]); + + useEffect(() => { const handler = (e: MouseEvent) => { @@ const toggle = (opt: string) => { + if (disabled) return; if (value.includes(opt)) { onChange(value.filter((v) => v !== opt)); } else { onChange([...value, opt]); } }; @@ const remove = (opt: string, e: React.MouseEvent | React.KeyboardEvent) => { + if (disabled) return; e.stopPropagation(); onChange(value.filter((v) => v !== opt)); }; @@ <button @@ - onClick={() => toggle(optValue)} + onClick={() => !disabled && toggle(optValue)}Also applies to: 183-209
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/components/ui/MultiSelect.tsx` around lines 67 - 78, The toggle and remove handlers currently call onChange even when the component is disabled; update the functions (toggle and remove) to early-return when the component's disabled prop is true, e.g. if (disabled) return;, and likewise add the same disabled guard to any inline option click/keyboard handlers (the option button elements and the tag remove buttons referenced later around the option rendering) so no events call onChange when disabled; also ensure you stopPropagation and preserve accessibility attributes (aria-disabled) on those interactive elements.
🧹 Nitpick comments (1)
app/components/guardrails/BanListField.tsx (1)
201-221: ⚡ Quick winUse a native
<button>for the copy action.The current clickable element is a
<span role="button">. Switching to<button type="button">improves keyboard/screen-reader semantics and reduces custom key handling.Suggested diff
- <span - role="button" - tabIndex={0} - onClick={handleCopy} - onKeyDown={(e) => { - if (e.key === "Enter" || e.key === " ") { - e.preventDefault(); - handleCopy(); - } - }} + <button + type="button" + onClick={handleCopy} aria-label={copied ? "Copied" : "Copy banned words"} title={copied ? "Copied" : "Copy comma-separated list"} className="shrink-0 inline-flex items-center gap-1 px-2 py-1 rounded-md text-[11px] font-medium text-accent-primary bg-accent-primary/5 hover:bg-accent-primary/10 transition-colors cursor-pointer" > {copied ? ( <CheckLineIcon className="w-3 h-3 text-status-success" /> ) : ( <CopyIcon className="w-3 h-3" /> )} {copied ? "Copied" : "Copy"} - </span> + </button>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/components/guardrails/BanListField.tsx` around lines 201 - 221, Replace the interactive <span role="button"> with a native <button type="button"> to improve accessibility: remove role and tabIndex and drop the custom onKeyDown handler, keep the onClick={handleCopy}, aria-label and title (using copied to toggle values), preserve the className and children (the conditional icons CheckLineIcon / CopyIcon and the copied ? "Copied" : "Copy" text), and ensure handleCopy and copied state/props remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/`(main)/guardrails/page.tsx:
- Around line 136-141: The POST body currently serializes only configValues when
calling guardrailsFetch; include the config name so the backend receives the
label — update the call in the guardrailsFetch request (where you build the body
for /api/guardrails/validators/configs) to merge the validated name into the
payload (e.g., combine name with configValues) so ValidatorConfigPanel's
validated name is sent and SavedConfigsList can render it.
---
Outside diff comments:
In `@app/components/ui/MultiSelect.tsx`:
- Around line 67-78: The toggle and remove handlers currently call onChange even
when the component is disabled; update the functions (toggle and remove) to
early-return when the component's disabled prop is true, e.g. if (disabled)
return;, and likewise add the same disabled guard to any inline option
click/keyboard handlers (the option button elements and the tag remove buttons
referenced later around the option rendering) so no events call onChange when
disabled; also ensure you stopPropagation and preserve accessibility attributes
(aria-disabled) on those interactive elements.
---
Nitpick comments:
In `@app/components/guardrails/BanListField.tsx`:
- Around line 201-221: Replace the interactive <span role="button"> with a
native <button type="button"> to improve accessibility: remove role and tabIndex
and drop the custom onKeyDown handler, keep the onClick={handleCopy}, aria-label
and title (using copied to toggle values), preserve the className and children
(the conditional icons CheckLineIcon / CopyIcon and the copied ? "Copied" :
"Copy" text), and ensure handleCopy and copied state/props remain unchanged.
🪄 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: 631652f6-784d-4365-99c7-c6eaea0ee3a6
📒 Files selected for processing (13)
app/(main)/guardrails/page.tsxapp/api/guardrails/validators/configs/[config_id]/route.tsapp/components/guardrails/BanListField.tsxapp/components/guardrails/BanListModal.tsxapp/components/guardrails/DeleteConfigModal.tsxapp/components/guardrails/SavedConfigsList.tsxapp/components/guardrails/TopicRelevanceField.tsxapp/components/guardrails/TopicRelevanceModal.tsxapp/components/guardrails/ValidatorConfigPanel.tsxapp/components/ui/MultiSelect.tsxapp/globals.cssapp/lib/types/guardrails.tsapp/lib/utils/guardrails.ts
💤 Files with no reviewable changes (3)
- app/components/guardrails/DeleteConfigModal.tsx
- app/lib/utils/guardrails.ts
- app/api/guardrails/validators/configs/[config_id]/route.ts
Prajna1999
approved these changes
Jun 1, 2026
vprashrex
approved these changes
Jun 1, 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
3 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.
Issue: #180 #181
Summary: