fix(codex): preserve valid tool inputs and result pairing

docs/formats/codex.md

@@ -83,7 +83,15 @@ the form `["bash"|"sh"|"zsh", "-lc"|"-c", cmd]` collapse to the inner command.
   representation keeps every payload as raw JSON, so native ↔ disk round-trips are lossless.
 - **`apply_patch` is best-effort.** Only a lone single-hunk update maps to `Edit` and a lone file
   add to `Write`; multi-file, multi-hunk, delete, and move patches stay as a raw `ApplyPatch`
-  with the touched paths listed.
+  with the touched paths listed. On export, `Edit` and `Write` become `apply_patch`
+  custom-tool calls, and raw `ApplyPatch` envelopes are unwrapped. Their results use
+  `custom_tool_call_output`, preserving the error flag. Patches are line-based:
+  `replace_all` becomes one hunk, Codex can normalize trailing blank lines, and the Common
+  reader does not retain the final line terminator. Exported `Write` calls use an add-file
+  patch, which does not encode whether the original call created or overwrote a file.
+- **Shell export.** `Bash` becomes `exec_command` with `cmd` and optional `workdir`.
+  The canonical timeout, description, and background fields have no matching fields in
+  this mapping and are omitted. Other tools keep their canonical function-call form.
 - **Resume is picky.** `from_common` must emit `model_provider: "openai"` in `session_meta` —
   current Codex resolves a null provider to the empty name and fails resume with
   ``Model provider `` not found``. `base_instructions` may be null (defaults substitute). Foreign
@@ -98,7 +106,9 @@ the form `["bash"|"sh"|"zsh", "-lc"|"-c", cmd]` collapse to the inner command.
   malformed lines are skipped by the JSONL parser rather than aborting the file; a `session_meta`
   without an id disqualifies a file from discovery instead of producing a broken session.
 - **Duplicate results are by design.** Seeing both `exec_command_end` and a matching
-  `function_call_output` in a file is normal; only one becomes a `ToolResult`.
+  `function_call_output` in a file is normal; only one becomes a `ToolResult`. Matching
+  and duplicate suppression follow each function/custom call occurrence, so a completed
+  ID can be reused without changing the result type or dropping an earlier result.
 
 ## References
 

src/harness/codex.rs

@@ -59,7 +59,7 @@ struct Queued {
     timestamp: DateTime<Utc>,
     model: Option<String>,
     usage: Option<Usage>,
-    result_call_id: Option<String>,
+    result_key: Option<(String, usize)>,
     is_fallback_result: bool,
 }
 
@@ -101,7 +101,8 @@ fn lines_to_messages(lines: &[Line], fallback_ts: DateTime<Utc>) -> Vec<Message>
     let mut turn_models: HashMap<String, String> = HashMap::new();
     let mut turn_usage: HashMap<String, Usage> = HashMap::new();
     let mut last_assistant_text_by_turn: HashMap<String, usize> = HashMap::new();
-    let mut canonical_results: HashSet<String> = HashSet::new();
+    let mut canonical_results = HashSet::new();
+    let mut call_occurrences = HashMap::new();
     let mut pending_web_search_ids: HashMap<String, Vec<String>> = HashMap::new();
     let mut unresolved_web_search_indices: HashMap<String, Vec<usize>> = HashMap::new();
 
@@ -116,6 +117,23 @@ fn lines_to_messages(lines: &[Line], fallback_ts: DateTime<Utc>) -> Vec<Message>
             .and_then(parse_ts)
             .unwrap_or(fallback_ts);
         let payload = &line.payload;
