mirror of
https://github.com/openai/codex.git
synced 2026-11-07 20:30:33 +00:00
Use captured step settings for tool planning and execution (#44242)
## Why After a model change within a turn, tools could still use the turn's original model capabilities, output limits, and telemetry attribution. ## What changed - Build each step's tool plan from its captured model settings. - Use those settings for image support and detail, command and MCP output truncation, extension metadata, and tool telemetry. - Preserve the issuing step's model for delayed command completion events while allowing later `write_stdin` calls to use their own step's output limits. ## Testing Add regression coverage for model changes within a turn, including tool replanning, Code Mode execution, image handling, MCP tool and resource output limits, extension settings, and delayed command completion. GitOrigin-RevId: ef991598e89a846889baf770a0c9e12e56886d5d
This commit is contained in:
@@ -24,6 +24,7 @@ use crate::unified_exec::head_tail_buffer::HeadTailBuffer;
|
||||
use codex_core_plugins::PluginCommandAttribution;
|
||||
use codex_protocol::exec_output::ExecToolCallOutput;
|
||||
use codex_protocol::exec_output::StreamOutput;
|
||||
use codex_protocol::openai_models::ModelInfo;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::ExecCommandOutputDeltaEvent;
|
||||
use codex_protocol::protocol::ExecCommandSource;
|
||||
@@ -164,9 +165,7 @@ pub(crate) fn start_streaming_output(
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
pub(crate) fn spawn_exit_watcher(
|
||||
process: Arc<UnifiedExecProcess>,
|
||||
session_ref: Arc<Session>,
|
||||
turn_ref: Arc<TurnContext>,
|
||||
call_id: String,
|
||||
context: &UnifiedExecContext,
|
||||
command: Vec<String>,
|
||||
cwd: PathUri,
|
||||
process_id: i32,
|
||||
@@ -176,6 +175,10 @@ pub(crate) fn spawn_exit_watcher(
|
||||
network_denial_monitor: Option<tokio::task::JoinHandle<()>>,
|
||||
plugin_metrics_sidecar: Option<SharedPluginMetricsSidecar>,
|
||||
) {
|
||||
let session_ref = Arc::clone(&context.session);
|
||||
let turn_ref = Arc::clone(&context.step_context.turn);
|
||||
let model_info = Arc::clone(&context.step_context.settings.model_info);
|
||||
let call_id = context.call_id.clone();
|
||||
let exit_token = process.cancellation_token();
|
||||
let output_drained = process.output_drained_notify();
|
||||
let interaction_lock = process.interaction_lock();
|
||||
@@ -200,6 +203,7 @@ pub(crate) fn spawn_exit_watcher(
|
||||
emit_failed_exec_end_for_unified_exec(
|
||||
session_ref,
|
||||
turn_ref,
|
||||
model_info,
|
||||
call_id,
|
||||
command,
|
||||
cwd,
|
||||
@@ -225,6 +229,7 @@ pub(crate) fn spawn_exit_watcher(
|
||||
emit_exec_end_for_unified_exec(
|
||||
session_ref,
|
||||
turn_ref,
|
||||
model_info,
|
||||
call_id,
|
||||
command,
|
||||
cwd,
|
||||
@@ -337,6 +342,7 @@ impl Emitter {
|
||||
pub(crate) async fn emit_exec_end_for_unified_exec(
|
||||
session_ref: Arc<Session>,
|
||||
turn_ref: Arc<TurnContext>,
|
||||
model_info: Arc<ModelInfo>,
|
||||
call_id: String,
|
||||
command: Vec<String>,
|
||||
cwd: PathUri,
|
||||
@@ -360,6 +366,7 @@ pub(crate) async fn emit_exec_end_for_unified_exec(
|
||||
let event_ctx = ToolEventCtx::new(
|
||||
session_ref.as_ref(),
|
||||
turn_ref.as_ref(),
|
||||
&model_info,
|
||||
&call_id,
|
||||
/*turn_diff_tracker*/ None,
|
||||
);
|
||||
@@ -385,6 +392,7 @@ pub(crate) async fn emit_exec_end_for_unified_exec(
|
||||
pub(crate) async fn emit_failed_exec_end_for_unified_exec(
|
||||
session_ref: Arc<Session>,
|
||||
turn_ref: Arc<TurnContext>,
|
||||
model_info: Arc<ModelInfo>,
|
||||
call_id: String,
|
||||
command: Vec<String>,
|
||||
cwd: PathUri,
|
||||
@@ -416,6 +424,7 @@ pub(crate) async fn emit_failed_exec_end_for_unified_exec(
|
||||
let event_ctx = ToolEventCtx::new(
|
||||
session_ref.as_ref(),
|
||||
turn_ref.as_ref(),
|
||||
&model_info,
|
||||
&call_id,
|
||||
/*turn_diff_tracker*/ None,
|
||||
);
|
||||
|
||||
@@ -208,7 +208,7 @@ async fn exit_watcher_waits_for_late_network_denial_before_classifying_end() ->
|
||||
stdout_tx,
|
||||
exit_tx,
|
||||
transcript,
|
||||
context,
|
||||
mut context,
|
||||
rx_event,
|
||||
} = streaming_output_harness().await?;
|
||||
|
||||
@@ -226,11 +226,15 @@ async fn exit_watcher_waits_for_late_network_denial_before_classifying_end() ->
|
||||
|
||||
#[allow(deprecated)]
|
||||
let cwd = context.step_context.turn.cwd.clone().into();
|
||||
let step = Arc::get_mut(&mut context.step_context).expect("unshared test step");
|
||||
let model_info = Arc::make_mut(&mut Arc::make_mut(&mut step.settings).model_info);
|
||||
model_info.truncation_policy = codex_protocol::openai_models::TruncationPolicyConfig {
|
||||
mode: codex_protocol::openai_models::TruncationMode::Bytes,
|
||||
limit: 4,
|
||||
};
|
||||
spawn_exit_watcher(
|
||||
Arc::clone(&process),
|
||||
Arc::clone(&context.session),
|
||||
Arc::clone(&context.step_context.turn),
|
||||
context.call_id,
|
||||
&context,
|
||||
vec!["proof".to_string()],
|
||||
cwd,
|
||||
/*process_id*/ 123,
|
||||
@@ -266,6 +270,13 @@ async fn exit_watcher_waits_for_late_network_denial_before_classifying_end() ->
|
||||
Some("LATE_DENIAL")
|
||||
)
|
||||
);
|
||||
assert_eq!(
|
||||
item.formatted_output,
|
||||
Some(codex_utils_output_truncation::formatted_truncate_text(
|
||||
"LATE_DENIAL",
|
||||
codex_utils_output_truncation::TruncationPolicy::Bytes(4),
|
||||
))
|
||||
);
|
||||
assert!(
|
||||
elapsed >= Duration::from_millis(10) && elapsed < TRAILING_OUTPUT_GRACE,
|
||||
"completion should wait for denial without falling back to the output grace: {elapsed:?}"
|
||||
|
||||
@@ -406,6 +406,7 @@ async fn emit_failed_initial_exec_end_if_unstored(
|
||||
emit_failed_exec_end_for_unified_exec(
|
||||
Arc::clone(&context.session),
|
||||
Arc::clone(&context.step_context.turn),
|
||||
Arc::clone(&context.step_context.settings.model_info),
|
||||
context.call_id.clone(),
|
||||
request.command.clone(),
|
||||
cwd,
|
||||
@@ -532,6 +533,7 @@ impl UnifiedExecProcessManager {
|
||||
let event_ctx = ToolEventCtx::new(
|
||||
context.session.as_ref(),
|
||||
context.step_context.turn.as_ref(),
|
||||
&context.step_context.settings.model_info,
|
||||
&context.call_id,
|
||||
/*turn_diff_tracker*/ None,
|
||||
);
|
||||
@@ -766,6 +768,7 @@ impl UnifiedExecProcessManager {
|
||||
emit_exec_end_for_unified_exec(
|
||||
Arc::clone(&context.session),
|
||||
Arc::clone(&context.step_context.turn),
|
||||
Arc::clone(&context.step_context.settings.model_info),
|
||||
context.call_id.clone(),
|
||||
request.command.clone(),
|
||||
cwd.clone(),
|
||||
@@ -796,8 +799,8 @@ impl UnifiedExecProcessManager {
|
||||
raw_output: collected,
|
||||
truncation_policy: context
|
||||
.step_context
|
||||
.turn
|
||||
.model_info()
|
||||
.settings
|
||||
.model_info
|
||||
.truncation_policy
|
||||
.into(),
|
||||
max_output_tokens: request.max_output_tokens,
|
||||
@@ -1175,9 +1178,7 @@ impl UnifiedExecProcessManager {
|
||||
|
||||
spawn_exit_watcher(
|
||||
Arc::clone(&process),
|
||||
Arc::clone(&context.session),
|
||||
Arc::clone(&context.step_context.turn),
|
||||
context.call_id.clone(),
|
||||
context,
|
||||
command.to_vec(),
|
||||
cwd,
|
||||
process_id,
|
||||
|
||||
Reference in New Issue
Block a user