feat(mcp-guardrails): ui changes - #5366
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds shared redaction mapping utilities and types, gates MCP log value reveal through RBAC, and updates log detail views to display original values when permitted. ChangesLog redaction reveal
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant MCPLogsPage
participant MCPLogDetailSheet
participant RedactionUtilities
participant CodeEditors
MCPLogsPage->>MCPLogDetailSheet: pass reveal permission
MCPLogDetailSheet->>RedactionUtilities: apply enabled mappings
RedactionUtilities-->>MCPLogDetailSheet: return displayed values
MCPLogDetailSheet->>CodeEditors: render arguments, result, and error details
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Confidence Score: 4/5The redaction reconstruction path can display the wrong sensitive value and needs a fix before merging.
ui/lib/utils/redaction.ts and ui/lib/utils/redaction.test.ts
|
| Filename | Overview |
|---|---|
| ui/lib/utils/redaction.ts | Adds shared reveal helpers, but mixed-phase collisions and sequential substitution can produce incorrect values. |
| ui/lib/utils/redaction.test.ts | Adds basic helper coverage but codifies output-last collisions and omits chained replacement cases. |
| ui/app/workspace/mcp-logs/views/mcpLogDetailsSheet.tsx | Adds a reveal toggle, phase-specific mapping, and state reset when the selected log changes. |
| ui/app/workspace/mcp-logs/page.tsx | Passes the Logs reveal permission into the MCP details sheet. |
| ui/lib/types/logs.ts | Extracts the shared mapping type and adds it to MCP detail records. |
| ui/app/workspace/logs/sheets/logDetailView.tsx | Replaces local redaction helpers with the shared utility. |
Reviews (1): Last reviewed commit: "feat(mcp-guardrails): ui changes" | Re-trigger Greptile
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ui/app/workspace/mcp-logs/views/mcpLogDetailsSheet.tsx (1)
333-358: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse nullish checks for revealed text sections.
A valid redaction mapping can restore a string to
"". The current truthiness guards then remove the Arguments or Result section after reveal. Check explicitly forundefined/nullso empty content remains visible.Proposed fix
- {displayedArguments && ( + {displayedArguments !== undefined && displayedArguments !== null && ( ... - {displayedResult && displayLog.status !== "processing" && ( + {displayedResult !== undefined && displayedResult !== null && displayLog.status !== "processing" && (🤖 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 `@ui/app/workspace/mcp-logs/views/mcpLogDetailsSheet.tsx` around lines 333 - 358, Update the displayedArguments and displayedResult rendering guards in the MCP log details view to check only for nullish values, not truthiness. Preserve the Arguments and Result sections when reveal resolves their content to an empty string, while still hiding them when the corresponding value is null or undefined.
🤖 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 `@ui/app/workspace/mcp-logs/views/mcpLogDetailsSheet.tsx`:
- Around line 155-162: Add an accessible name to the reveal Switch in the
revealAvailable block, using aria-label="Show original values" or associating
the visible text through aria-labelledby/Label and a matching control
identifier. Preserve the existing revealEnabled state and onCheckedChange
behavior.
- Line 86: Scope showRevealedValues to the current displayLog in the MCP log
details view so navigation cannot briefly reveal the new log’s original values.
Update the state handling around displayLog.id to reset immediately on log
changes, or key the relevant component by displayLog.id, while preserving the
existing reveal toggle behavior within one active log.
In `@ui/lib/utils/redaction.ts`:
- Around line 9-16: Update applyRedactionMapping to perform one replacement pass
over the original text using a token-matching regular expression and a callback
that looks up each token’s mapping value. Ensure replacement values are returned
literally, without interpreting $ sequences, and prevent mapped values from
being processed again.
---
Outside diff comments:
In `@ui/app/workspace/mcp-logs/views/mcpLogDetailsSheet.tsx`:
- Around line 333-358: Update the displayedArguments and displayedResult
rendering guards in the MCP log details view to check only for nullish values,
not truthiness. Preserve the Arguments and Result sections when reveal resolves
their content to an empty string, while still hiding them when the corresponding
value is null or undefined.
🪄 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 Plus
Run ID: 78cfbebd-e605-48fe-81b2-cd73aa3bca64
📒 Files selected for processing (6)
ui/app/workspace/logs/sheets/logDetailView.tsxui/app/workspace/mcp-logs/page.tsxui/app/workspace/mcp-logs/views/mcpLogDetailsSheet.tsxui/lib/types/logs.tsui/lib/utils/redaction.test.tsui/lib/utils/redaction.ts
69f47a6 to
2c028ab
Compare
794c468 to
ff17a2e
Compare
ff17a2e to
3973b8f
Compare
3973b8f to
ac9aab8
Compare
2c028ab to
f17ae4d
Compare
ac9aab8 to
5ba4636
Compare
f17ae4d to
3cd7f5b
Compare
5ba4636 to
459e718
Compare
3cd7f5b to
04bc854
Compare
Merge activity
|
The base branch was changed.

Summary
Adds a "Show original values" toggle to the MCP log detail sheet, allowing users with the
Logs:Revealpermission to view redacted placeholders replaced with their original values. Redaction utility functions are extracted into a shared module and extended to support recursive reveal over structured JSON-like values.Changes
hasRedactionMappingEntriesandapplyRedactionMappingfromlogDetailView.tsxinto a new sharedui/lib/utils/redaction.tsmodule, removing the inline definitions.applyRedactionMappingToValuefor recursive reveal over nested objects and arrays without mutating the fetched log data.mergeRedactionMappingsto combine input and output phase maps for fields like error details that may contain content from both phases.RedactionMappinginterface inlogs.tsand applied it to bothLogEntryandMCPToolLogEntry(the latter now includesredaction_mappingon detail responses).canRevealprop toMCPLogDetailSheet, gated by theLogs:RevealRBAC permission checked inMCPLogsPage.Switchtoggle labeled "Show original values" in the sheet header when reveal data is available; toggling it applies the redaction mapping to arguments, result, and error details before display.hasRedactionMappingEntries,applyRedactionMappingToValue(including immutability), andmergeRedactionMappingsconflict resolution.Type of change
Affected areas
How to test
Logs:Revealpermission.[EMAIL-1]).Logs:Reveal, confirm the toggle does not appear.Screenshots/Recordings
Add before/after screenshots showing the toggle and revealed values in the MCP log detail sheet.
Breaking changes
Related issues
Link related issues here.
Security considerations
The reveal toggle is gated behind the
Logs:RevealRBAC permission. Original values are only substituted client-side when the backend includesredaction_mappingin the response, which itself is only returned to callers with the appropriate permission. No original values are persisted or logged by the UI.Checklist
docs/contributing/README.mdand followed the guidelines