+        // Function/custom IDs may be reused after a call completes. Scope
+        // mirror suppression to the latest occurrence, not the entire log.
+        if line.kind == "response_item"
+            && matches!(
+                payload.get("type").and_then(Value::as_str),
+                Some("function_call" | "custom_tool_call")
+            )
+            && let Some(id) = payload.get("call_id").and_then(Value::as_str)
+        {
+            call_occurrences.insert(id, queued.len() + 1);
+        }
+        let result_key = |id: &str| {
+            (
+                id.to_string(),
+                call_occurrences.get(id).copied().unwrap_or(0),
+            )
+        };
         match line.kind.as_str() {
             "turn_context" => {
                 current_turn_id = payload
@@ -165,10 +183,10 @@ fn lines_to_messages(lines: &[Line], fallback_ts: DateTime<Utc>) -> Vec<Message>
                             .and_then(Value::as_str)
                             .map(String::from)
                         {
-                            canonical_results.insert(call_id.clone());
+                            canonical_results.insert(result_key(&call_id));
                             queued.push(tool_result(
                                 ts,
-                                call_id,
+                                result_key(&call_id),
                                 ToolOutput::Text(format_exec_output(payload)),
                                 payload
                                     .get("exit_code")
@@ -202,10 +220,10 @@ fn lines_to_messages(lines: &[Line], fallback_ts: DateTime<Utc>) -> Vec<Message>
                             {
                                 id.clone_from(&call_id);
                             }
-                            canonical_results.insert(call_id.clone());
+                            canonical_results.insert(result_key(&call_id));
                             queued.push(tool_result(
                                 ts,
-                                call_id,
+                                result_key(&call_id),
                                 ToolOutput::Text(format_web_search_result(payload)),
                                 false,
                                 false,
@@ -313,7 +331,13 @@ fn lines_to_messages(lines: &[Line], fallback_ts: DateTime<Utc>) -> Vec<Message>
                                 .map_or(ToolOutput::Text(String::new()), |s| {
                                     ToolOutput::Text(s.to_string())
                                 });
-                            queued.push(tool_result(ts, call_id, content, false, true));
+                            queued.push(tool_result(
+                                ts,
+                                result_key(&call_id),
+                                content,
+                                false,
+                                true,
+                            ));
                         }
                     }
                     "custom_tool_call" => {
@@ -357,8 +381,14 @@ fn lines_to_messages(lines: &[Line], fallback_ts: DateTime<Utc>) -> Vec<Message>
                                 .and_then(Value::as_str)
                                 .unwrap_or_default();
                             let (content, is_error) = parse_custom_tool_output(raw);
-                            canonical_results.insert(call_id.clone());
-                            queued.push(tool_result(ts, call_id, content, is_error, false));
+                            canonical_results.insert(result_key(&call_id));
+                            queued.push(tool_result(
+                                ts,
+                                result_key(&call_id),
+                                content,
+                                is_error,
+                                false,
+                            ));
                         }
                     }
                     "web_search_call" => {
@@ -398,12 +428,12 @@ fn lines_to_messages(lines: &[Line], fallback_ts: DateTime<Utc>) -> Vec<Message>
         }
     }
 
-    // Drop the fallback function_call_output when a canonical result exists.
+    // Drop a fallback only when this call occurrence has a canonical result.
     queued
         .into_iter()
         .filter(|q| {
             !q.is_fallback_result
-                || q.result_call_id
+                || q.result_key
                     .as_ref()
                     .is_none_or(|c| !canonical_results.contains(c))
         })
@@ -425,7 +455,7 @@ fn plain(role: Role, content: Vec<Block>, ts: DateTime<Utc>, model: Option<Strin
         timestamp: ts,
         model,
         usage: None,
-        result_call_id: None,
+        result_key: None,
         is_fallback_result: false,
     }
 }
@@ -446,29 +476,29 @@ fn tool_use(
         timestamp: ts,
         model,
         usage: None,
-        result_call_id: None,
+        result_key: None,
         is_fallback_result: false,
     }
 }
 
 fn tool_result(
     ts: DateTime<Utc>,
-    call_id: String,
+    result_key: (String, usize),
     content: ToolOutput,
     is_error: bool,
     is_fallback: bool,
 ) -> Queued {
     Queued {
         role: Role::User,
         content: vec![Block::ToolResult {
-            tool_use_id: call_id.clone(),
+            tool_use_id: result_key.0.clone(),
             content,
             is_error,
         }],
         timestamp: ts,
         model: None,
         usage: None,
-        result_call_id: Some(call_id),
+        result_key: Some(result_key),
         is_fallback_result: is_fallback,
     }
 }
@@ -501,17 +531,9 @@ fn messages_to_lines(meta: &Meta, messages: &[Message]) -> Vec<Line> {
     }
     lines.push(meta_line(&meta.timestamp, "session_meta", payload));
 
-    // Calls Codex issued as `custom_tool_call`, looked up when emitting
-    // results: a patch call pairs with `custom_tool_call_output`,
-    // everything else with `function_call_output`.
-    let patch_ids: HashSet<&str> = messages
-        .iter()
-        .flat_map(|msg| &msg.content)
-        .filter_map(|block| match block {
-            Block::ToolUse { id, tool } if is_patch_tool(tool) => Some(id.as_str()),
-            _ => None,
-        })
-        .collect();
+    // Only pending patch calls need custom-tool results. Completed IDs can
+    // be reused by a different tool later in the transcript.
+    let mut pending_patch_ids = HashSet::new();
 
     for (i, msg) in messages.iter().enumerate() {
         let ts = msg.timestamp.to_rfc3339_opts(SecondsFormat::Millis, true);
@@ -528,7 +550,7 @@ fn messages_to_lines(meta: &Meta, messages: &[Message]) -> Vec<Line> {
             lines.push(meta_line(&msg.timestamp, "turn_context", tc));
         }
 
-        push_message_lines(&mut lines, msg, &ts, &patch_ids);
+        push_message_lines(&mut lines, msg, &ts, &mut pending_patch_ids);
 
         if matches!(msg.role, Role::Assistant)
             && let Some(usage) = msg.usage.as_ref()
@@ -556,7 +578,12 @@ fn messages_to_lines(meta: &Meta, messages: &[Message]) -> Vec<Line> {
 }
 
 /// Emit the `response_item` (and paired display `event_msg`) lines for one message.
-fn push_message_lines(lines: &mut Vec<Line>, msg: &Message, ts: &str, patch_ids: &HashSet<&str>) {
+fn push_message_lines<'a>(
+    lines: &mut Vec<Line>,
+    msg: &'a Message,
+    ts: &str,
+    pending_patch_ids: &mut HashSet<&'a str>,
+) {
     let role_str = match msg.role {
         Role::User => "user",
         Role::Assistant => "assistant",
@@ -605,33 +632,32 @@ fn push_message_lines(lines: &mut Vec<Line>, msg: &Message, ts: &str, patch_ids:
                     json!({ "type": "agent_reasoning", "text": text }),
                 ));
             }
-            Block::ToolUse { id, tool } => push_tool_use_lines(lines, ts, id, tool),
+            Block::ToolUse { id, tool } => {
+                if is_patch_tool(tool) {
+                    pending_patch_ids.insert(id.as_str());
+                } else {
+                    pending_patch_ids.remove(id.as_str());
+                }
+                push_tool_use_lines(lines, ts, id, tool);
+            }
             Block::ToolResult {
                 tool_use_id,
                 content,
                 is_error,
             } => {
-                if patch_ids.contains(tool_use_id.as_str()) {
-                    lines.push(meta_line_str(
-                        ts,
-                        "response_item",
-                        json!({
-                            "type": "custom_tool_call_output",
-                            "call_id": tool_use_id,
-                            "output": custom_tool_output(content, *is_error),
-                        }),
-                    ));
+                let (kind, output) = if pending_patch_ids.remove(tool_use_id.as_str()) {
+                    (
+                        "custom_tool_call_output",
+                        custom_tool_output(content, *is_error),
+                    )
                 } else {
-                    lines.push(meta_line_str(
-                        ts,
-                        "response_item",
-                        json!({
-                            "type": "function_call_output",
-                            "call_id": tool_use_id,
-                            "output": tool_output_text(content),
-                        }),
-                    ));
-                }
+                    ("function_call_output", tool_output_text(content))
+                };
+                lines.push(meta_line_str(
+                    ts,
+                    "response_item",
+                    json!({ "type": kind, "call_id": tool_use_id, "output": output }),
+                ));
             }
         }
     }
