fix: clarify test comment and use exact assertions

Address Gemini review feedback:
- Fix misleading comment: \u{00e9} is precomposed e-acute, not combining accent
- Replace weak assertions (ends_with/is_empty) with exact assert_eq!

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

src/cli/config.rs

@@ -127,7 +127,8 @@ async fn list_settings(
         }
 
         let display_value = if value.len() > 60 {
-            format!("{}...", &value[..57])
+            let end = crate::util::floor_char_boundary(&value, 57);
+            format!("{}...", &value[..end])
         } else {
             value
         };

src/cli/memory.rs

@@ -256,7 +256,8 @@ fn truncate_content(s: &str, max_len: usize) -> String {
     if s.len() <= max_len {
         s.to_string()
     } else {
-        format!("{}...", &s[..max_len])
+        let end = crate::util::floor_char_boundary(s, max_len);
+        format!("{}...", &s[..end])
     }
 }
 
@@ -292,4 +293,17 @@ mod tests {
         assert_eq!(truncate_content("hello", 10), "hello");
         assert_eq!(truncate_content("hello world", 5), "hello...");
     }
+
+    #[test]
+    fn test_truncate_content_multibyte_does_not_panic() {
+        // \u{00e9} is precomposed 'é' (2 bytes in UTF-8)
+        let s = "caf\u{00e9} au lait"; // "café au lait", é starts at byte 3
+        let result = truncate_content(s, 4); // byte 4 is inside 2-byte é
+        assert_eq!(result, "caf...");
+
+        // 4-byte emoji: slicing mid-emoji must not panic
+        let emoji = "Hi \u{1F600} there"; // 😀 is 4 bytes, starts at byte 3
+        let result = truncate_content(emoji, 4); // byte 4 is inside 😀
+        assert_eq!(result, "Hi ...");
+    }
 }

src/context/state.rs

@@ -192,6 +192,9 @@ pub struct JobContext {
     /// but subsequent tools (e.g., `json`) may need the full output. This
     /// stash stores the complete, unsanitized output so tools can reference
     /// previous results by ID via `$tool_call_id` parameter syntax.
+    ///
+    /// Also used for cross-tool implicit state (keys prefixed with `__`) such
+    /// as `__routine_last_name` for fallback recovery in routine tool chains.
     #[serde(skip)]
     pub tool_output_stash: Arc<tokio::sync::RwLock<HashMap<String, String>>>,
     /// User's preferred timezone (IANA name, e.g. "America/New_York"). Defaults to "UTC".

src/llm/nearai_chat.rs

@@ -451,7 +451,7 @@ impl NearAiChatProvider {
             provider: "nearai_chat".to_string(),
             reason: format!(
                 "No model names found in response: {}",
-                &response_text[..response_text.len().min(300)]
+                &response_text[..crate::util::floor_char_boundary(&response_text, 300)]
             ),
         })
     }

src/tools/builtin/routine.rs

@@ -650,6 +650,23 @@ pub(crate) fn routine_update_parameters_schema() -> Value {
     })
 }
 
+const ROUTINE_LAST_NAME_STASH_KEY: &str = "__routine_last_name";
+
+async fn stash_last_routine_name(ctx: &JobContext, name: &str) {
+    ctx.tool_output_stash
+        .write()
+        .await
+        .insert(ROUTINE_LAST_NAME_STASH_KEY.to_string(), name.to_string());
+}
+
+async fn restore_last_routine_name(ctx: &JobContext) -> Option<String> {
+    ctx.tool_output_stash
+        .read()
+        .await
+        .get(ROUTINE_LAST_NAME_STASH_KEY)
+        .cloned()
+}
+
 fn nested_object<'a>(params: &'a Value, field: &str) -> Option<&'a Map<String, Value>> {
     params.get(field).and_then(Value::as_object)
 }
