feat: add permission request UI for sensitive tool operations#1200
Open
Gkrumbach07 wants to merge 9 commits intomainfrom
Open
feat: add permission request UI for sensitive tool operations#1200Gkrumbach07 wants to merge 9 commits intomainfrom
Gkrumbach07 wants to merge 9 commits intomainfrom
Conversation
When Claude Code SDK needs user approval for sensitive operations (e.g. editing .mcp.json), the can_use_tool callback now emits a synthetic PermissionRequest tool call that halts the stream and surfaces an interactive Allow/Deny UI in the frontend. Approved operations are tracked per-session so retries succeed automatically. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughAdds a permission-request human-in-the-loop flow: new PermissionRequest tool input and frontend UI, shared result-presence helper, and adapter/runtime plumbing that emits synthetic permission events, awaits user approval, and resumes or denies tool execution accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant Claude as Claude SDK
participant Adapter as ClaudeAgentAdapter
participant WorkerQueue as SessionWorker Output Queue
participant Frontend as Frontend UI
participant User as User
Claude->>Adapter: can_use_tool(tool_call)
Adapter->>Adapter: compute permission_key (_PERM prefix)
alt key in _approved_operations
Adapter->>Claude: allow tool usage
else not approved
Adapter->>WorkerQueue: inject synthetic PermissionRequest (ToolCallStart/Args/End)
Adapter->>Claude: deny tool usage
WorkerQueue->>Frontend: deliver PermissionRequest event
Frontend->>Frontend: render PermissionRequestMessage
Frontend->>User: display prompt (Allow/Deny)
User->>Frontend: respond (approved/denied)
Frontend->>WorkerQueue: submit JSON response (approved, key)
WorkerQueue->>Adapter: deliver permission response
Adapter->>Adapter: update _approved_operations if approved
Adapter->>Claude: retry tool call (allow/deny)
end
Claude->>Claude: execute tool (if allowed)
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
- Remove hasResult alias, use hasToolResult directly - Collapse config/resolvedConfig/activeConfig triple to single lookup - Remove redundant hasattr/isinstance guards on ToolCallEndEvent - Use getattr for placeholder ID checks - Clear _permission_worker in bridge finally block (prevent GC leak) - Remove dead description initializer - Add PermissionRequest to use-agent-status.ts waiting_input detection Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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 `@components/runners/ambient-runner/ag_ui_claude_sdk/adapter.py`:
- Around line 976-992: The conditional uses
message.tool_call_id.startswith(_PERM_TOOL_ID_PREFIX) without guarding against
tool_call_id being None, which can raise AttributeError for ToolCallEndEvent
instances missing tool_call_id; update the if condition in the block handling
ToolCallEndEvent to first verify message.tool_call_id is not None (or truthy)
before calling startswith, then proceed to call flush_pending_msg(), set
self._halted, self._halted_tool_call_id and halt_event_stream as currently
implemented when the guarded check passes.
🪄 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: 019bb7d7-c398-4f4e-a5ff-c25149531d0b
📒 Files selected for processing (5)
components/frontend/src/components/session/ask-user-question.tsxcomponents/frontend/src/components/session/permission-request.tsxcomponents/frontend/src/hooks/use-agent-status.tscomponents/runners/ambient-runner/ag_ui_claude_sdk/adapter.pycomponents/runners/ambient-runner/ambient_runner/bridges/claude/bridge.py
🚧 Files skipped from review as they are similar to previous changes (1)
- components/frontend/src/components/session/permission-request.tsx
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
github-actions
bot
added
ambient-code:needs-human
Contributor
|
AI was unable to resolve issues after 3 attempts. Needs human attention. |
Contributor
Author
|
@ambient-code please fix Performance And Algorithmic Complexity | ❌ Error | The _approved_operations set persists across runs without eviction, accumulating indefinitely as the adapter is intentionally cached and reused. | Implement LRU cache with fixed size (e.g., max 1000 entries), add TTL-based auto-eviction, or clear at session boundaries; document approval persistence scope. |
✅ Deploy Preview for cheerful-kitten-f556a0 canceled.
|
Contributor
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 `@components/runners/ambient-runner/ambient_runner/bridges/claude/bridge.py`:
- Around line 267-270: The adapter’s global permission-worker is being set and
cleared via self._adapter.set_permission_worker(worker) while only locking per
thread_id, causing cross-session races; change the binding to be scoped by
thread_id (or task-local context) so the adapter stores & retrieves permission
workers keyed by thread_id instead of a single global slot, use
self._session_manager.get_lock(thread_id) to guard only that key, set the keyed
binding at the start of the run (where set_permission_worker(worker) is called)
and clear only that key in the finally block (matching the existing clear at
lines around 324-325), and update adapter methods (e.g.,
set_permission_worker/get_permission_worker or similar) to accept a thread_id
param so injection and clearing affect only the current session.
🪄 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: 281652d6-2ca8-43c4-9bb9-ac0fa71435c7
📒 Files selected for processing (2)
components/frontend/src/types/agentic-session.tscomponents/runners/ambient-runner/ambient_runner/bridges/claude/bridge.py
🚧 Files skipped from review as they are similar to previous changes (1)
- components/frontend/src/types/agentic-session.ts
Comment on lines
267
to
+270
| async with self._session_manager.get_lock(thread_id): | ||
| # Expose the worker to the adapter so the can_use_tool callback | ||
| # can inject synthetic events into the active output queue. | ||
| self._adapter.set_permission_worker(worker) |
Contributor
There was a problem hiding this comment.
Shared permission-worker state is unsafe across concurrent sessions (critical).
Line 267 serializes only per thread_id, but Lines 270 and 325 mutate a single adapter-global permission worker reference. With concurrent runs on different thread IDs, one run can overwrite or clear another run’s worker, so permission events may be injected into the wrong session queue or dropped mid-run.
Scope permission-worker binding per run/session (e.g., keyed by thread_id or task-local context), and clear only that scoped binding in finally.
As per coding guidelines, "Flag only errors, security risks, or functionality-breaking problems."
Also applies to: 324-325
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/runners/ambient-runner/ambient_runner/bridges/claude/bridge.py`
around lines 267 - 270, The adapter’s global permission-worker is being set and
cleared via self._adapter.set_permission_worker(worker) while only locking per
thread_id, causing cross-session races; change the binding to be scoped by
thread_id (or task-local context) so the adapter stores & retrieves permission
workers keyed by thread_id instead of a single global slot, use
self._session_manager.get_lock(thread_id) to guard only that key, set the keyed
binding at the start of the run (where set_permission_worker(worker) is called)
and clear only that key in the finally block (matching the existing clear at
lines around 324-325), and update adapter methods (e.g.,
set_permission_worker/get_permission_worker or similar) to accept a thread_id
param so injection and clearing affect only the current session.
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
1 participant
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
can_use_toolcallback in the Claude SDK adapter to intercept sensitive tool operations (e.g., editing.mcp.json)PermissionRequesttool calls through the AG-UI event stream, reusing the existing halt-interrupt-resume pattern fromAskUserQuestionPermissionRequestMessagefrontend component with Allow/Deny buttons that surfaces permission requests in the session chat UITest plan
npm run build— 0 errors, 0 warnings)tsc --noEmit)npx vitest run)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Refactor