feat: better UX during refusal (#5260)
<img width="568" height="169" alt="Screenshot 2025-10-16 at 18 28 05" src="https://github.com/user-attachments/assets/f42e8d6d-b7de-4948-b291-a5fbb50b1312" />
This commit is contained in:
@@ -916,6 +916,7 @@ impl Session {
|
|||||||
duration,
|
duration,
|
||||||
exit_code,
|
exit_code,
|
||||||
timed_out: _,
|
timed_out: _,
|
||||||
|
..
|
||||||
} = output;
|
} = output;
|
||||||
// Send full stdout/stderr to clients; do not truncate.
|
// Send full stdout/stderr to clients; do not truncate.
|
||||||
let stdout = stdout.text.clone();
|
let stdout = stdout.text.clone();
|
||||||
@@ -980,15 +981,28 @@ impl Session {
|
|||||||
let sub_id = context.sub_id.clone();
|
let sub_id = context.sub_id.clone();
|
||||||
let call_id = context.call_id.clone();
|
let call_id = context.call_id.clone();
|
||||||
|
|
||||||
self.on_exec_command_begin(turn_diff_tracker.clone(), context.clone())
|
let begin_turn_diff = turn_diff_tracker.clone();
|
||||||
.await;
|
let begin_context = context.clone();
|
||||||
|
let session = self;
|
||||||
let result = self
|
let result = self
|
||||||
.services
|
.services
|
||||||
.executor
|
.executor
|
||||||
.run(request, self, approval_policy, &context)
|
.run(request, self, approval_policy, &context, move || {
|
||||||
|
let turn_diff = begin_turn_diff.clone();
|
||||||
|
let ctx = begin_context.clone();
|
||||||
|
async move {
|
||||||
|
session.on_exec_command_begin(turn_diff, ctx).await;
|
||||||
|
}
|
||||||
|
})
|
||||||
.await;
|
.await;
|
||||||
|
|
||||||
|
if matches!(
|
||||||
|
&result,
|
||||||
|
Err(ExecError::Function(FunctionCallError::Denied(_)))
|
||||||
|
) {
|
||||||
|
return result;
|
||||||
|
}
|
||||||
|
|
||||||
let normalized = normalize_exec_result(&result);
|
let normalized = normalize_exec_result(&result);
|
||||||
let borrowed = normalized.event_output();
|
let borrowed = normalized.event_output();
|
||||||
|
|
||||||
@@ -2262,7 +2276,8 @@ async fn try_run_turn(
|
|||||||
response: Some(response),
|
response: Some(response),
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
Err(FunctionCallError::RespondToModel(message)) => {
|
Err(FunctionCallError::RespondToModel(message))
|
||||||
|
| Err(FunctionCallError::Denied(message)) => {
|
||||||
let response = ResponseInputItem::FunctionCallOutput {
|
let response = ResponseInputItem::FunctionCallOutput {
|
||||||
call_id: String::new(),
|
call_id: String::new(),
|
||||||
output: FunctionCallOutputPayload {
|
output: FunctionCallOutputPayload {
|
||||||
|
|||||||
@@ -60,5 +60,9 @@ pub mod errors {
|
|||||||
pub(crate) fn rejection(msg: impl Into<String>) -> Self {
|
pub(crate) fn rejection(msg: impl Into<String>) -> Self {
|
||||||
FunctionCallError::RespondToModel(msg.into()).into()
|
FunctionCallError::RespondToModel(msg.into()).into()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
pub(crate) fn denied(msg: impl Into<String>) -> Self {
|
||||||
|
FunctionCallError::Denied(msg.into()).into()
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,3 +1,4 @@
|
|||||||
|
use std::future::Future;
|
||||||
use std::path::PathBuf;
|
use std::path::PathBuf;
|
||||||
use std::sync::Arc;
|
use std::sync::Arc;
|
||||||
use std::sync::RwLock;
|
use std::sync::RwLock;
|
||||||
@@ -74,13 +75,18 @@ impl Executor {
|
|||||||
/// Runs a prepared execution request end-to-end: prepares parameters, decides on
|
/// Runs a prepared execution request end-to-end: prepares parameters, decides on
|
||||||
/// sandbox placement (prompting the user when necessary), launches the command,
|
/// sandbox placement (prompting the user when necessary), launches the command,
|
||||||
/// and lets the backend post-process the final output.
|
/// and lets the backend post-process the final output.
|
||||||
pub(crate) async fn run(
|
pub(crate) async fn run<F, Fut>(
|
||||||
&self,
|
&self,
|
||||||
mut request: ExecutionRequest,
|
mut request: ExecutionRequest,
|
||||||
session: &Session,
|
session: &Session,
|
||||||
approval_policy: AskForApproval,
|
approval_policy: AskForApproval,
|
||||||
context: &ExecCommandContext,
|
context: &ExecCommandContext,
|
||||||
) -> Result<ExecToolCallOutput, ExecError> {
|
on_exec_begin: F,
|
||||||
|
) -> Result<ExecToolCallOutput, ExecError>
|
||||||
|
where
|
||||||
|
F: FnOnce() -> Fut,
|
||||||
|
Fut: Future<Output = ()>,
|
||||||
|
{
|
||||||
if matches!(request.mode, ExecutionMode::Shell) {
|
if matches!(request.mode, ExecutionMode::Shell) {
|
||||||
request.params =
|
request.params =
|
||||||
maybe_translate_shell_command(request.params, session, request.use_shell_profile);
|
maybe_translate_shell_command(request.params, session, request.use_shell_profile);
|
||||||
@@ -119,7 +125,7 @@ impl Executor {
|
|||||||
if sandbox_decision.record_session_approval {
|
if sandbox_decision.record_session_approval {
|
||||||
self.approval_cache.insert(request.approval_command.clone());
|
self.approval_cache.insert(request.approval_command.clone());
|
||||||
}
|
}
|
||||||
|
on_exec_begin().await;
|
||||||
// Step 4: Launch the command within the chosen sandbox.
|
// Step 4: Launch the command within the chosen sandbox.
|
||||||
let first_attempt = self
|
let first_attempt = self
|
||||||
.spawn(
|
.spawn(
|
||||||
@@ -210,7 +216,7 @@ impl Executor {
|
|||||||
Ok(retry_output)
|
Ok(retry_output)
|
||||||
}
|
}
|
||||||
ReviewDecision::Denied | ReviewDecision::Abort => {
|
ReviewDecision::Denied | ReviewDecision::Abort => {
|
||||||
Err(ExecError::rejection("exec command rejected by user"))
|
Err(ExecError::denied("exec command rejected by user"))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -301,7 +307,8 @@ pub(crate) fn normalize_exec_result(
|
|||||||
}
|
}
|
||||||
Err(err) => {
|
Err(err) => {
|
||||||
let message = match err {
|
let message = match err {
|
||||||
ExecError::Function(FunctionCallError::RespondToModel(msg)) => msg.clone(),
|
ExecError::Function(FunctionCallError::RespondToModel(msg))
|
||||||
|
| ExecError::Function(FunctionCallError::Denied(msg)) => msg.clone(),
|
||||||
ExecError::Codex(e) => get_error_message_ui(e),
|
ExecError::Codex(e) => get_error_message_ui(e),
|
||||||
err => err.to_string(),
|
err => err.to_string(),
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -149,7 +149,7 @@ async fn select_shell_sandbox(
|
|||||||
ReviewDecision::Approved => Ok(SandboxDecision::user_override(false)),
|
ReviewDecision::Approved => Ok(SandboxDecision::user_override(false)),
|
||||||
ReviewDecision::ApprovedForSession => Ok(SandboxDecision::user_override(true)),
|
ReviewDecision::ApprovedForSession => Ok(SandboxDecision::user_override(true)),
|
||||||
ReviewDecision::Denied | ReviewDecision::Abort => {
|
ReviewDecision::Denied | ReviewDecision::Abort => {
|
||||||
Err(ExecError::rejection("exec command rejected by user"))
|
Err(ExecError::denied("exec command rejected by user"))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -4,6 +4,8 @@ use thiserror::Error;
|
|||||||
pub enum FunctionCallError {
|
pub enum FunctionCallError {
|
||||||
#[error("{0}")]
|
#[error("{0}")]
|
||||||
RespondToModel(String),
|
RespondToModel(String),
|
||||||
|
#[error("{0}")]
|
||||||
|
Denied(String),
|
||||||
#[error("LocalShellCall without call_id or id")]
|
#[error("LocalShellCall without call_id or id")]
|
||||||
MissingLocalShellCallId,
|
MissingLocalShellCallId,
|
||||||
#[error("Fatal error: {0}")]
|
#[error("Fatal error: {0}")]
|
||||||
|
|||||||
@@ -238,6 +238,7 @@ fn truncate_function_error(err: FunctionCallError) -> FunctionCallError {
|
|||||||
FunctionCallError::RespondToModel(msg) => {
|
FunctionCallError::RespondToModel(msg) => {
|
||||||
FunctionCallError::RespondToModel(format_exec_output(&msg))
|
FunctionCallError::RespondToModel(format_exec_output(&msg))
|
||||||
}
|
}
|
||||||
|
FunctionCallError::Denied(msg) => FunctionCallError::Denied(format_exec_output(&msg)),
|
||||||
FunctionCallError::Fatal(msg) => FunctionCallError::Fatal(format_exec_output(&msg)),
|
FunctionCallError::Fatal(msg) => FunctionCallError::Fatal(format_exec_output(&msg)),
|
||||||
other => other,
|
other => other,
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user