mirror of
https://github.com/openai/codex.git
synced 2026-04-24 14:45:27 +00:00
Stabilize abort task follow-up handling (#13874)
- production logic plus tests; cancel running tasks before clearing pending turn state - suppress follow-up model requests after cancellation and assert on stabilized request counts instead of fixed sleeps
This commit is contained in:
23
codex-rs/app-server/src/bin/test_notify_capture.rs
Normal file
23
codex-rs/app-server/src/bin/test_notify_capture.rs
Normal file
@@ -0,0 +1,23 @@
|
||||
use anyhow::Result;
|
||||
use anyhow::anyhow;
|
||||
use std::env;
|
||||
use std::path::PathBuf;
|
||||
|
||||
fn main() -> Result<()> {
|
||||
let mut args = env::args_os().skip(1);
|
||||
let output_path = PathBuf::from(
|
||||
args.next()
|
||||
.ok_or_else(|| anyhow!("missing output path argument"))?,
|
||||
);
|
||||
let payload = args
|
||||
.next()
|
||||
.ok_or_else(|| anyhow!("missing payload argument"))?
|
||||
.into_string()
|
||||
.map_err(|_| anyhow!("payload must be valid UTF-8"))?;
|
||||
|
||||
let temp_path = output_path.with_extension("json.tmp");
|
||||
std::fs::write(&temp_path, payload)?;
|
||||
std::fs::rename(&temp_path, &output_path)?;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
@@ -14,6 +14,7 @@ use codex_app_server_protocol::ThreadStartResponse;
|
||||
use codex_app_server_protocol::TurnStartParams;
|
||||
use codex_app_server_protocol::TurnStartResponse;
|
||||
use codex_app_server_protocol::UserInput as V2UserInput;
|
||||
use codex_utils_cargo_bin::cargo_bin;
|
||||
use core_test_support::fs_wait;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::Value;
|
||||
@@ -191,29 +192,22 @@ async fn turn_start_notify_payload_includes_initialize_client_name() -> Result<(
|
||||
let responses = vec![create_final_assistant_message_sse_response("Done")?];
|
||||
let server = create_mock_responses_server_sequence_unchecked(responses).await;
|
||||
let codex_home = TempDir::new()?;
|
||||
let notify_script = codex_home.path().join("notify.py");
|
||||
std::fs::write(
|
||||
¬ify_script,
|
||||
r#"from pathlib import Path
|
||||
import sys
|
||||
|
||||
payload_path = Path(__file__).with_name("notify.json")
|
||||
tmp_path = payload_path.with_suffix(".json.tmp")
|
||||
tmp_path.write_text(sys.argv[-1], encoding="utf-8")
|
||||
tmp_path.replace(payload_path)
|
||||
"#,
|
||||
)?;
|
||||
let notify_file = codex_home.path().join("notify.json");
|
||||
let notify_script = notify_script
|
||||
let notify_capture = cargo_bin("test_notify_capture")?;
|
||||
let notify_capture = notify_capture
|
||||
.to_str()
|
||||
.expect("notify script path should be valid UTF-8");
|
||||
.expect("notify capture path should be valid UTF-8");
|
||||
let notify_file = notify_file
|
||||
.to_str()
|
||||
.expect("notify output path should be valid UTF-8");
|
||||
create_config_toml_with_extra(
|
||||
codex_home.path(),
|
||||
&server.uri(),
|
||||
"never",
|
||||
&format!(
|
||||
"notify = [\"python3\", {}]",
|
||||
toml_basic_string(notify_script)
|
||||
"notify = [{}, {}]",
|
||||
toml_basic_string(notify_capture),
|
||||
toml_basic_string(notify_file)
|
||||
),
|
||||
)?;
|
||||
|
||||
@@ -261,8 +255,9 @@ tmp_path.replace(payload_path)
|
||||
)
|
||||
.await??;
|
||||
|
||||
fs_wait::wait_for_path_exists(¬ify_file, Duration::from_secs(5)).await?;
|
||||
let payload_raw = tokio::fs::read_to_string(¬ify_file).await?;
|
||||
let notify_file = Path::new(notify_file);
|
||||
fs_wait::wait_for_path_exists(notify_file, Duration::from_secs(5)).await?;
|
||||
let payload_raw = tokio::fs::read_to_string(notify_file).await?;
|
||||
let payload: Value = serde_json::from_str(&payload_raw)?;
|
||||
assert_eq!(payload["client"], "xcode");
|
||||
|
||||
|
||||
@@ -2702,8 +2702,9 @@ impl Session {
|
||||
/// Emit an exec approval request event and await the user's decision.
|
||||
///
|
||||
/// The request is keyed by `call_id` + `approval_id` so matching responses
|
||||
/// are delivered to the correct in-flight turn. If the task is aborted,
|
||||
/// this returns the default `ReviewDecision` (`Denied`).
|
||||
/// are delivered to the correct in-flight turn. If the pending approval is
|
||||
/// cleared before a response arrives, treat it as an abort so interrupted
|
||||
/// turns do not continue on a synthetic denial.
|
||||
///
|
||||
/// Note that if `available_decisions` is `None`, then the other fields will
|
||||
/// be used to derive the available decisions via
|
||||
@@ -2777,7 +2778,7 @@ impl Session {
|
||||
parsed_cmd,
|
||||
});
|
||||
self.send_event(turn_context, event).await;
|
||||
rx_approve.await.unwrap_or_default()
|
||||
rx_approve.await.unwrap_or(ReviewDecision::Abort)
|
||||
}
|
||||
|
||||
pub async fn request_patch_approval(
|
||||
@@ -6859,6 +6860,10 @@ async fn try_run_sampling_request(
|
||||
|
||||
drain_in_flight(&mut in_flight, sess.clone(), turn_context.clone()).await?;
|
||||
|
||||
if cancellation_token.is_cancelled() {
|
||||
return Err(CodexErr::TurnAborted);
|
||||
}
|
||||
|
||||
if should_emit_turn_diff {
|
||||
let unified_diff = {
|
||||
let mut tracker = turn_diff_tracker.lock().await;
|
||||
|
||||
Reference in New Issue
Block a user