feedback

codex-rs/core/src/conversation_history.rs

@@ -79,50 +79,62 @@ impl ConversationHistory {
     }
 
     pub(crate) fn handle_missing_tool_call_output(&mut self, task_kind: TaskKind) {
-        // call_ids that are part of this response.
-        let content = match task_kind {
-            TaskKind::Regular => self.contents(),
-            TaskKind::Review => self.review_thread_contents(),
-            TaskKind::Compact => self.contents(),
+        // Select the appropriate thread's history vector.
+        let thread_vec: &mut Vec<ResponseItem> = match task_kind {
+            TaskKind::Regular | TaskKind::Compact => &mut self.items,
+            TaskKind::Review => &mut self.review_thread_history,
         };
-        let completed_call_ids = content
+
+        // Determine which calls are already completed.
+        let completed_call_ids = thread_vec
             .iter()
             .filter_map(|ri| match ri {
-                ResponseItem::FunctionCallOutput { call_id, .. } => Some(call_id),
-                ResponseItem::CustomToolCallOutput { call_id, .. } => Some(call_id),
+                ResponseItem::FunctionCallOutput { call_id, .. } => Some(call_id.clone()),
+                ResponseItem::CustomToolCallOutput { call_id, .. } => Some(call_id.clone()),
                 _ => None,
             })
             .collect::<Vec<_>>();
 
-        // call_ids that were pending but are not part of this response.
-        // This usually happens because the user interrupted the model before we responded to one of its tool calls
-        // and then the user sent a follow-up message.
-        let missing_calls = {
-            content
-                .iter()
-                .filter_map(|ri| match ri {
-                    ResponseItem::FunctionCall { call_id, .. } => Some(call_id),
-                    ResponseItem::LocalShellCall {
-                        call_id: Some(call_id),
-                        ..
-                    } => Some(call_id),
-                    ResponseItem::CustomToolCall { call_id, .. } => Some(call_id),
-                    _ => None,
-                })
-                .filter_map(|call_id| {
-                    if completed_call_ids.contains(&call_id) {
-                        None
-                    } else {
-                        Some(call_id.clone())
-                    }
-                })
-                .map(|call_id| ResponseItem::CustomToolCallOutput {
-                    call_id,
-                    output: "aborted".to_string(),
-                })
-                .collect::<Vec<_>>()
-        };
-        self.record_items(missing_calls, task_kind);
+        // Find tool calls that are missing outputs and insert synthetic outputs
+        // immediately after the corresponding call so the call appears resolved
+        // before any subsequent user input is processed.
+        // We re-scan the vector for each missing call id so insertion indices stay valid.
+        let pending_call_ids = thread_vec
+            .iter()
+            .filter_map(|ri| match ri {
+                ResponseItem::FunctionCall { call_id, .. } => Some(call_id.clone()),
+                ResponseItem::LocalShellCall {
+                    call_id: Some(call_id),
+                    ..
+                } => Some(call_id.clone()),
+                ResponseItem::CustomToolCall { call_id, .. } => Some(call_id.clone()),
+                _ => None,
+            })
+            .collect::<Vec<_>>();
+
+        for call_id in pending_call_ids {
+            if completed_call_ids.iter().any(|c| c == &call_id) {
+                continue;
+            }
+
+            // Find the last index of the corresponding call in the current vector.
+            if let Some(idx) = thread_vec.iter().rposition(|ri| match ri {
+                ResponseItem::FunctionCall { call_id: id, .. } => id == &call_id,
+                ResponseItem::LocalShellCall {
+                    call_id: Some(id), ..
+                } => id == &call_id,
+                ResponseItem::CustomToolCall { call_id: id, .. } => id == &call_id,
+                _ => false,
+            }) {
+                thread_vec.insert(
+                    idx + 1,
+                    ResponseItem::CustomToolCallOutput {
+                        call_id: call_id.clone(),
+                        output: "aborted".to_string(),
+                    },
+                );
+            }
+        }
     }
 
     pub(crate) fn prompt(&self, task_kind: TaskKind) -> Vec<ResponseItem> {
@@ -153,6 +165,7 @@ fn is_api_message(message: &ResponseItem) -> bool {
 mod tests {
     use super::*;
     use codex_protocol::models::ContentItem;
+    use pretty_assertions::assert_eq;
 
     fn assistant_msg(text: &str) -> ResponseItem {
         ResponseItem::Message {
@@ -213,4 +226,47 @@ mod tests {
             ],
         );
     }
+
+    #[test]
+    fn inserts_synthetic_tool_output_next_to_call_before_user_input() {
+        let mut h = ConversationHistory::default();
+
+        // Record a pending tool call without an output.
+        let call_id = "call-1".to_string();
+        let tool_call = ResponseItem::FunctionCall {
+            id: None,
+            name: "shell".to_string(),
+            arguments: "{\"command\":[\"echo\",\"hi\"]}".to_string(),
+            call_id: call_id.clone(),
+        };
+        h.record_items([tool_call], TaskKind::Regular);
+
+        // Ensure missing outputs are handled first.
+        h.handle_missing_tool_call_output(TaskKind::Regular);
+
+        // Then record a new user message.
+        let user = user_msg("follow up");
+        h.add_pending_input(vec![user.clone()], TaskKind::Regular);
+
+        // Expect the synthetic output to be inserted immediately after the call
+        // and before the user message.
+        let items = h.contents();
+        assert_eq!(items.len(), 3);
+        match (&items[0], &items[1], &items[2]) {
+            (
+                ResponseItem::FunctionCall { call_id: id0, .. },
+                ResponseItem::CustomToolCallOutput {
+                    call_id: id1,
+                    output,
+                },
+                ResponseItem::Message { role, .. },
+            ) => {
+                assert_eq!(id0, &call_id);
+                assert_eq!(id1, &call_id);
+                assert_eq!(output, "aborted");
+                assert_eq!(role, "user");
+            }
+            _ => panic!("unexpected item ordering: {items:?}"),
+        }
+    }
 }

codex-rs/core/src/state/session.rs

@@ -60,10 +60,12 @@ impl SessionState {
         task_kind: TaskKind,
         pending_input: Vec<ResponseItem>,
     ) -> Vec<ResponseItem> {
+        // Ensure any missing tool-call outputs are recorded before
+        // accepting new pending input to preserve correct ordering.
+        self.history.handle_missing_tool_call_output(task_kind);
         if !pending_input.is_empty() {
             self.history.add_pending_input(pending_input, task_kind);
         }
-        self.history.handle_missing_tool_call_output(task_kind);
         self.history.prompt(task_kind)
     }