Studio: Add inline confirmation (Allow/Always allow/Deny) for tool calls#5869
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
for more information, see https://pre-commit.ci
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a "Confirm tool calls" feature, allowing users to manually approve or deny tool executions during agentic loops. It adds a backend confirmation gate using threading events, a new /tool-confirm API endpoint, and frontend UI controls for user interaction. Feedback on the changes highlights a critical concurrency race condition and potential resource leak in the backend's tool approval state management (request_tool_decision), where concurrent requests for the same session could overwrite pending states and cause threads to hang.
The Allow / Always allow / Deny controls only lived in the fallback tool
card, but the built-in tools (web search, python, terminal, code
execution, image generation) render with their own components and so
never showed the buttons. Those calls paused after tool_start with no way
to approve them, hanging until the 1 hour timeout. Only MCP tools, which
use the fallback renderer, actually worked.
Render the controls for every tool card by wrapping each registered tool
component (and the fallback) in thread.tsx with a shared
ToolConfirmationControls, so the gate applies uniformly.
Also make the handshake robust:
- The gate keys on a per-call approval_id minted by the backend and
echoed in tool_start, instead of session_id alone, so a stale or
concurrent confirmation can no longer resolve the wrong call.
- The approval slot is registered before tool_start is yielded, closing
the race where a fast click or an auto "Always allow" could reach the
backend before the waiter existed.
- The frontend resolves with the same session id the request was sent
with (plus the approval_id), fixing the new-thread mismatch where the
confirmation targeted a different session than the blocked stream.
- The confirm endpoint returns {resolved}; the UI keeps the buttons and
shows a retry hint until the backend confirms a match, instead of
hiding them on a failed or mistargeted post.
- The gate runs after the disabled-tool and duplicate-call checks, so a
call that will not execute is not put up for approval. A denied call is
still excluded from duplicate detection, so re-issuing and approving it
works.
- "Always allow" is scoped per session to match the backend gate.
Add backend tests for the approval registry, the SSE no-deadlock
handshake, and the loop integration (allow, deny, disabled, duplicate,
re-issue after deny).
for more information, see https://pre-commit.ci
|
Thanks for this, the opt-in confirmation gate is a great addition. I reviewed it end to end and pushed a commit on top ( Main issue: built-in tools never showed the controlsThe Allow / Always allow / Deny buttons only lived in Those custom components only import the Fix: a shared Handshake hardeningWhile fixing the above I also addressed a few correctness gaps in the gate:
What I did not change (worth a follow-up)External-provider hosted tools and the Anthropic Validation
One note for whoever merges: an automated check flagged a large "revert" against |
|
I have reviewed the additional changes and can confirm they harden the implementation and look correct to me. I also tested the updated code manually and it worked fine. Good to merge from my side. |
|
Since the confirmation buttons apply to all tools, I propose moving them out of the MCP section and into the Tools section. My latest commit in the PR does that: 
|
there seems to be a small issue where if I disable confirm tool calls while a chat session is running, it doesn't disable it completely, but it doesn't show the pop-up for allowance as well. I had to manually expand the end tool calls dropdown where it showed me the tool call and the pop-up. Screen.Recording.2026-06-01.at.3.26.39.PM.mov |
|
This should be resolved now after the latest commit @Datta0, it was a bug specific to turns with multiple tool calls, which render in a collapsed group by default. Test: 
About the mid-streaming toggle change: the toggle value is passed once on send, so changing it during streaming has no effect, similar to changing a slider like temperature mid-generation. I'd keep this as-is rather than add a warning to the tooltip for an edge case. |
yeah its better now . Ignore the "switching while session is going on" thing. its just how I did it, need not be the only way to rep
But I think we can benefit from a warning popup saying that disabling mid session ain't gonna do much |
# Conflicts: # studio/backend/core/inference/orchestrator.py # studio/backend/core/inference/safetensors_agentic.py # studio/backend/routes/inference.py # studio/frontend/src/components/assistant-ui/tool-group.tsx
for more information, see https://pre-commit.ci
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 67fb3b9e21
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c53302ebf3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08810f4f77
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
resolve_tool_decision accepted a second confirmation for the same approval_id and overwrote slot["decision"] in the window before the waiter reads it and pops the slot, so a duplicate or out-of-order POST could flip an Allow to Deny (and returned a misleading resolved:true). Reject once the slot's event is already set so the first decision wins. Adds a regression test.
|
One small follow-up on the confirmation gate: the recorded decision was not immutable. The other review points I checked are already handled on the current branch (the confirm_tool_calls rejection on the provider-routed / non-streaming / Anthropic paths, the pending-approval cleanup when the stream is closed at the prompt, and the settings copy), so this is the only change. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba6a670e92
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| setToolConfirmation: (toolCallId, approvalId, sessionId, autoAllowKey) => | ||
| set((state) => ({ | ||
| toolConfirmations: { | ||
| ...state.toolConfirmations, | ||
| [toolCallId]: { approvalId, sessionId, autoAllowKey }, | ||
| }, |
There was a problem hiding this comment.
When two local streams are active at once, this global map can overwrite one pending confirmation with another that uses the same toolCallId; the safetensors loop starts every run at call_0, and GGUF/model-emitted ids are not scoped either. In compare panes or a background chat plus the active chat, two gated call_0 tool starts will leave only the later approvalId here, so clicking Allow/Deny on the first card can resolve the wrong backend approval or fail while the first generation remains blocked. Key this by the same per-run/thread scope you already pass as autoAllowKey (or by approvalId) rather than by toolCallId alone.
Useful? React with 👍 / 👎.
| rag_scope: Optional[dict] = None, | ||
| seed: Optional[int] = None, | ||
| disable_parallel_tool_use: bool = False, | ||
| confirm_tool_calls: bool = False, |
There was a problem hiding this comment.
When confirm_tool_calls is true and Docs/RAG autoinject is enabled (the default auto path for small or unknown-size models), the loop still calls build_rag_autoinject before any of the new confirmation logic below; that helper performs search_for_autoinject immediately and only then emits a search_knowledge_base tool_start/tool_end without an approval_id. In that RAG-autoinject scenario, a local Studio tool runs despite the user enabling confirmation, so the autoinject path needs to be skipped, rejected, or routed through the same approval gate before retrieval.
Useful? React with 👍 / 👎.
|
Pushed Summary:
Validation:
|
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a4cb057670
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const id = | ||
| (toolEvent.tool_call_id as string) || | ||
| `${toolEvent.tool_name}_${Date.now()}`; | ||
| awaitingConfirmation && approvalId | ||
| ? `${toolConfirmationScopeId}:${approvalId}` | ||
| : backendToolCallId || |
There was a problem hiding this comment.
When render_html is detected early, the backend can already emit a provisional tool_start with the backend tool_call_id and no approval fields; in that scenario this branch assigns the real confirmed start a new ${scope}:${approvalId} id instead of the existing backend id. The result is two UI cards for one render_html call: the provisional card stays running forever while the second card gets the confirmation and completion. Either update the existing provisional card by backend id or suppress provisional starts while confirmations are enabled.
Useful? React with 👍 / 👎.
Bring the Bypass Permissions feature up to date with main (262 commits). main landed the tool-call-confirmation base (#5869) and evolved it, so the conflicts were resolved by taking main's version of every touched file and re-applying the bypass deltas on top: - tools.py: bypass env scrubber (_build_bypass_env, secret/cred-location classifiers), _bypass_preexec, /proc fail-closed hardening, and the disable_sandbox path in execute_tool/_python_exec/_bash_exec. - safetensors_agentic.py / llama_cpp.py: thread bypass_permissions and gate needs_confirm with 'and not bypass_permissions' (main simplified the gate to bool(confirm_tool_calls)). - routes/inference.py, models/inference.py, orchestrator.py: precedence (confirm and not bypass) plus the request fields and loop wiring. - frontend: bypassPermissions store state, chat-adapter payload, the settings toggle + warning dialog, and the red composer badge in both composers. - tests: fakes accept rag_scope (main added it); GGUF AST test matches main's needs_confirm name.
4 participants
When tools are enabled the model runs them automatically. This PR adds an opt-in confirmation step so the user approves each call first.
A "Confirm tool calls" toggle (in the MCP settings section, below "Use MCP Servers") gates every tool call: web search, python, terminal, and MCP tools. When on, the tool card shows inline Allow / Always allow / Deny buttons. Allow runs the call once. Always allow auto-approves that tool for the rest of the session. Deny feeds a rejection back to the model so it keeps responding.
How to use
Implementation
The tool loop already yields a
tool_startevent before executing each call, so the gate slots in there. It blocks on a session-keyedthreading.Eventuntil the decision arrives over a separatePOST /api/inference/tool-confirm, then resumes the same SSE stream. No bidirectional streaming or architecture change. The loop is sequential, so the gate keys on session id alone.New
confirm_tool_callsrequest field and one endpoint on the backend. The toggle and buttons reuse the existing toggle pattern and tool card on the frontend. Both the GGUF and safetensors loops get the gate.