From ef740c54d1adb791f45c7a08536610dd262740bc Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 28 Oct 2026 11:48:51 -0400 Subject: [PATCH 1/3] fix(codex): distinguish command exits from wrapper completion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit > 🤖 **This text was AI-generated.** Codex can complete an exec wrapper while a nested command fails, including when write_stdin returns the final result. Previously, traces could show: ```text wrapper: Script completed command: exit_code=1 span.error: null ``` Interpret native command results separately from wrapper completion while retaining the returned output: ```text span.error: Command exited with code 1 metadata.wrapper_outcome: completed metadata.command_outcomes: [{outcome: nonzero_exit, exit_code: 1}] ``` Keep successful calls free of empty errors and record explicit cancellation without inferring it from missing output. Ignore failure-like text in stdout. Nonzero exits describe process status; grep can return one for an expected result. Statuses discarded or truncated by scripts remain unknown. Refs: SDK-708 --- bt-daemon/src/translate/codex.rs | 131 ++++----- bt-daemon/src/translate/codex/tool_output.rs | 264 +++++++++++++++++++ bt-daemon/tests/agent_integration.rs | 69 ++--- bt-daemon/tests/codex_translator.rs | 178 +++++++++++++ 4 files changed, 512 insertions(+), 130 deletions(-) create mode 100644 bt-daemon/src/translate/codex/tool_output.rs diff --git a/bt-daemon/src/translate/codex.rs b/bt-daemon/src/translate/codex.rs index ea5a8a6c..f6731819 100644 --- a/bt-daemon/src/translate/codex.rs +++ b/bt-daemon/src/translate/codex.rs @@ -20,7 +20,7 @@ use super::git::GitMetadataCache; use super::recent::{RecentMap, RecentSet}; use super::tool::{ - error_text, explicit_skill_metadata, nonempty_error_text, tool_approval_metadata, ToolApproval, + add_tool_approval, explicit_skill_metadata, tool_approval_metadata, ToolApproval, }; use super::{ local_username, AgentTranslator, SessionCtx, SpanOp, SpanRow, SpanType, TranslatorFactory, @@ -35,6 +35,8 @@ use std::collections::HashMap; use std::path::Path; use std::sync::{Arc, OnceLock}; +mod tool_output; + const SPAWN_AGENT_TOOL: &str = "spawn_agent"; const MISSING_TOOL_OUTPUT_ERROR: &str = "Tool output missing before turn ended"; const MAX_OPEN_TOOL_SPAN_MS: i64 = 24 * 60 * 60 * 1000; @@ -169,6 +171,13 @@ struct OpenLlm { output_preset: bool, } +struct OpenTool { + span_id: String, + turn_id: String, + start_ms: i64, + name: String, +} + struct Scope { path: String, kind: ScopeKind, @@ -182,7 +191,7 @@ struct Scope { open_turns: Vec, message_history: Vec, open_llm: Option, - open_tools: HashMap, // call_id -> (tool span_id, turn_id, start_ms) + open_tools: HashMap, // call_id -> native tool invocation last_turn_end_ms: Option, last_turn_output: Option, turn_seq: u32, @@ -1191,7 +1200,15 @@ impl CodexTranslator { tags: if tags.is_empty() { None } else { Some(tags) }, ..Default::default() })); - scope.open_tools.insert(call_id, (span_id, turn_id, ts)); + scope.open_tools.insert( + call_id, + OpenTool { + span_id, + turn_id, + start_ms: ts, + name: tool_name, + }, + ); } fn on_tool_output( @@ -1206,13 +1223,13 @@ impl CodexTranslator { return; }; push_tool_result(scope, Some(&call_id), payload); - let Some((span_id, turn_id, _start_ms)) = scope.open_tools.remove(&call_id) else { + let Some(tool) = scope.open_tools.remove(&call_id) else { return; }; if let Some(turn) = scope .open_turns .iter_mut() - .find(|turn| turn.turn_id == turn_id) + .find(|turn| turn.turn_id == tool.turn_id) { turn.last_child_end_ms = Some(turn.last_child_end_ms.map_or(ts, |p| p.max(ts))); } @@ -1220,15 +1237,19 @@ impl CodexTranslator { .get("output") .or_else(|| payload.get("result")) .cloned(); - let error = output.as_ref().and_then(classify_tool_output); + let mut outcome = output + .as_ref() + .map(|output| tool_output::classify(&tool.name, output)) + .unwrap_or_default(); + add_tool_approval(&mut outcome.metadata, Some(ToolApproval::Approved)); ops.push(SpanOp::Merge(SpanRow { - span_id, + span_id: tool.span_id, root_span_id: self.effective_root_span_id.clone(), - parent_span_ids: self.turn_parent_span_ids(&turn_id), + parent_span_ids: self.turn_parent_span_ids(&tool.turn_id), end_ms: Some(ts), output, - metadata: Some(tool_approval_metadata(Some(ToolApproval::Approved))), - error, + metadata: Some(Value::Object(outcome.metadata)), + error: outcome.error, ..Default::default() })); } @@ -1321,7 +1342,7 @@ impl CodexTranslator { let has_open_tool = scope .open_tools .values() - .any(|(_, owner, _)| owner == &turn_id); + .any(|tool| tool.turn_id == turn_id); if scope .open_llm @@ -1395,11 +1416,11 @@ impl CodexTranslator { let call_ids: Vec = scope .open_tools .iter() - .filter(|(_, (_, owner, _))| owner == turn_id) + .filter(|(_, tool)| tool.turn_id == turn_id) .map(|(call_id, _)| call_id.clone()) .collect(); for call_id in call_ids { - if let Some((span_id, _, start_ms)) = scope.open_tools.remove(&call_id) { + if let Some(tool) = scope.open_tools.remove(&call_id) { let mut metadata = tool_approval_metadata(Some(ToolApproval::Approved)); if let Some(reason) = interruption_reason { if let Some(fields) = metadata.as_object_mut() { @@ -1408,12 +1429,12 @@ impl CodexTranslator { } } ops.push(SpanOp::Merge(SpanRow { - span_id, + span_id: tool.span_id, root_span_id: self.effective_root_span_id.clone(), parent_span_ids: parent_span_ids.clone(), end_ms: Some(end_ms.map_or_else( - || start_ms.saturating_add(MAX_OPEN_TOOL_SPAN_MS), - |end| end.min(start_ms.saturating_add(MAX_OPEN_TOOL_SPAN_MS)), + || tool.start_ms.saturating_add(MAX_OPEN_TOOL_SPAN_MS), + |end| end.min(tool.start_ms.saturating_add(MAX_OPEN_TOOL_SPAN_MS)), )), metadata: Some(metadata), error: interruption_reason @@ -1586,29 +1607,18 @@ impl CodexTranslator { ..Default::default() })); } - let tools: Vec<(String, String, i64, String)> = scope - .open_tools - .drain() - .map(|(_, (span_id, turn_id, start_ms))| { - ( - span_id, - turn_id, - start_ms, - MISSING_TOOL_OUTPUT_ERROR.to_string(), - ) - }) - .collect(); - for (sid, turn_id, start_ms, error) in tools { + let tools: Vec = scope.open_tools.drain().map(|(_, tool)| tool).collect(); + for tool in tools { ops.push(SpanOp::Merge(SpanRow { - span_id: sid, + span_id: tool.span_id, root_span_id: self.effective_root_span_id.clone(), - parent_span_ids: self.turn_parent_span_ids(&turn_id), + parent_span_ids: self.turn_parent_span_ids(&tool.turn_id), end_ms: Some(end_ms.map_or_else( - || start_ms.saturating_add(MAX_OPEN_TOOL_SPAN_MS), - |end| end.min(start_ms.saturating_add(MAX_OPEN_TOOL_SPAN_MS)), + || tool.start_ms.saturating_add(MAX_OPEN_TOOL_SPAN_MS), + |end| end.min(tool.start_ms.saturating_add(MAX_OPEN_TOOL_SPAN_MS)), )), metadata: Some(tool_approval_metadata(Some(ToolApproval::Approved))), - error: Some(error), + error: Some(MISSING_TOOL_OUTPUT_ERROR.to_string()), ..Default::default() })); } @@ -1809,59 +1819,6 @@ fn args_object(args: Option<&Value>) -> Option> { } } -fn has_error_marker(value: &Value) -> bool { - match value { - Value::Null => false, - Value::Bool(value) => *value, - Value::String(_) => nonempty_error_text(value).is_some(), - Value::Array(values) => !values.is_empty(), - Value::Object(values) => !values.is_empty(), - Value::Number(_) => true, - } -} - -fn classify_tool_output(output: &Value) -> Option { - if let Some(object) = output.as_object() { - if object.get("is_error").and_then(Value::as_bool) == Some(true) - || object.get("isError").and_then(Value::as_bool) == Some(true) - || matches!( - object.get("status").and_then(Value::as_str), - Some("error" | "failed") - ) - { - return Some(error_text(Some(output), "Tool execution failed")); - } - if let Some(error) = object.get("error").filter(|error| has_error_marker(error)) { - return Some(error_text(Some(error), "Tool execution failed")); - } - if let Some(exit_code) = object - .get("exit_code") - .or_else(|| object.get("exitCode")) - .and_then(Value::as_i64) - { - if exit_code != 0 { - return Some(error_text(Some(output), &format!("Exit code {exit_code}"))); - } - } - } - if let Some(text) = output.as_str() { - let first = text.lines().next().unwrap_or(text); - if first.to_ascii_lowercase().starts_with("error:") { - return Some(first.to_string()); - } - if let Some(code) = first - .strip_prefix("Exit code ") - .and_then(|value| value.split_whitespace().next()) - .and_then(|value| value.parse::().ok()) - { - if code != 0 { - return Some(first.to_string()); - } - } - } - None -} - #[derive(Default)] struct SkillLoad { name: Option, diff --git a/bt-daemon/src/translate/codex/tool_output.rs b/bt-daemon/src/translate/codex/tool_output.rs new file mode 100644 index 00000000..f43fc6fb --- /dev/null +++ b/bt-daemon/src/translate/codex/tool_output.rs @@ -0,0 +1,264 @@ +//! Interpret native result envelopes without interpreting command stdout. +//! +//! Code-mode `exec` can complete while one or more commands fail or keep +//! running. Keep those facts separate, and never turn missing output into a +//! confirmed interruption. The metadata allowlist here is wrapper_outcome, +//! command_outcomes (outcome, exit_code, session_id), and outcome for an explicit +//! interruption. Raw tool output is retained by the caller. + +use super::super::tool::{error_text, nonempty_error_text}; +use serde_json::{json, Map, Value}; + +#[derive(Default)] +pub(super) struct ToolOutcome { + pub error: Option, + pub metadata: Map, + commands: Vec, +} + +pub(super) fn classify(tool_name: &str, output: &Value) -> ToolOutcome { + let mut outcome = ToolOutcome::default(); + if matches!( + tool_name.rsplit('.').next(), + Some("exec" | "wait" | "exec_command" | "write_stdin" | "shell" | "shell_command") + ) { + outcome.observe(output, 0); + if outcome.commands.is_empty() && outcome.error.is_none() { + outcome.error = native_error(output); + } + } else { + // Preserve explicit failure signals on other tools, but do not search + // their JSON documents or text blocks for execution telemetry. + outcome.error = native_error(output); + if let Some(code) = exit_code(output) { + outcome.command(Some(code), None, false); + } + } + if !outcome.commands.is_empty() { + if outcome.error.is_none() + && outcome + .commands + .iter() + .any(|command| command["outcome"] == "interrupted") + { + outcome + .metadata + .insert("outcome".into(), json!("interrupted")); + } + outcome.metadata.insert( + "command_outcomes".into(), + Value::Array(std::mem::take(&mut outcome.commands)), + ); + } + outcome +} + +impl ToolOutcome { + fn command(&mut self, code: Option, session_id: Option<&Value>, interrupted: bool) { + let status = if interrupted { + "interrupted" + } else if let Some(code) = code { + if code == 0 { + "success" + } else { + self.error + .get_or_insert_with(|| format!("Command exited with code {code}")); + "nonzero_exit" + } + } else { + "running" + }; + let mut fields = Map::new(); + fields.insert("outcome".into(), json!(status)); + if let Some(code) = code { + fields.insert("exit_code".into(), json!(code)); + } + if let Some(id) = session_id { + fields.insert("session_id".into(), id.clone()); + } + self.commands.push(Value::Object(fields)); + } + + fn observe(&mut self, value: &Value, depth: usize) { + // Bound recursion through JSON-encoded and nested code-mode results. + if depth > 16 { + return; + } + match value { + Value::Object(object) => { + let interrupted = matches!( + object.get("status").and_then(Value::as_str), + Some("cancelled" | "canceled" | "interrupted" | "aborted") + ); + if !interrupted && self.error.is_none() { + self.error = native_error(value); + } + let code = exit_code(value); + let session_id = object + .get("session_id") + .filter(|id| id.is_i64() || id.is_string()); + if code.is_some() + || interrupted + || (session_id.is_some() && object.contains_key("output")) + { + self.command(code, session_id, interrupted); + // stdout/stderr/output can themselves contain JSON or + // error-like text. They are diagnostics, not child results. + return; + } + if matches!( + object.get("type").and_then(Value::as_str), + Some("input_text" | "output_text" | "text") + ) { + if let Some(text) = object.get("text").and_then(Value::as_str) { + self.observe_text(text, depth + 1); + } + } else { + // Known container shapes, including Promise.allSettled. + // Do not recursively scan arbitrary API payload fields. + for key in ["result", "results", "value", "content"] { + if let Some(nested) = object.get(key) { + self.observe(nested, depth + 1); + } + } + } + } + Value::Array(values) => { + for value in values { + self.observe(value, depth + 1); + } + } + Value::String(text) => { + self.observe_text(text, depth + 1); + } + _ => {} + } + } + + fn observe_text(&mut self, text: &str, depth: usize) { + if depth > 16 { + return; + } + let first = text.lines().next().unwrap_or(text); + let wrapper = if first == "Script completed" { + Some("completed") + } else if first.starts_with("Script running with cell ID ") { + Some("running") + } else { + None + }; + if let Some(wrapper) = wrapper { + self.metadata + .insert("wrapper_outcome".into(), json!(wrapper)); + if let Some((_, body)) = text.split_once("\nOutput:\n") { + self.observe_text(body, depth + 1); + } + return; + } + if self.observe_command_header(text) { + return; + } + if let Ok(value) = serde_json::from_str::(text) { + self.observe(&value, depth + 1); + } else { + // Each text() call can add a JSON result to one output block. A + // truncation notice may precede these independently valid records. + for line in text.lines() { + if let Ok(value) = serde_json::from_str::(line) { + self.observe(&value, depth + 1); + } + } + } + } + + fn observe_command_header(&mut self, text: &str) -> bool { + let first = text.lines().next().unwrap_or(text); + if ![ + "Chunk ID:", + "Wall time:", + "Process exited with code ", + "Process running with session ID ", + "Exit code ", + ] + .iter() + .any(|prefix| first.starts_with(prefix)) + { + return false; + } + // Native result metadata ends before Final output. Never search the + // command's printed output for an exit status, including on success. + let header = text + .split_once("\nFinal output:") + .map_or(text, |(header, _)| header); + for line in header.lines() { + if let Some(code) = line + .strip_prefix("Process exited with code ") + .or_else(|| line.strip_prefix("Exit code ")) + .and_then(|s| s.split_whitespace().next()) + .and_then(|s| s.parse::().ok()) + { + self.command(Some(code), None, false); + return true; + } + if let Some(id) = line.strip_prefix("Process running with session ID ") { + let id = id.trim(); + let id = id.parse::().map_or_else(|_| json!(id), |id| json!(id)); + self.command(None, Some(&id), false); + return true; + } + } + // A recognized header still separates stdout from telemetry when its + // terminal status was truncated or omitted. Do not parse the body. + true + } +} + +fn exit_code(value: &Value) -> Option { + value + .get("exit_code") + .or_else(|| value.get("exitCode")) + .and_then(Value::as_i64) +} + +fn has_error_marker(value: &Value) -> bool { + match value { + Value::Null => false, + Value::Bool(value) => *value, + Value::String(_) => nonempty_error_text(value).is_some(), + Value::Array(values) => !values.is_empty(), + Value::Object(values) => !values.is_empty(), + Value::Number(_) => true, + } +} + +fn native_error(output: &Value) -> Option { + if let Some(object) = output.as_object() { + if object.get("is_error").and_then(Value::as_bool) == Some(true) + || object.get("isError").and_then(Value::as_bool) == Some(true) + || matches!( + object.get("status").and_then(Value::as_str), + Some("error" | "failed") + ) + { + return Some(error_text(Some(output), "Tool execution failed")); + } + if let Some(error) = object.get("error").filter(|error| has_error_marker(error)) { + return Some(error_text(Some(error), "Tool execution failed")); + } + } + if let Some(text) = output.as_str() { + let first = text.lines().next().unwrap_or(text); + if first.to_ascii_lowercase().starts_with("error:") { + return Some(first.to_owned()); + } + if first + .strip_prefix("Exit code ") + .and_then(|value| value.split_whitespace().next()) + .and_then(|value| value.parse::().ok()) + .is_some_and(|code| code != 0) + { + return Some(first.to_owned()); + } + } + None +} diff --git a/bt-daemon/tests/agent_integration.rs b/bt-daemon/tests/agent_integration.rs index 20de9b3b..fec5de37 100644 --- a/bt-daemon/tests/agent_integration.rs +++ b/bt-daemon/tests/agent_integration.rs @@ -19,35 +19,9 @@ use support::inference::{ use support::ingest::IngestScenario; use support::server::TestServer; -fn codex_tool_call(request: &OpenAiRequest) -> OpenAiTurn { - let names = request.tool_names(); - if names.contains(&"exec_command") { - return OpenAiTurn::tool_call( - "call_mock_1", - "exec_command", - json!({"cmd":codex_tool_command(),"login":false}), - ); - } - if names.contains(&"shell") { - return OpenAiTurn::tool_call( - "call_mock_1", - "shell", - json!({"command":codex_tool_command()}), - ); - } - if names.contains(&"shell_command") { - return OpenAiTurn::tool_call( - "call_mock_1", - "shell_command", - json!({"command":codex_tool_command()}), - ); - } - panic!("Codex offered no supported shell tool; offered tools: {names:?}"); -} - fn shell_tool_call(request: &OpenAiRequest, call_id: &str, command: &str) -> OpenAiTurn { let names = request.tool_names(); - let name = ["bash", "shell", "shell_command", "exec_command"] + let name = ["bash", "exec_command", "shell", "shell_command"] .into_iter() .find(|name| names.contains(name)) .unwrap_or_else(|| { @@ -72,17 +46,6 @@ fn tool_command(marker: &str) -> String { } } -fn codex_tool_command() -> &'static str { - #[cfg(unix)] - { - "printf CODEX_TOOL_OK" - } - #[cfg(windows)] - { - "Write-Output CODEX_TOOL_OK" - } -} - fn row_contains(row: &Value, fragments: &[&str]) -> bool { let serialized = serde_json::to_string(row).expect("serialize trace row"); fragments @@ -102,7 +65,11 @@ async fn codex_session_emits_traces() { "unexpected Codex request: {}", request.body ); - MockReply::response(codex_tool_call(&request)) + MockReply::response(shell_tool_call( + &request, + "call_mock_1", + &tool_command("CODEX_TOOL_OK"), + )) } 1 => { assert!( @@ -110,9 +77,17 @@ async fn codex_session_emits_traces() { "Codex did not return the tool result: {}", request.body ); + MockReply::response(shell_tool_call( + &request, + "call_mock_failure", + &format!("{}; exit 7", tool_command("CODEX_TOOL_FAILED")), + )) + } + 2 => { + assert!(request.has_function_output("call_mock_failure")); MockReply::response(OpenAiTurn::text("CODEX_MOCK_OK")) } - 2 => MockReply::http_error( + 3 => MockReply::http_error( StatusCode::BAD_REQUEST, json!({ "error": { @@ -142,7 +117,7 @@ async fn codex_session_emits_traces() { output.assert_success(); if world.uses_mock_inference() { output.assert_contains("CODEX_MOCK_OK"); - assert_eq!(inference.requests().len(), 2); + assert_eq!(inference.requests().len(), 3); let failed = codex .run( @@ -153,7 +128,7 @@ async fn codex_session_emits_traces() { .await; failed.assert_failure(); failed.assert_contains("deterministic Codex inference failure"); - assert_eq!(inference.requests().len(), 3); + assert_eq!(inference.requests().len(), 4); } let rows = world.wait_for_trace_delivery().await; @@ -171,6 +146,14 @@ async fn codex_session_emits_traces() { }) .expect("Codex tool output", |row| { row_contains(row, &[r#""type":"tool""#, "CODEX_TOOL_OK"]) + && row["error"].is_null() + && row["metadata"]["command_outcomes"][0]["exit_code"] == 0 + }) + .expect("Codex command failure", |row| { + row_contains(row, &["CODEX_TOOL_FAILED"]) + && row["error"] == "Command exited with code 7" + && row["metadata"]["command_outcomes"][0]["outcome"] == "nonzero_exit" + && row["metadata"]["command_outcomes"][0]["exit_code"] == 7 }); world.wait_for_mock_ingest_scenario(&scenario).await; } @@ -517,7 +500,7 @@ fn request_helpers_recognize_tool_results_and_advertised_tools() { }; assert!(openai.has_function_output("call-1")); assert_eq!(openai.tool_names(), vec!["shell_command"]); - match codex_tool_call(&openai) { + match shell_tool_call(&openai, "call_mock_1", "echo CODEX_TOOL_OK") { OpenAiTurn::ToolCall { name, arguments, .. } => { diff --git a/bt-daemon/tests/codex_translator.rs b/bt-daemon/tests/codex_translator.rs index 84937d8f..b52bd358 100644 --- a/bt-daemon/tests/codex_translator.rs +++ b/bt-daemon/tests/codex_translator.rs @@ -1286,6 +1286,180 @@ fn codex_successful_structured_tool_outputs_do_not_populate_error() { ); } +/// Exercise the public translator with the native code-mode output shape. +fn exec_output(result: Value) -> Value { + json!([ + {"type":"input_text","text":"Script completed\nWall time 0.2 seconds\nOutput:\n"}, + {"type":"input_text","text":result.to_string()} + ]) +} + +fn translate_tool_output(name: &str, output: Value) -> SpanRow { + let tmp = tempfile::tempdir().unwrap(); + let transcript = tmp.path().join("rollout.jsonl"); + for record in [ + json!({"timestamp":"2026-01-01T00:00:01Z","type":"session_meta","payload":{"id":"s","cwd":"/x/app"}}), + json!({"timestamp":"2026-01-01T00:00:02Z","type":"event_msg","payload":{"type":"task_started","turn_id":"t1"}}), + json!({"timestamp":"2026-01-01T00:00:03Z","type":"response_item","payload":{"type":"custom_tool_call","name":name,"call_id":"c1","input":"text(await tools.exec_command({cmd:'git commit'}));"}}), + json!({"timestamp":"2026-01-01T00:00:04Z","type":"response_item","payload":{"type":"custom_tool_call_output","call_id":"c1","output":output}}), + json!({"timestamp":"2026-01-01T00:00:05Z","type":"event_msg","payload":{"type":"task_complete","turn_id":"t1"}}), + ] { + append(&transcript, record); + } + let mut translator = Registry::default_agents().create("codex", "s"); + let ctx = SessionCtx { + session_id: "s".into(), + config: None, + }; + let mut ops = translator + .handle( + &envelope("s", "SessionStart", transcript.to_str().unwrap(), json!({})), + &ctx, + ) + .unwrap(); + ops.extend(translator.flush(&ctx).unwrap()); + assert_merges_preserve_insert_identity(&ops); + let rows = reduce(ops); + find(&rows, SpanType::Tool, name).clone() +} + +#[test] +fn codex_nested_command_failures_survive_wrapper_success() { + // Sanitized shape of SDK-708's completed write_stdin result, Codex 0.158. + let output = exec_output(json!({ + "chunk_id":"a1b8b4", "wall_time_seconds":0.00001, + "exit_code":1, "original_token_count":291, + "output":"Passed\nDetect hardcoded secrets........Failed\n- hook id: gitleaks\n- exit code: 1\n" + })); + let tool = translate_tool_output("exec", output.clone()); + assert_eq!(tool.error.as_deref(), Some("Command exited with code 1")); + assert_eq!(tool.output, Some(output)); + let metadata = tool.metadata.unwrap(); + assert_eq!(metadata["wrapper_outcome"], "completed"); + assert_eq!( + metadata["command_outcomes"], + json!([{"outcome":"nonzero_exit","exit_code":1}]) + ); +} + +#[test] +fn codex_command_result_formats_preserve_outcomes() { + let failed = json!([{"outcome":"nonzero_exit","exit_code":2}]); + let success = json!([{"outcome":"success","exit_code":0}]); + let mut multiple = exec_output(Value::Null); + multiple[1]["text"] = json!(format!( + "{}\n{}", + json!({"status":"fulfilled","value":{"exit_code":2,"output":"no matches"}}), + json!({"result":{"exit_code":0,"output":"ok"}}), + )); + // Each case covers a different native envelope or a false-positive boundary. + // PR #71's test above already covers null, blank, and false error members. + for (name, output, error, commands) in [ + ("functions.exec_command", json!({"exit_code":2,"output":"bad command"}), Some("Command exited with code 2"), failed.clone()), + ("shell", json!(r#"{"exitCode":2,"output":"bad command"}"#), Some("Command exited with code 2"), failed.clone()), + ("exec_command", json!("Chunk ID: abc\nWall time: 0.1 seconds\nProcess exited with code 2\nFinal output:\nbad command"), Some("Command exited with code 2"), failed.clone()), + ("exec_command", json!("Exit code 2\nbad command"), Some("Command exited with code 2"), failed.clone()), + ("exec", json!("Script completed\nWall time 0.1 seconds\nOutput:\n{\"exit_code\":2,\"output\":\"bad command\"}"), Some("Command exited with code 2"), failed), + ("exec", multiple, Some("Command exited with code 2"), json!([{"outcome":"nonzero_exit","exit_code":2},{"outcome":"success","exit_code":0}])), + ("exec", exec_output(json!({"exit_code":0,"error":null,"output":"{\"exit_code\":1,\"error\":\"example\"}","stderr":"Error: harmless diagnostic"})), None, success.clone()), + ("exec_command", json!("Chunk ID: abc\nProcess exited with code 0\nFinal output:\nProcess exited with code 1\n{\"exit_code\":1}"), None, success), + ("exec_command", json!("Chunk ID: abc\nFinal output:\n{\"exit_code\":1}"), None, Value::Null), + ("read_document", json!({"content":[{"type":"text","text":"{\"exit_code\":1}"}]}), None, Value::Null), + ("read_document", json!(r#"{"exit_code":1,"output":"an example"}"#), None, Value::Null), + ("exec_command", json!({"status":"cancelled","exit_code":130,"output":"user cancelled"}), None, json!([{"outcome":"interrupted","exit_code":130}])), + ("exec", exec_output(json!({"status":"interrupted","output":"user interrupted"})), None, json!([{"outcome":"interrupted"}])), + ("exec_command", json!({"session_id":77,"exit_code":null,"output":""}), None, json!([{"outcome":"running","session_id":77}])), + ("exec_command", json!("Error: sandbox denied execution"), Some("Error: sandbox denied execution"), Value::Null), + ] { + let tool = translate_tool_output(name, output.clone()); + assert_eq!(tool.error.as_deref(), error, "{name}: {output}"); + assert_eq!(tool.output, Some(output)); + let metadata = tool.metadata.unwrap(); + assert_eq!(metadata["command_outcomes"], commands, "{name}"); + let interrupted = commands.as_array().is_some_and(|values| values.iter().any(|v| v["outcome"]=="interrupted")); + assert_eq!(metadata.get("outcome").and_then(Value::as_str), interrupted.then_some("interrupted")); + } +} + +#[test] +fn codex_command_polling_records_the_terminal_result() { + let tmp = tempfile::tempdir().unwrap(); + let transcript = tmp.path().join("rollout.jsonl"); + let path = transcript.to_str().unwrap(); + let initial_output = exec_output(json!({"session_id":38764,"output":""})); + for record in [ + json!({"timestamp":"2026-01-01T00:00:01Z","type":"session_meta","payload":{"id":"s","cwd":"/x/app"}}), + json!({"timestamp":"2026-01-01T00:00:02Z","type":"event_msg","payload":{"type":"task_started","turn_id":"t1"}}), + json!({"timestamp":"2026-01-01T00:00:03Z","type":"response_item","payload":{"type":"custom_tool_call","name":"exec","call_id":"start","input":"text(await tools.exec_command({cmd:'git commit'}));"}}), + json!({"timestamp":"2026-01-01T00:00:04Z","type":"response_item","payload":{"type":"custom_tool_call_output","call_id":"start","output":initial_output}}), + json!({"timestamp":"2026-01-01T00:00:05Z","type":"response_item","payload":{"type":"custom_tool_call","name":"exec","call_id":"poll","input":"text(await tools.write_stdin({session_id:38764,chars:''}));"}}), + ] { + append(&transcript, record); + } + let mut translator = Registry::default_agents().create("codex", "s"); + let ctx = SessionCtx { + session_id: "s".into(), + config: None, + }; + let mut ops = translator + .handle(&envelope("s", "SessionStart", path, json!({})), &ctx) + .unwrap(); + let before = reduce(ops.clone()); + let start = before + .values() + .find(|r| r.metadata.as_ref().is_some_and(|m| m["call_id"] == "start")) + .unwrap(); + assert_eq!(start.error, None); + assert_eq!( + start.metadata.as_ref().unwrap()["command_outcomes"][0]["outcome"], + "running" + ); + let output = exec_output(json!({"chunk_id":"a1b8b4","exit_code":1,"output":"gitleaks failed"})); + append( + &transcript, + json!({"timestamp":"2026-01-01T00:00:06Z","type":"response_item","payload":{"type":"custom_tool_call_output","call_id":"poll","output":output}}), + ); + append( + &transcript, + json!({"timestamp":"2026-01-01T00:00:07Z","type":"event_msg","payload":{"type":"task_complete","turn_id":"t1"}}), + ); + ops.extend(translator.checkpoint(&ctx).unwrap()); + // A repeated checkpoint must neither duplicate results nor lose the failure. + ops.extend(translator.checkpoint(&ctx).unwrap()); + let after = reduce(ops); + let poll = after + .values() + .find(|r| r.metadata.as_ref().is_some_and(|m| m["call_id"] == "poll")) + .unwrap(); + assert_eq!(poll.error.as_deref(), Some("Command exited with code 1")); + assert_eq!(poll.output, Some(output)); + assert_eq!( + poll.metadata.as_ref().unwrap()["wrapper_outcome"], + "completed" + ); + assert_eq!( + poll.metadata.as_ref().unwrap()["command_outcomes"], + json!([{"outcome":"nonzero_exit","exit_code":1}]) + ); + assert_eq!( + after + .values() + .filter(|r| r.span_type == SpanType::Tool) + .count(), + 2 + ); + // Replaying the full durable transcript gives the same outcome and span IDs. + let mut replay = Registry::default_agents().create("codex", "s"); + let replayed = reduce( + replay + .handle(&envelope("s", "SessionStart", path, json!({})), &ctx) + .unwrap(), + ); + let replayed_poll = &replayed[&poll.span_id]; + assert_eq!(replayed_poll.error, poll.error); + assert_eq!(replayed_poll.metadata, poll.metadata); +} + #[test] fn codex_mcp_error_results_keep_the_content_text() { let tmp = tempfile::tempdir().unwrap(); @@ -1370,6 +1544,10 @@ fn missing_tool_output_is_an_error() { tool.metadata.as_ref().unwrap()["tool_approval"], json!("approved") ); + assert!( + tool.metadata.as_ref().unwrap().get("outcome").is_none(), + "missing output does not establish cancellation" + ); } #[test] From 0513faf5a7ac59a5d2eeabb7f3a32a6153bebb31 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 28 Oct 2026 13:01:18 -0400 Subject: [PATCH 2/3] fix(codex): make command outcome checks portable in ci MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit > 🤖 **This text was AI-generated.** The Codex smoke test now checks actual command exits, exposing sandbox startup failures on Linux and policy rejection on Windows: ```text read-only sandbox -> shell blocked -> expected exit never observed ``` Run fixed mock-inference commands without the agent OS sandbox, while keeping live-inference runs read-only: ```text mock inference -> fixed command -> exit 0 or 7 -> trace assertion ``` Also recognize the native Output: header seen in CI, so stdout cannot supply an exit status when the command header omits one. Refs: SDK-708 --- bt-daemon/src/translate/codex/tool_output.rs | 10 +++++----- bt-daemon/tests/codex_translator.rs | 1 + bt-daemon/tests/support/agents/codex.rs | 9 ++++++++- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/bt-daemon/src/translate/codex/tool_output.rs b/bt-daemon/src/translate/codex/tool_output.rs index f43fc6fb..9928c105 100644 --- a/bt-daemon/src/translate/codex/tool_output.rs +++ b/bt-daemon/src/translate/codex/tool_output.rs @@ -185,12 +185,12 @@ impl ToolOutcome { { return false; } - // Native result metadata ends before Final output. Never search the + // Native result metadata ends before Output/Final output. Never search the // command's printed output for an exit status, including on success. - let header = text - .split_once("\nFinal output:") - .map_or(text, |(header, _)| header); - for line in header.lines() { + for line in text + .lines() + .take_while(|line| !matches!(*line, "Output:" | "Final output:")) + { if let Some(code) = line .strip_prefix("Process exited with code ") .or_else(|| line.strip_prefix("Exit code ")) diff --git a/bt-daemon/tests/codex_translator.rs b/bt-daemon/tests/codex_translator.rs index b52bd358..0e8b5fe9 100644 --- a/bt-daemon/tests/codex_translator.rs +++ b/bt-daemon/tests/codex_translator.rs @@ -1364,6 +1364,7 @@ fn codex_command_result_formats_preserve_outcomes() { ("exec", exec_output(json!({"exit_code":0,"error":null,"output":"{\"exit_code\":1,\"error\":\"example\"}","stderr":"Error: harmless diagnostic"})), None, success.clone()), ("exec_command", json!("Chunk ID: abc\nProcess exited with code 0\nFinal output:\nProcess exited with code 1\n{\"exit_code\":1}"), None, success), ("exec_command", json!("Chunk ID: abc\nFinal output:\n{\"exit_code\":1}"), None, Value::Null), + ("exec_command", json!("Chunk ID: abc\nOutput:\nProcess exited with code 1\n{\"exit_code\":1}"), None, Value::Null), ("read_document", json!({"content":[{"type":"text","text":"{\"exit_code\":1}"}]}), None, Value::Null), ("read_document", json!(r#"{"exit_code":1,"output":"an example"}"#), None, Value::Null), ("exec_command", json!({"status":"cancelled","exit_code":130,"output":"user cancelled"}), None, json!([{"outcome":"interrupted","exit_code":130}])), diff --git a/bt-daemon/tests/support/agents/codex.rs b/bt-daemon/tests/support/agents/codex.rs index c9ffdd5b..f0d4261c 100644 --- a/bt-daemon/tests/support/agents/codex.rs +++ b/bt-daemon/tests/support/agents/codex.rs @@ -112,13 +112,20 @@ impl CodexAgent { fn command(&self, world: &AgentTestWorld) -> Command { let mut command = command_from_env("CODEX_BIN", "codex"); + // Mock inference emits fixed test commands. CI host sandbox support + // must not prevent those commands from reaching their expected exits. + let sandbox = if world.uses_mock_inference() { + "danger-full-access" + } else { + "read-only" + }; command .args([ "exec", "--skip-git-repo-check", "--dangerously-bypass-hook-trust", "--sandbox", - "read-only", + sandbox, "-c", r#"approval_policy="never""#, ]) From 9a542ce77f1c960f053eb676bc47f57f1bf44dc0 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 28 Oct 2026 13:19:49 -0400 Subject: [PATCH 3/3] fix(codex): retain rejected allsettled tool results MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit > 🤖 **This text was AI-generated.** A completed code-mode wrapper could hide a failed tool promise because Promise.allSettled reports rejection through status and reason: ```text {status: rejected, reason: {message: "command launch failed"}} -> span.error: null ``` Recognize rejected entries, extract the reason with the shared error helper, and retain the failure alongside other observed command outcomes: ```text span.error: command launch failed metadata.command_outcomes: [{outcome: rejected}] ``` Use a clear fallback when the reason has no message. Preserve raw output and leave the command exit code unknown when it was not reported. Refs: SDK-708 --- bt-daemon/src/translate/codex/tool_output.rs | 17 ++++++++++++----- bt-daemon/tests/codex_translator.rs | 7 +++++-- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/bt-daemon/src/translate/codex/tool_output.rs b/bt-daemon/src/translate/codex/tool_output.rs index 9928c105..20c3d197 100644 --- a/bt-daemon/src/translate/codex/tool_output.rs +++ b/bt-daemon/src/translate/codex/tool_output.rs @@ -86,13 +86,19 @@ impl ToolOutcome { } match value { Value::Object(object) => { + let status = object.get("status").and_then(Value::as_str); let interrupted = matches!( - object.get("status").and_then(Value::as_str), + status, Some("cancelled" | "canceled" | "interrupted" | "aborted") ); if !interrupted && self.error.is_none() { self.error = native_error(value); } + if status == Some("rejected") { + // allSettled reports a failed promise, not a process exit. + self.commands.push(json!({"outcome":"rejected"})); + return; + } let code = exit_code(value); let session_id = object .get("session_id") @@ -233,12 +239,13 @@ fn has_error_marker(value: &Value) -> bool { fn native_error(output: &Value) -> Option { if let Some(object) = output.as_object() { + let status = object.get("status").and_then(Value::as_str); + if status == Some("rejected") { + return Some(error_text(object.get("reason"), "Tool promise rejected")); + } if object.get("is_error").and_then(Value::as_bool) == Some(true) || object.get("isError").and_then(Value::as_bool) == Some(true) - || matches!( - object.get("status").and_then(Value::as_str), - Some("error" | "failed") - ) + || matches!(status, Some("error" | "failed")) { return Some(error_text(Some(output), "Tool execution failed")); } diff --git a/bt-daemon/tests/codex_translator.rs b/bt-daemon/tests/codex_translator.rs index 0e8b5fe9..15d5aec5 100644 --- a/bt-daemon/tests/codex_translator.rs +++ b/bt-daemon/tests/codex_translator.rs @@ -1348,9 +1348,10 @@ fn codex_command_result_formats_preserve_outcomes() { let success = json!([{"outcome":"success","exit_code":0}]); let mut multiple = exec_output(Value::Null); multiple[1]["text"] = json!(format!( - "{}\n{}", + "{}\n{}\n{}", json!({"status":"fulfilled","value":{"exit_code":2,"output":"no matches"}}), json!({"result":{"exit_code":0,"output":"ok"}}), + json!({"status":"rejected","reason":"command launch failed"}), )); // Each case covers a different native envelope or a false-positive boundary. // PR #71's test above already covers null, blank, and false error members. @@ -1360,7 +1361,9 @@ fn codex_command_result_formats_preserve_outcomes() { ("exec_command", json!("Chunk ID: abc\nWall time: 0.1 seconds\nProcess exited with code 2\nFinal output:\nbad command"), Some("Command exited with code 2"), failed.clone()), ("exec_command", json!("Exit code 2\nbad command"), Some("Command exited with code 2"), failed.clone()), ("exec", json!("Script completed\nWall time 0.1 seconds\nOutput:\n{\"exit_code\":2,\"output\":\"bad command\"}"), Some("Command exited with code 2"), failed), - ("exec", multiple, Some("Command exited with code 2"), json!([{"outcome":"nonzero_exit","exit_code":2},{"outcome":"success","exit_code":0}])), + ("exec", multiple, Some("Command exited with code 2"), json!([{"outcome":"nonzero_exit","exit_code":2},{"outcome":"success","exit_code":0},{"outcome":"rejected"}])), + ("exec", exec_output(json!([{"status":"rejected","reason":{"message":"command launch failed"}},{"status":"fulfilled","value":{"exit_code":0,"output":"ok"}}])), Some("command launch failed"), json!([{"outcome":"rejected"},{"outcome":"success","exit_code":0}])), + ("exec", exec_output(json!({"status":"rejected","reason":{}})), Some("Tool promise rejected"), json!([{"outcome":"rejected"}])), ("exec", exec_output(json!({"exit_code":0,"error":null,"output":"{\"exit_code\":1,\"error\":\"example\"}","stderr":"Error: harmless diagnostic"})), None, success.clone()), ("exec_command", json!("Chunk ID: abc\nProcess exited with code 0\nFinal output:\nProcess exited with code 1\n{\"exit_code\":1}"), None, success), ("exec_command", json!("Chunk ID: abc\nFinal output:\n{\"exit_code\":1}"), None, Value::Null),