@@ -656,8 +682,8 @@ fn push_message_lines(lines: &mut Vec<Line>, msg: &Message, ts: &str, patch_ids:
 }
 
 /// Emit the native call line for one tool invocation: `exec_command` for
-/// shell, `custom_tool_call` for edits, `web_search_call` for search — the
-/// exact reverse of the inbound normalization, so Codex validates the replay.
+/// shell and `custom_tool_call` for edits. Foreign tools keep their canonical
+/// function-call form and their paired function-call results.
 fn push_tool_use_lines(lines: &mut Vec<Line>, ts: &str, id: &str, tool: &Tool) {
     match tool {
         Tool::Bash {
@@ -683,18 +709,6 @@ fn push_tool_use_lines(lines: &mut Vec<Line>, ts: &str, id: &str, tool: &Tool) {
                 }),
             ));
         }
-        Tool::Raw { tool_name, input } if tool_name == "WebSearch" => {
-            lines.push(meta_line_str(
-                ts,
-                "response_item",
-                json!({
-                    "type": "web_search_call",
-                    "status": "completed",
-                    "call_id": id,
-                    "action": input,
-                }),
-            ));
-        }
         Tool::Edit {
             file_path,
             old_string,
@@ -705,26 +719,24 @@ fn push_tool_use_lines(lines: &mut Vec<Line>, ts: &str, id: &str, tool: &Tool) {
                 lines,
                 ts,
                 id,
-                &Value::String(apply_patch_update(file_path, old_string, new_string)),
+                &apply_patch_update(file_path, old_string, new_string),
             );
         }
         Tool::Write { file_path, content } => {
-            push_custom_tool_call(
-                lines,
-                ts,
-                id,
-                &Value::String(apply_patch_add(file_path, content)),
-            );
+            push_custom_tool_call(lines, ts, id, &apply_patch_add(file_path, content));
         }
         Tool::Raw { tool_name, input } if tool_name == "ApplyPatch" => {
             // The fallback shape inbound keeps is
             // `{"patch": <envelope>, "files": [...]}`; unwrap it
             // so live Codex sees the string input it wrote.
-            let input = match input {
-                Value::Object(obj) if obj.get("patch").is_some_and(Value::is_string) => {
-                    obj["patch"].clone()
-                }
-                _ => input.clone(),
+            let input = match input.get("patch").and_then(Value::as_str) {
+                Some(patch) => patch.to_owned(),
+                None => match input {
+                    Value::String(text) => text.clone(),
+                    // Even a malformed historical call must satisfy Codex's
+                    // string input type. The reader decodes JSON strings.
+                    other => other.to_string(),
+                },
             };
             push_custom_tool_call(lines, ts, id, &input);
         }
@@ -777,7 +789,7 @@ fn apply_patch_add(file_path: &str, content: &str) -> String {
     patch.join("\n")
 }
 
-fn push_custom_tool_call(lines: &mut Vec<Line>, ts: &str, id: &str, input: &Value) {
+fn push_custom_tool_call(lines: &mut Vec<Line>, ts: &str, id: &str, input: &str) {
     lines.push(meta_line_str(
         ts,
         "response_item",