fix(tracing): stop marking successful tool calls as errors

bt-daemon/src/translate/claude.rs

@@ -949,16 +949,7 @@ impl AgentTranslator for ClaudeTranslator {
             "PostToolUseFailure" => self.finish_tool(
                 event,
                 ToolApproval::Approved,
-                tool_error(&event.payload)
-                    .or_else(|| {
-                        event
-                            .payload
-                            .pointer("/tool_response/output")
-                            .and_then(Value::as_str)
-                            .filter(|value| !value.is_empty())
-                            .map(str::to_owned)
-                    })
-                    .or_else(|| Some("Tool execution failed".into())),
+                tool_failure_text(&event.payload).or_else(|| Some("Tool execution failed".into())),
                 &mut ops,
             ),
             "PermissionDenied" => self.finish_tool(event, ToolApproval::Denied, None, &mut ops),
@@ -988,7 +979,8 @@ impl AgentTranslator for ClaudeTranslator {
             "Stop" => self.stop_turn(event, None, &mut ops),
             "StopFailure" => self.stop_turn(
                 event,
-                tool_error(&event.payload).or_else(|| Some("Claude Code turn failed".into())),
+                tool_failure_text(&event.payload)
+                    .or_else(|| Some("Claude Code turn failed".into())),
                 &mut ops,
             ),
             "SessionEnd" => self.end_session(event, &mut ops),
@@ -1800,51 +1792,48 @@ fn tool_span_name(tool: &str, input: &Value) -> String {
 }
 
 fn tool_error(payload: &Value) -> Option<String> {
-    for value in [
+    let response = payload.get("tool_response");
+    let explicit_error = [
         payload.get("error"),
-        payload.get("message"),
-        payload.pointer("/tool_response/error"),
-        payload.pointer("/tool_response/stderr"),
-        payload.pointer("/tool_response/message"),
+        response.and_then(|response| response.get("error")),
     ]
     .into_iter()
     .flatten()
-    {
-        if let Some(text) = nonempty_error_text(value) {
-            return Some(text);
-        }
+    .find_map(nonempty_error_text);
+    if let Some(error) = explicit_error {
+        return Some(error);
     }
-    let response = payload.get("tool_response")?;
-    let failed = response
-        .get("interrupted")
-        .and_then(Value::as_bool)
-        .unwrap_or(false)
-        || response
-            .get("is_error")
-            .or_else(|| response.get("isError"))
+    let failed = response.is_some_and(|response| {
+        response
+            .get("interrupted")
             .and_then(Value::as_bool)
             .unwrap_or(false)
-        || response
-            .get("status")
-            .and_then(Value::as_str)
-            .is_some_and(|value| matches!(value, "error" | "failed"));
-    if !failed {
-        return None;
-    }
-    for value in [
-        response.get("error"),
-        response.get("stderr"),
-        response.get("message"),
-        response.get("output"),
+            || response
+                .get("is_error")
+                .or_else(|| response.get("isError"))
+                .and_then(Value::as_bool)
+                .unwrap_or(false)
+            || response
+                .get("status")
+                .and_then(Value::as_str)
+                .is_some_and(|value| matches!(value, "error" | "failed"))
+    });
+    failed
+        .then(|| tool_failure_text(payload).unwrap_or_else(|| "Tool execution failed".to_string()))
+}
+
+fn tool_failure_text(payload: &Value) -> Option<String> {
+    [
+        payload.get("error"),
+        payload.get("message"),
+        payload.pointer("/tool_response/error"),
+        payload.pointer("/tool_response/stderr"),
+        payload.pointer("/tool_response/message"),
+        payload.pointer("/tool_response/output"),
     ]
     .into_iter()
     .flatten()
-    {
-        if let Some(text) = nonempty_error_text(value) {
-            return Some(text);
-        }
-    }
-    Some("Tool execution failed".to_string())
+    .find_map(nonempty_error_text)
 }
 
 #[cfg(test)]

bt-daemon/src/translate/codex.rs

@@ -19,7 +19,7 @@
 
 use super::git::GitMetadataCache;
 use super::recent::{RecentMap, RecentSet};
-use super::tool::{error_text, tool_approval_metadata, ToolApproval};
+use super::tool::{error_text, nonempty_error_text, tool_approval_metadata, ToolApproval};
 use super::{
     local_username, AgentTranslator, SessionCtx, SpanOp, SpanRow, SpanType, TranslatorFactory,
 };
@@ -1465,6 +1465,17 @@ fn args_object(args: Option<&Value>) -> Option<Map<String, Value>> {
     }
 }
 
+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<String> {
     if let Some(object) = output.as_object() {
         if object.get("is_error").and_then(Value::as_bool) == Some(true)
@@ -1476,7 +1487,7 @@ fn classify_tool_output(output: &Value) -> Option<String> {
         {
             return Some(error_text(Some(output), "Tool execution failed"));
         }
-        if let Some(error) = object.get("error") {
+        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

bt-daemon/src/translate/tool.rs

@@ -52,27 +52,17 @@ pub fn error_text(value: Option<&Value>, fallback: &str) -> String {
 /// Extracts a concise message when the source has already identified a value
 /// as an error. Callers must not use this to infer failure from normal output.
 pub fn nonempty_error_text(value: &Value) -> Option<String> {
-    if let Some(text) = value.as_str().filter(|text| !text.is_empty()) {
-        return Some(text.lines().next().unwrap_or(text).to_string());
+    if let Some(text) = value.as_str() {
+        return text
+            .lines()
+            .map(str::trim)
+            .find(|line| !line.is_empty())
+            .map(str::to_string);
     }
     let object = value.as_object()?;
     for key in ["error", "message", "stderr", "output", "result"] {
-        let Some(candidate) = object.get(key) else {
-            continue;
-        };
-        if let Some(text) = candidate.as_str().filter(|text| !text.is_empty()) {
-            return Some(text.lines().next().unwrap_or(text).to_string());
-        }
-        if let Some(nested) = candidate.as_object() {
-            for key in ["error", "message"] {
-                if let Some(text) = nested
-                    .get(key)
-                    .and_then(Value::as_str)
-                    .filter(|text| !text.is_empty())
-                {
-                    return Some(text.lines().next().unwrap_or(text).to_string());
-                }
-            }
+        if let Some(text) = object.get(key).and_then(nonempty_error_text) {
+            return Some(text);
         }
     }
     None
@@ -101,5 +91,9 @@ mod tests {
             "disk full"
         );
         assert_eq!(error_text(None, "fallback"), "fallback");
+        assert_eq!(
+            nonempty_error_text(&json!("\n  disk full\ntrace")),
+            Some("disk full".into())
+        );
     }
 }

bt-daemon/tests/claude_translator.rs

@@ -10,6 +10,22 @@ fn fixture(name: &str) -> PathBuf {
         .join(name)
 }
 
+fn claude_event(session_id: &str, event: &str, ts_ms: i64, payload: Value) -> Envelope {
+    Envelope {
+        source: "claude-code".into(),
+        source_version: None,
+        plugin_version: None,
+        session_id: session_id.into(),
+        event: event.into(),
+        ts_ms,
+        managed_run_id: None,
+        payload,
+        route: None,
+        config: None,
+        capture: None,
+    }
+}
+
 /// How a replayed event points at its transcript bytes.
 #[derive(Clone, Copy, PartialEq)]
 enum Source {
@@ -496,19 +512,7 @@ fn claude_permission_denied_and_failed_tools_are_first_class_spans() {
         session_id: "s".into(),
         config: None,
     };
-    let event = |name: &str, payload: Value| Envelope {
-        source: "claude-code".into(),
-        source_version: None,
-        plugin_version: None,
-        session_id: "s".into(),
-        event: name.into(),
-        ts_ms: 1,
-        managed_run_id: None,
-        payload,
-        route: None,
-        config: None,
-        capture: None,
-    };
+    let event = |name: &str, payload: Value| claude_event("s", name, 1, payload);
     let mut ops = translator
         .handle(
             &event(
@@ -540,7 +544,7 @@ fn claude_permission_denied_and_failed_tools_are_first_class_spans() {
             .handle(
                 &event(
                     "PostToolUse",
-                    json!({"session_id":"s","tool_name":"Write","tool_use_id":"c","tool_input":{"file_path":"x"},"tool_response":{"is_error":true,"stderr":"disk full"}}),
+                    json!({"session_id":"s","tool_name":"Write","tool_use_id":"c","tool_input":{"file_path":"x"},"tool_response":{"is_error":true,"stderr":"\ndisk full"}}),
                 ),
                 &ctx,
             )
@@ -592,6 +596,78 @@ fn claude_permission_denied_and_failed_tools_are_first_class_spans() {
     }));
 }
 
+#[test]
+fn claude_successful_tool_outputs_do_not_populate_error() {
+    let registry = Registry::default_agents();
+    let mut translator = registry.create("claude-code", "successful-tools");
+    let ctx = SessionCtx {
+        session_id: "successful-tools".into(),
+        config: None,
+    };
+    let event = |name: &str, ts_ms: i64, payload: Value| {
+        claude_event("successful-tools", name, ts_ms, payload)
+    };
+    let mut ops = translator
+        .handle(
+            &event(
+                "UserPromptSubmit",
+                10,
+                json!({"session_id":"successful-tools","prompt":"run tools"}),
+            ),
+            &ctx,
+        )
+        .unwrap();
+    for envelope in [
+        event(
+            "PostToolUse",
+            20,
+            json!({
+                "session_id":"successful-tools",
+                "tool_name":"Bash",
+                "tool_use_id":"bash-1",
+                "tool_input":{"command":"pwd"},
+                "tool_response":{
+                    "stdout":"/tmp",
+                    "stderr":"\nShell cwd was reset to /tmp",
+                    "interrupted":false
+                }
+            }),
+        ),
+        event(
+            "PostToolUse",
+            30,
+            json!({
+                "session_id":"successful-tools",
+                "tool_name":"TaskStop",
+                "tool_use_id":"stop-1",
+                "tool_input":{"task_id":"abc"},
+                "tool_response":{
+                    "task_id":"abc",
+                    "message":"Successfully stopped task: abc"
+                }
+            }),
+        ),
+    ] {
+        ops.extend(translator.handle(&envelope, &ctx).unwrap());
+    }
+
+    let rows = reduce(ops);
+    let tools = rows
+        .values()
+        .filter(|row| row.span_type == SpanType::Tool)
+        .collect::<Vec<_>>();
+    assert_eq!(tools.len(), 2);
+    assert!(tools.iter().all(|row| row.error.is_none()));
+    assert!(tools.iter().any(|row| {
+        row.metadata.as_ref().unwrap()["tool_name"] == json!("Bash")
+            && row.output.as_ref().unwrap()["stdout"] == json!("/tmp")
+    }));
+    assert!(tools.iter().any(|row| {
+        row.metadata.as_ref().unwrap()["tool_name"] == json!("TaskStop")
+            && row.output.as_ref().unwrap()["message"] == json!("Successfully stopped task: abc")
+    }));
+}
+
 #[test]
 fn claude_pairs_tool_lifecycle_and_marks_explicit_skills_and_stop_failures() {
     let registry = Registry::default_agents();
@@ -600,18 +676,10 @@ fn claude_pairs_tool_lifecycle_and_marks_explicit_skills_and_stop_failures() {
         session_id: "lifecycle".into(),
         config: None,
     };
-    let event = |name: &str, ts_ms: i64, payload: Value| Envelope {
-        source: "claude-code".into(),
-        source_version: Some("2.0.0".into()),
-        plugin_version: None,
-        session_id: "lifecycle".into(),
-        event: name.into(),
-        ts_ms,
-        managed_run_id: None,
-        payload,
-        route: None,
-        config: None,
-        capture: None,
+    let event = |name: &str, ts_ms: i64, payload: Value| {
+        let mut event = claude_event("lifecycle", name, ts_ms, payload);
+        event.source_version = Some("2.0.0".into());
+        event
     };
     let mut ops = Vec::new();
     for envelope in [
@@ -633,7 +701,7 @@ fn claude_pairs_tool_lifecycle_and_marks_explicit_skills_and_stop_failures() {
         event(
             "StopFailure",
             40,
-            json!({"session_id":"lifecycle","error":"model process exited"}),
+            json!({"session_id":"lifecycle","message":"model process exited"}),
         ),
     ] {
         ops.extend(translator.handle(&envelope, &ctx).unwrap());