-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Add option to approve and remember MCP/Apps tool usage #10584
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
codex-rs/core/src/mcp_tool_call.rs
Outdated
| } | ||
|
|
||
| const MCP_TOOL_APPROVAL_QUESTION_ID_PREFIX: &str = "mcp_tool_call_approval"; | ||
| const MCP_TOOL_APPROVAL_ACCEPT: &str = "Accept"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make them match the phrases we use for command approval so we can unify them later? i.e.
"Accept" -> "Approve Once"
"Allow and remember" -> "Approve this Session"
"Decline" -> "Deny"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mzeng-openai For commands, here are the phrases we have now:
1. Yes, proceed (y)
2. Yes, and don't ask again for commands that start with `echo hello world` (p)
3. No, and tell Codex what to do differently (esc)
How would you feel about something like:
1. Yes, proceed once (y)
2. Yes, and don't ask again for this tool and this session (p)
3. No (esc)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ive updated the phrasing, we can adjust or unify next.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh you are right, the phrases I was mentioning are from the webview. That complicates things a little bit because the phrases we use for elicitation here are reused both in the CLI and the webview. So I guess either way works (my slight preference is to use the webview version since it's cleaner but your call)
|
This is great! Just one nit on the copy, otherwise good to go! Have we tested locally? |
mzeng-openai
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approve to unblock
Yes, I tested with the slack connector. |
codex-rs/core/src/mcp_tool_call.rs
Outdated
| const MCP_TOOL_APPROVAL_QUESTION_ID_PREFIX: &str = "mcp_tool_call_approval"; | ||
| const MCP_TOOL_APPROVAL_ACCEPT: &str = "Accept"; | ||
| const MCP_TOOL_APPROVAL_DECLINE: &str = "Decline"; | ||
| const MCP_TOOL_APPROVAL_ACCEPT: &str = "Yes, proceed once (y)"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: The hotkey here will not work on webview, you have to either
- Drop the hotkey
- Check the originator() and optionally add it when the originator is codex_cli_rs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for catching that! For now I'm going to remove the hotkeys, and implement the second recommendation in a fast follow. Also adjusting the labels as described above.
|
/merge |
This PR adds a new approval option for app/MCP tool calls: “Allow and remember” (session-scoped).
When selected, Codex stores a temporary approval and auto-approves matching future calls for the rest of the session.
Added a session-scoped approval key (
server,connector_id,tool_name) and persisted it intool_approvalsasApprovedForSession.On subsequent matching calls, approval is skipped and treated as accepted.
The new “Allow and remember” option is only shown when all of these are true:
If no
connector_idis present, the prompt still appears (when approval is required), but only with the existing choices (Accept / Decline / Cancel). Approval prompting in this path has an explicit early return unless server ==codex_apps.