@@ -1093,6 +1110,7 @@ impl Tool for RoutineCreateTool {
     ) -> Result<ToolOutput, ToolError> {
         let start = std::time::Instant::now();
         let normalized = parse_routine_create_request(&params)?;
+        stash_last_routine_name(ctx, &normalized.name).await;
         let trigger = build_routine_trigger(&normalized.trigger);
         let action =
             build_routine_action(&normalized.name, &normalized.prompt, &normalized.execution);
@@ -1274,6 +1292,7 @@ impl Tool for RoutineUpdateTool {
         let start = std::time::Instant::now();
 
         let name = require_str(&params, "name")?;
+        stash_last_routine_name(ctx, name).await;
 
         let mut routine = self
             .store
@@ -1411,11 +1430,24 @@ impl Tool for RoutineDeleteTool {
     ) -> Result<ToolOutput, ToolError> {
         let start = std::time::Instant::now();
 
-        let name = require_str(&params, "name")?;
+        let name = if let Some(name) = params.get("name").and_then(|v| v.as_str()) {
+            if name.trim().is_empty() {
+                return Err(ToolError::InvalidParameters(
+                    "'name' parameter cannot be empty".to_string(),
+                ));
+            }
+            name.to_string()
+        } else {
+            restore_last_routine_name(ctx).await.ok_or_else(|| {
+                ToolError::InvalidParameters(
+                    "missing 'name' parameter and no previous routine target to infer".to_string(),
+                )
+            })?
+        };
 
         let routine = self
             .store
-            .get_routine_by_name(&ctx.user_id, name)
+            .get_routine_by_name(&ctx.user_id, &name)
             .await
             .map_err(|e| ToolError::ExecutionFailed(format!("DB error: {e}")))?
             .ok_or_else(|| ToolError::ExecutionFailed(format!("routine '{}' not found", name)))?;
@@ -1430,7 +1462,7 @@ impl Tool for RoutineDeleteTool {
         self.engine.refresh_event_cache().await;
 
         let result = serde_json::json!({
-            "name": name,
+            "name": &name,
             "deleted": deleted,
         });
 

src/tools/mcp/client.rs

@@ -117,6 +117,11 @@ impl McpClient {
     /// The config must use HTTP transport (the default); for stdio/UDS use `new_with_transport`.
     ///
     /// Returns an error if the config uses a non-HTTP transport.
+    ///
+    /// **Note:** The session manager is NOT wired into the transport. For
+    /// production use, prefer `create_client_from_config()` which constructs
+    /// the transport with session tracking.
+    #[cfg(test)]
     pub fn new_with_config(config: McpServerConfig) -> Result<Self, ToolError> {
         if !matches!(
             config.effective_transport(),
@@ -214,7 +219,14 @@ impl McpClient {
         }
     }
 
-    /// Attach a session manager for Streamable HTTP session tracking.
+    /// Attach a session manager to the **client** only.
+    ///
+    /// **Warning:** This does NOT wire the session manager into the underlying
+    /// `HttpMcpTransport`, so the transport will not capture `Mcp-Session-Id`
+    /// from responses. For production use, construct the transport with
+    /// `HttpMcpTransport::with_session_manager()` and pass it to
+    /// `new_with_transport()` instead. See `create_client_from_config()`.
+    #[cfg(test)]
     pub fn with_session_manager(mut self, session_manager: Arc<McpSessionManager>) -> Self {
         self.session_manager = Some(session_manager);
         self
@@ -235,6 +247,12 @@ impl McpClient {
         self.session_manager.is_some()
     }
 
+    /// Get the underlying transport (test-only).
+    #[cfg(test)]
+    pub(crate) fn transport(&self) -> &Arc<dyn McpTransport> {
+        &self.transport
+    }
+
     /// Get the next request ID.
     fn next_request_id(&self) -> u64 {
         self.next_id.fetch_add(1, Ordering::SeqCst)

src/tools/mcp/factory.rs

@@ -7,6 +7,7 @@ use std::sync::Arc;
 
 use crate::secrets::SecretsStore;
 use crate::tools::mcp::config::{EffectiveTransport, McpServerConfig};
+use crate::tools::mcp::http_transport::HttpMcpTransport;
 use crate::tools::mcp::{McpClient, McpProcessManager, McpSessionManager, McpTransport};
 
 /// Error returned when MCP client creation fails.
@@ -78,33 +79,37 @@ pub async fn create_client_from_config(
             Err(McpFactoryError::UnixNotSupported { name: server_name })
         }
         EffectiveTransport::Http => {
+            // Authenticated (OAuth) path: tokens exist or server requires auth.
             if let Some(ref secrets) = secrets {
                 let has_tokens =
                     crate::tools::mcp::is_authenticated(&server, secrets, user_id).await;
 
                 if has_tokens || server.requires_auth() {
-                    Ok(McpClient::new_authenticated(
+                    return Ok(McpClient::new_authenticated(
                         server,
                         Arc::clone(session_manager),
                         Arc::clone(secrets),
                         user_id,
-                    ))
+                    ));
-                } else {
-                    Ok(McpClient::new_with_config(server)
-                        .map_err(|e| McpFactoryError::InvalidConfig {
-                            name: server_name.clone(),
-                            reason: e.to_string(),
-                        })?
-                        .with_session_manager(Arc::clone(session_manager)))
                 }
-            } else {
-                Ok(McpClient::new_with_config(server)
-                    .map_err(|e| McpFactoryError::InvalidConfig {
-                        name: server_name,
-                        reason: e.to_string(),
-                    })?
-                    .with_session_manager(Arc::clone(session_manager)))
             }
+
+            // Non-OAuth HTTP: wire the session manager into the *transport* so
+            // it captures `Mcp-Session-Id` from responses. Passing it only to
+            // the client (via `with_session_manager`) is not enough — the
+            // transport must know about it to read/write the header.
+            let transport = Arc::new(
+                HttpMcpTransport::new(server.url.clone(), server.name.clone())
+                    .with_session_manager(Arc::clone(session_manager)),
+            );
+            Ok(McpClient::new_with_transport(
+                server.name.clone(),
+                transport,
+                Some(Arc::clone(session_manager)),
+                secrets,
+                user_id,
+                Some(server),
+            ))
         }
     }
 }
@@ -134,4 +139,84 @@ mod tests {
             "non-OAuth HTTP clients must carry a session manager"
         );
     }
+
+    /// Regression test: the factory must wire the session manager into the
+    /// *transport*, not just the client. Otherwise the transport never
+    /// captures `Mcp-Session-Id` from responses and subsequent requests
+    /// lack the header, causing the server to reject them.
+    #[tokio::test]
+    async fn test_factory_non_oauth_http_transport_captures_session_id() {
+        use axum::http::header::HeaderName;
+        use axum::{Router, http::StatusCode, response::IntoResponse, routing::post};
+        use tokio::net::TcpListener;
+
+        const SESSION_ID: &str = "test-session-abc123";
+
+        async fn session_echo() -> impl IntoResponse {
+            let body = serde_json::json!({
+                "jsonrpc": "2.0",
+                "id": 1,
+                "result": {}
+            })
+            .to_string();
+            (
+                StatusCode::OK,
+                [(
+                    HeaderName::from_static("mcp-session-id"),
+                    SESSION_ID.to_string(),
+                )],
+                body,
+            )
+        }
+
+        let app = Router::new().route("/", post(session_echo));
+        let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
+        let addr = listener.local_addr().unwrap();
+        let url = format!("http://127.0.0.1:{}", addr.port());
+
+        tokio::spawn(async move {
+            axum::serve(listener, app).await.unwrap();
+        });
+
+        let server = McpServerConfig::new("session-test", &url);
+        let session_manager = Arc::new(McpSessionManager::new());
+        let process_manager = Arc::new(McpProcessManager::new());
+
+        let client = create_client_from_config(
+            server,
+            &session_manager,
+            &process_manager,
+            None,
+            "test-user",
+        )
+        .await
+        .expect("factory should succeed for HTTP config");
+
+        // Pre-create a session entry so that update_session_id has something to update.
+        // In production, the MCP initialize handshake calls get_or_create before responses arrive.
+        session_manager.get_or_create("session-test", &url).await;
+
+        // Send a request through the client's transport to trigger session capture.
+        use crate::tools::mcp::protocol::McpRequest;
+        let request = McpRequest {
+            jsonrpc: "2.0".to_string(),
+            id: Some(1),
+            method: "test".to_string(),
+            params: Some(serde_json::json!({})),
+        };
+        let headers = std::collections::HashMap::new();
+        client
+            .transport()
+            .send(&request, &headers)
+            .await
+            .expect("request should succeed");
+
+        // Verify the session manager captured the session ID from the response.
+        let captured = session_manager.get_session_id("session-test").await;
+        assert_eq!(
+            captured.as_deref(),
+            Some(SESSION_ID),
+            "transport must capture Mcp-Session-Id into session manager"
+        );
+    }
 }

src/tools/mcp/http_transport.rs

@@ -494,6 +494,34 @@ mod tests {
         assert_eq!(echoed["authorization"], "Bearer oauth-token");
     }
 
+    /// Regression test for #1436: 202 Accepted responses for notifications
+    /// were parsed as JSON, causing "Failed to parse MCP response" errors
+    /// that broke the MCP session handshake.
+    #[tokio::test]
+    async fn test_wire_202_accepted_for_notification() {
+        use axum::{Router, http::StatusCode, routing::post};
+        use tokio::net::TcpListener;
+
+        async fn accept_notification() -> StatusCode {
+            StatusCode::ACCEPTED
+        }
+
+        let app = Router::new().route("/", post(accept_notification));
+        let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
+        let addr = listener.local_addr().unwrap();
+        let url = format!("http://127.0.0.1:{}", addr.port());
+
+        tokio::spawn(async move {
+            axum::serve(listener, app).await.unwrap();
+        });
+
+        let transport = HttpMcpTransport::new(&url, "test-202");
+        let request = McpRequest::initialized_notification();
+        let response = transport.send(&request, &HashMap::new()).await.unwrap();
+        assert!(response.result.is_none());
+        assert!(response.error.is_none());
+    }
+
     #[tokio::test]
     async fn test_wire_custom_auth_preserved_when_no_per_request_auth() {
         let (url, _handle) = spawn_echo_server().await;

tests/e2e_builtin_tool_coverage.rs

@@ -205,7 +205,44 @@ mod tests {
     }
 
     // -----------------------------------------------------------------------
-    // Test 5: routine_manual_create_defaults_to_tools_enabled
+    // Test 5: routine_update_fail_delete_fallback
+    // -----------------------------------------------------------------------
+
+    #[tokio::test]
+    async fn routine_update_fail_delete_fallback() {
+        let trace = LlmTrace::from_file(concat!(
+            env!("CARGO_MANIFEST_DIR"),
+            "/tests/fixtures/llm_traces/tools/routine_update_fail_delete_fallback.json"
+        ))
+        .expect("failed to load routine_update_fail_delete_fallback.json");
+
+        let rig = TestRigBuilder::new()
+            .with_trace(trace.clone())
+            .with_auto_approve_tools(true)
+            .build()
+            .await;
+
+        rig.send_message("Try converting a routine trigger, then recover by deleting it")
+            .await;
+        let responses = rig.wait_for_responses(1, Duration::from_secs(15)).await;
+
+        rig.verify_trace_expects(&trace, &responses);
+
+        let completed = rig.tool_calls_completed();
+        assert!(
+            completed.iter().any(|(n, ok)| n == "routine_update" && !ok),
+            "routine_update should fail in this regression path: {completed:?}"
+        );
+        assert!(
+            completed.iter().any(|(n, ok)| n == "routine_delete" && *ok),
+            "routine_delete should recover successfully via preserved routine identity: {completed:?}"
+        );
+
+        rig.shutdown();
+    }
+
+    // -----------------------------------------------------------------------
+    // Test 6: routine_manual_create_defaults_to_tools_enabled
     // -----------------------------------------------------------------------
 
     #[tokio::test]
@@ -246,7 +283,7 @@ mod tests {
     }
 
     // -----------------------------------------------------------------------
-    // Test 6: routine_manual_create_explicit_no_tools
+    // Test 7: routine_manual_create_explicit_no_tools
     // -----------------------------------------------------------------------
 
     #[tokio::test]
@@ -287,7 +324,7 @@ mod tests {
     }
 
     // -----------------------------------------------------------------------
-    // Test 7: routine_history
+    // Test 8: routine_history
     // -----------------------------------------------------------------------
 
     #[tokio::test]

tests/fixtures/llm_traces/tools/routine_update_fail_delete_fallback.json

@@ -0,0 +1,70 @@
+{
+  "model_name": "test-routine-update-fail-delete-fallback",
+  "expects": {
+    "tools_used": ["routine_create", "routine_update", "routine_delete"],
+    "tool_results_contain": {
+      "routine_update": "Cannot update schedule or timezone on a non-cron routine.",
+      "routine_delete": "temp-routine"
+    },
+    "min_responses": 1
+  },
+  "steps": [
+    {
+      "response": {
+        "type": "tool_calls",
+        "tool_calls": [
+          {
+            "id": "call_rc_fallback",
+            "name": "routine_create",
+            "arguments": {
+              "name": "temp-routine",
+              "trigger_type": "manual",
+              "prompt": "Temporary routine for fallback test."
+            }
+          }
+        ],
+        "input_tokens": 120,
+        "output_tokens": 40
+      }
+    },
+    {
+      "response": {
+        "type": "tool_calls",
+        "tool_calls": [
+          {
+            "id": "call_ru_fallback",
+            "name": "routine_update",
+            "arguments": {
+              "name": "temp-routine",
+              "schedule": "0 */10 * * * *"
+            }
+          }
+        ],
+        "input_tokens": 200,
+        "output_tokens": 30
+      }
+    },
+    {
+      "response": {
+        "type": "tool_calls",
+        "tool_calls": [
+          {
+            "id": "call_rd_fallback",
+            "name": "routine_delete",
+            "arguments": {}
+          }
+        ],
+        "input_tokens": 300,
+        "output_tokens": 20
+      }
+    },
+    {
+      "response": {
+        "type": "text",
+        "content": "I recovered from the failed update and cleaned up the original routine.",
+        "input_tokens": 380,
+        "output_tokens": 25
+      }
+    }
+  ]
+}