Fix regression 1

This commit is contained in:
jimmyfraiture
2025-10-01 12:37:08 +01:00
parent 0718b00b80
commit e23cbcaaf6
13 changed files with 38 additions and 32 deletions

View File

@@ -51,7 +51,9 @@ use crate::exec::StreamOutput;
use crate::exec_command::ExecCommandParams;
use crate::exec_command::ExecSessionManager;
use crate::exec_command::WriteStdinParams;
use crate::executor::{normalize_exec_result, Executor, ExecutorConfig};
use crate::executor::Executor;
use crate::executor::ExecutorConfig;
use crate::executor::normalize_exec_result;
use crate::mcp_connection_manager::McpConnectionManager;
use crate::model_family::find_family_for_model;
use crate::openai_model_info::get_model_info;
@@ -2353,10 +2355,10 @@ pub(crate) async fn exit_review_mode(
.await;
}
use crate::tools::context::ApplyPatchCommandContext;
use crate::tools::context::ExecCommandContext;
use crate::executor::errors::ExecError;
use crate::executor::linkers::PreparedExec;
use crate::tools::context::ApplyPatchCommandContext;
use crate::tools::context::ExecCommandContext;
#[cfg(test)]
pub(crate) use tests::make_session_and_context;
@@ -2372,12 +2374,12 @@ mod tests {
use crate::state::TaskKind;
use crate::tasks::SessionTask;
use crate::tasks::SessionTaskContext;
use codex_app_server_protocol::AuthMode;
use crate::tools::MODEL_FORMAT_HEAD_LINES;
use crate::tools::MODEL_FORMAT_MAX_BYTES;
use crate::tools::MODEL_FORMAT_MAX_LINES;
use crate::tools::MODEL_FORMAT_TAIL_LINES;
use crate::tools::handle_container_exec_with_params;
use codex_app_server_protocol::AuthMode;
use codex_protocol::models::ContentItem;
use codex_protocol::models::ResponseItem;

View File

@@ -6,7 +6,7 @@ use std::time::Duration;
use super::backends::ExecutionMode;
use super::backends::backend_for_mode;
use super::cache::ApprovalCache;
use crate::codex::{Session};
use crate::codex::Session;
use crate::error::CodexErr;
use crate::error::SandboxErr;
use crate::error::get_error_message_ui;
@@ -23,8 +23,8 @@ use crate::protocol::AskForApproval;
use crate::protocol::ReviewDecision;
use crate::protocol::SandboxPolicy;
use crate::shell;
use codex_otel::otel_event_manager::ToolDecisionSource;
use crate::tools::context::ExecCommandContext;
use codex_otel::otel_event_manager::ToolDecisionSource;
#[derive(Clone, Debug)]
pub(crate) struct ExecutorConfig {

View File

@@ -59,7 +59,7 @@ impl ToolPayload {
pub enum ToolOutput {
Function {
content: String,
success: bool,
success: Option<bool>,
},
Mcp {
result: Result<CallToolResult, String>,
@@ -74,6 +74,13 @@ impl ToolOutput {
}
}
pub fn success_for_logging(&self) -> bool {
match self {
ToolOutput::Function { success, .. } => success.unwrap_or(true),
ToolOutput::Mcp { result } => result.is_ok(),
}
}
pub fn into_response(self, call_id: &str, payload: &ToolPayload) -> ResponseInputItem {
match self {
ToolOutput::Function { content, success } => {
@@ -85,10 +92,7 @@ impl ToolOutput {
} else {
ResponseInputItem::FunctionCallOutput {
call_id: call_id.to_string(),
output: FunctionCallOutputPayload {
content,
success: Some(success),
},
output: FunctionCallOutputPayload { content, success },
}
}
}
@@ -112,7 +116,7 @@ mod tests {
};
let response = ToolOutput::Function {
content: "patched".to_string(),
success: true,
success: Some(true),
}
.into_response("call-42", &payload);
@@ -132,7 +136,7 @@ mod tests {
};
let response = ToolOutput::Function {
content: "ok".to_string(),
success: true,
success: Some(true),
}
.into_response("fn-1", &payload);

View File

@@ -79,7 +79,7 @@ impl ToolHandler for ApplyPatchHandler {
Ok(ToolOutput::Function {
content,
success: true,
success: Some(true),
})
}
}

View File

@@ -65,7 +65,7 @@ impl ToolHandler for ExecStreamHandler {
Ok(ToolOutput::Function {
content,
success: true,
success: Some(true),
})
}
}

View File

@@ -60,11 +60,8 @@ impl ToolHandler for McpHandler {
Ok(ToolOutput::Mcp { result })
}
codex_protocol::models::ResponseInputItem::FunctionCallOutput { output, .. } => {
let success = output.success.unwrap_or(false);
Ok(ToolOutput::Function {
content: output.content,
success,
})
let codex_protocol::models::FunctionCallOutputPayload { content, success } = output;
Ok(ToolOutput::Function { content, success })
}
_ => Err(FunctionCallError::RespondToModel(
"mcp handler received unexpected response variant".to_string(),

View File

@@ -41,7 +41,7 @@ impl ToolHandler for PlanHandler {
Ok(ToolOutput::Function {
content,
success: true,
success: Some(true),
})
}
}

View File

@@ -91,7 +91,7 @@ impl ToolHandler for ReadFileHandler {
let collected = read_file_slice(&path, offset, limit).await?;
Ok(ToolOutput::Function {
content: collected.join("\n"),
success: true,
success: Some(true),
})
}
}

View File

@@ -75,7 +75,7 @@ impl ToolHandler for ShellHandler {
.await?;
Ok(ToolOutput::Function {
content,
success: true,
success: Some(true),
})
}
ToolPayload::LocalShell { params } => {
@@ -92,7 +92,7 @@ impl ToolHandler for ShellHandler {
.await?;
Ok(ToolOutput::Function {
content,
success: true,
success: Some(true),
})
}
_ => Err(FunctionCallError::RespondToModel(format!(

View File

@@ -106,7 +106,7 @@ impl ToolHandler for UnifiedExecHandler {
Ok(ToolOutput::Function {
content,
success: true,
success: Some(true),
})
}
}

View File

@@ -58,7 +58,7 @@ impl ToolHandler for ViewImageHandler {
Ok(ToolOutput::Function {
content: "attached local image path".to_string(),
success: true,
success: Some(true),
})
}
}

View File

@@ -108,11 +108,12 @@ impl ToolRegistry {
match handler.handle(invocation).await {
Ok(output) => {
let preview = output.log_preview();
let success = output.success_for_logging();
let mut guard = output_cell
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner);
*guard = Some(output);
Ok(preview)
Ok((preview, success))
}
Err(err) => Err(err),
}

View File

@@ -366,10 +366,10 @@ impl OtelEventManager {
call_id: &str,
arguments: &str,
f: F,
) -> Result<String, E>
) -> Result<(String, bool), E>
where
F: FnOnce() -> Fut,
Fut: Future<Output = Result<String, E>>,
Fut: Future<Output = Result<(String, bool), E>>,
E: Display,
{
let start = Instant::now();
@@ -377,10 +377,12 @@ impl OtelEventManager {
let duration = start.elapsed();
let (output, success) = match &result {
Ok(content) => (content, true),
Err(error) => (&error.to_string(), false),
Ok((preview, success)) => (preview.clone(), *success),
Err(error) => (error.to_string(), false),
};
let success_str = if success { "true" } else { "false" };
tracing::event!(
tracing::Level::INFO,
event.name = "codex.tool_result",
@@ -396,7 +398,7 @@ impl OtelEventManager {
call_id = %call_id,
arguments = %arguments,
duration_ms = %duration.as_millis(),
success = %success,
success = %success_str,
output = %output,
);