Merge pull request #865 from agorevski/feat/systematic-test-coverage-852

test: add systematic test coverage for 7 bug pattern groups (#852)

src/agent/loop_.rs

@@ -2737,4 +2737,185 @@ browser_open/url>https://example.com"#;
         assert_eq!(calls[0].arguments["command"], "pwd");
         assert_eq!(text, "Done");
     }
+
+    // ─────────────────────────────────────────────────────────────────────
+    // TG4 (inline): parse_tool_calls robustness — malformed/edge-case inputs
+    // Prevents: Pattern 4 issues #746, #418, #777, #848
+    // ─────────────────────────────────────────────────────────────────────
+
+    #[test]
+    fn parse_tool_calls_empty_input_returns_empty() {
+        let (text, calls) = parse_tool_calls("");
+        assert!(calls.is_empty(), "empty input should produce no tool calls");
+        assert!(text.is_empty(), "empty input should produce no text");
+    }
+
+    #[test]
+    fn parse_tool_calls_whitespace_only_returns_empty_calls() {
+        let (text, calls) = parse_tool_calls("   \n\t  ");
+        assert!(calls.is_empty());
+        assert!(text.is_empty() || text.trim().is_empty());
+    }
+
+    #[test]
+    fn parse_tool_calls_nested_xml_tags_handled() {
+        // Double-wrapped tool call should still parse the inner call
+        let response = r#"<tool_call><tool_call>{"name":"echo","arguments":{"msg":"hi"}}</tool_call></tool_call>"#;
+        let (_text, calls) = parse_tool_calls(response);
+        // Should find at least one tool call
+        assert!(
+            !calls.is_empty(),
+            "nested XML tags should still yield at least one tool call"
+        );
+    }
+
+    #[test]
+    fn parse_tool_calls_truncated_json_no_panic() {
+        // Incomplete JSON inside tool_call tags
+        let response = r#"<tool_call>{"name":"shell","arguments":{"command":"ls"</tool_call>"#;
+        let (_text, _calls) = parse_tool_calls(response);
+        // Should not panic — graceful handling of truncated JSON
+    }
+
+    #[test]
+    fn parse_tool_calls_empty_json_object_in_tag() {
+        let response = "<tool_call>{}</tool_call>";
+        let (_text, calls) = parse_tool_calls(response);
+        // Empty JSON object has no name field — should not produce valid tool call
+        assert!(
+            calls.is_empty(),
+            "empty JSON object should not produce a tool call"
+        );
+    }
+
+    #[test]
+    fn parse_tool_calls_closing_tag_only_returns_text() {
+        let response = "Some text </tool_call> more text";
+        let (text, calls) = parse_tool_calls(response);
+        assert!(calls.is_empty(), "closing tag only should not produce calls");
+        assert!(
+            !text.is_empty(),
+            "text around orphaned closing tag should be preserved"
+        );
+    }
+
+    #[test]
+    fn parse_tool_calls_very_large_arguments_no_panic() {
+        let large_arg = "x".repeat(100_000);
+        let response = format!(
+            r#"<tool_call>{{"name":"echo","arguments":{{"message":"{}"}}}}</tool_call>"#,
+            large_arg
+        );
+        let (_text, calls) = parse_tool_calls(&response);
+        assert_eq!(calls.len(), 1, "large arguments should still parse");
+        assert_eq!(calls[0].name, "echo");
+    }
+
+    #[test]
+    fn parse_tool_calls_special_characters_in_arguments() {
+        let response = r#"<tool_call>{"name":"echo","arguments":{"message":"hello \"world\" <>&'\n\t"}}</tool_call>"#;
+        let (_text, calls) = parse_tool_calls(response);
+        assert_eq!(calls.len(), 1);
+        assert_eq!(calls[0].name, "echo");
+    }
+
+    #[test]
+    fn parse_tool_calls_text_with_embedded_json_not_extracted() {
+        // Raw JSON without any tags should NOT be extracted as a tool call
+        let response = r#"Here is some data: {"name":"echo","arguments":{"message":"hi"}} end."#;
+        let (_text, calls) = parse_tool_calls(response);
+        assert!(
+            calls.is_empty(),
+            "raw JSON in text without tags should not be extracted"
+        );
+    }
+
+    #[test]
+    fn parse_tool_calls_multiple_formats_mixed() {
+        // Mix of text and properly tagged tool call
+        let response = r#"I'll help you with that.
+
+<tool_call>
+{"name":"shell","arguments":{"command":"echo hello"}}
+</tool_call>
+
+Let me check the result."#;
+        let (text, calls) = parse_tool_calls(response);
+        assert_eq!(calls.len(), 1, "should extract one tool call from mixed content");
+        assert_eq!(calls[0].name, "shell");
+        assert!(
+            text.contains("help you"),
+            "text before tool call should be preserved"
+        );
+    }
+
+    // ─────────────────────────────────────────────────────────────────────
+    // TG4 (inline): scrub_credentials edge cases
+    // ─────────────────────────────────────────────────────────────────────
+
+    #[test]
+    fn scrub_credentials_empty_input() {
+        let result = scrub_credentials("");
+        assert_eq!(result, "");
+    }
+
+    #[test]
+    fn scrub_credentials_no_sensitive_data() {
+        let input = "normal text without any secrets";
+        let result = scrub_credentials(input);
+        assert_eq!(result, input, "non-sensitive text should pass through unchanged");
+    }
+
+    #[test]
+    fn scrub_credentials_short_values_not_redacted() {
+        // Values shorter than 8 chars should not be redacted
+        let input = r#"api_key="short""#;
+        let result = scrub_credentials(input);
+        assert_eq!(result, input, "short values should not be redacted");
+    }
+
+    // ─────────────────────────────────────────────────────────────────────
+    // TG4 (inline): trim_history edge cases
+    // ─────────────────────────────────────────────────────────────────────
+
+    #[test]
+    fn trim_history_empty_history() {
+        let mut history: Vec<crate::providers::ChatMessage> = vec![];
+        trim_history(&mut history, 10);
+        assert!(history.is_empty());
+    }
+
+    #[test]
+    fn trim_history_system_only() {
+        let mut history = vec![crate::providers::ChatMessage::system("system prompt")];
+        trim_history(&mut history, 10);
+        assert_eq!(history.len(), 1);
+        assert_eq!(history[0].role, "system");
+    }
+
+    #[test]
+    fn trim_history_exactly_at_limit() {
+        let mut history = vec![
+            crate::providers::ChatMessage::system("system"),
+            crate::providers::ChatMessage::user("msg 1"),
+            crate::providers::ChatMessage::assistant("reply 1"),
+        ];
+        trim_history(&mut history, 2); // 2 non-system messages = exactly at limit
+        assert_eq!(history.len(), 3, "should not trim when exactly at limit");
+    }
+
+    #[test]
+    fn trim_history_removes_oldest_non_system() {
+        let mut history = vec![
+            crate::providers::ChatMessage::system("system"),
+            crate::providers::ChatMessage::user("old msg"),
+            crate::providers::ChatMessage::assistant("old reply"),
+            crate::providers::ChatMessage::user("new msg"),
+            crate::providers::ChatMessage::assistant("new reply"),
+        ];
+        trim_history(&mut history, 2);
+        assert_eq!(history.len(), 3); // system + 2 kept
+        assert_eq!(history[0].role, "system");
+        assert_eq!(history[1].content, "new msg");
+    }
 }

src/channels/discord.rs

@@ -843,4 +843,110 @@ mod tests {
         // Should have UUID dashes
         assert!(id.contains('-'));
     }
+
+    // ─────────────────────────────────────────────────────────────────────
+    // TG6: Channel platform limit edge cases for Discord (2000 char limit)
+    // Prevents: Pattern 6 — issues #574, #499
+    // ─────────────────────────────────────────────────────────────────────
+
+    #[test]
+    fn split_message_code_block_at_boundary() {
+        // Code block that spans the split boundary
+        let mut msg = String::new();
+        msg.push_str("```rust\n");
+        msg.push_str(&"x".repeat(1990));
+        msg.push_str("\n```\nMore text after code block");
+        let parts = split_message_for_discord(&msg);
+        assert!(parts.len() >= 2, "code block spanning boundary should split");
+        for part in &parts {
+            assert!(
+                part.len() <= DISCORD_MAX_MESSAGE_LENGTH,
+                "each part must be <= {DISCORD_MAX_MESSAGE_LENGTH}, got {}",
+                part.len()
+            );
+        }
+    }
+
+    #[test]
+    fn split_message_single_long_word_exceeds_limit() {
+        // A single word longer than 2000 chars must be hard-split
+        let long_word = "a".repeat(2500);
+        let parts = split_message_for_discord(&long_word);
+        assert!(parts.len() >= 2, "word exceeding limit must be split");
+        for part in &parts {
+            assert!(
+                part.len() <= DISCORD_MAX_MESSAGE_LENGTH,
+                "hard-split part must be <= {DISCORD_MAX_MESSAGE_LENGTH}, got {}",
+                part.len()
+            );
+        }
+        // Reassembled content should match original
+        let reassembled: String = parts.join("");
+        assert_eq!(reassembled, long_word);
+    }
+
+    #[test]
+    fn split_message_exactly_at_limit_no_split() {
+        let msg = "a".repeat(DISCORD_MAX_MESSAGE_LENGTH);
+        let parts = split_message_for_discord(&msg);
+        assert_eq!(parts.len(), 1, "message exactly at limit should not split");
+        assert_eq!(parts[0].len(), DISCORD_MAX_MESSAGE_LENGTH);
+    }
+
+    #[test]
+    fn split_message_one_over_limit_splits() {
+        let msg = "a".repeat(DISCORD_MAX_MESSAGE_LENGTH + 1);
+        let parts = split_message_for_discord(&msg);
+        assert!(parts.len() >= 2, "message 1 char over limit must split");
+    }
+
+    #[test]
+    fn split_message_many_short_lines() {
+        // Many short lines should be batched into chunks under the limit
+        let msg: String = (0..500).map(|i| format!("line {i}\n")).collect();
+        let parts = split_message_for_discord(&msg);
+        for part in &parts {
+            assert!(
+                part.len() <= DISCORD_MAX_MESSAGE_LENGTH,
+                "short-line batch must be <= limit"
+            );
+        }
+        // All content should be preserved
+        let reassembled: String = parts.join("");
+        assert_eq!(reassembled.trim(), msg.trim());
+    }
+
+    #[test]
+    fn split_message_only_whitespace() {
+        let msg = "   \n\n\t  ";
+        let parts = split_message_for_discord(msg);
+        // Should handle gracefully without panic
+        assert!(parts.len() <= 1);
+    }
+
+    #[test]
+    fn split_message_emoji_at_boundary() {
+        // Emoji are multi-byte; ensure we don't split mid-emoji
+        let mut msg = "a".repeat(1998);
+        msg.push_str("🎉🎊"); // 2 emoji at the boundary (2000 chars total)
+        let parts = split_message_for_discord(&msg);
+        for part in &parts {
+            // The function splits on character count, not byte count
+            assert!(
+                part.chars().count() <= DISCORD_MAX_MESSAGE_LENGTH,
+                "emoji boundary split must respect limit"
+            );
+        }
+    }
+
+    #[test]
+    fn split_message_consecutive_newlines_at_boundary() {
+        let mut msg = "a".repeat(1995);
+        msg.push_str("\n\n\n\n\n");
+        msg.push_str(&"b".repeat(100));
+        let parts = split_message_for_discord(&msg);
+        for part in &parts {
+            assert!(part.len() <= DISCORD_MAX_MESSAGE_LENGTH);
+        }
+    }
 }

src/channels/telegram.rs

@@ -18,7 +18,7 @@ const TELEGRAM_BIND_COMMAND: &str = "/bind";
 /// Split a message into chunks that respect Telegram's 4096 character limit.
 /// Tries to split at word boundaries when possible, and handles continuation.
 fn split_message_for_telegram(message: &str) -> Vec<String> {
-    if message.len() <= TELEGRAM_MAX_MESSAGE_LENGTH {
+    if message.chars().count() <= TELEGRAM_MAX_MESSAGE_LENGTH {
         return vec![message.to_string()];
     }
 
@@ -26,29 +26,35 @@ fn split_message_for_telegram(message: &str) -> Vec<String> {
     let mut remaining = message;
 
     while !remaining.is_empty() {
-        let chunk_end = if remaining.len() <= TELEGRAM_MAX_MESSAGE_LENGTH {
-            remaining.len()
+        // Find the byte offset for the Nth character boundary.
+        let hard_split = remaining
+            .char_indices()
+            .nth(TELEGRAM_MAX_MESSAGE_LENGTH)
+            .map_or(remaining.len(), |(idx, _)| idx);
+
+        let chunk_end = if hard_split == remaining.len() {
+            hard_split
         } else {
             // Try to find a good break point (newline, then space)
-            let search_area = &remaining[..TELEGRAM_MAX_MESSAGE_LENGTH];
+            let search_area = &remaining[..hard_split];
 
             // Prefer splitting at newline
             if let Some(pos) = search_area.rfind('\n') {
                 // Don't split if the newline is too close to the start
-                if pos >= TELEGRAM_MAX_MESSAGE_LENGTH / 2 {
+                if search_area[..pos].chars().count() >= TELEGRAM_MAX_MESSAGE_LENGTH / 2 {
                     pos + 1
                 } else {
                     // Try space as fallback
                     search_area
                         .rfind(' ')
-                        .unwrap_or(TELEGRAM_MAX_MESSAGE_LENGTH)
+                        .unwrap_or(hard_split)
                         + 1
                 }
             } else if let Some(pos) = search_area.rfind(' ') {
                 pos + 1
             } else {
-                // Hard split at the limit
-                TELEGRAM_MAX_MESSAGE_LENGTH
+                // Hard split at character boundary
+                hard_split
             }
         };
 
@@ -2825,4 +2831,100 @@ mod tests {
         let ch_disabled = TelegramChannel::new("token".into(), vec!["*".into()], false);
         assert!(!ch_disabled.mention_only);
     }
+
+    // ─────────────────────────────────────────────────────────────────────
+    // TG6: Channel platform limit edge cases for Telegram (4096 char limit)
+    // Prevents: Pattern 6 — issues #574, #499
+    // ─────────────────────────────────────────────────────────────────────
+
+    #[test]
+    fn telegram_split_code_block_at_boundary() {
+        let mut msg = String::new();
+        msg.push_str("```python\n");
+        msg.push_str(&"x".repeat(4085));
+        msg.push_str("\n```\nMore text after code block");
+        let parts = split_message_for_telegram(&msg);
+        assert!(parts.len() >= 2, "code block spanning boundary should split");
+        for part in &parts {
+            assert!(
+                part.len() <= TELEGRAM_MAX_MESSAGE_LENGTH,
+                "each part must be <= {TELEGRAM_MAX_MESSAGE_LENGTH}, got {}",
+                part.len()
+            );
+        }
+    }
+
+    #[test]
+    fn telegram_split_single_long_word() {
+        let long_word = "a".repeat(5000);
+        let parts = split_message_for_telegram(&long_word);
+        assert!(parts.len() >= 2, "word exceeding limit must be split");
+        for part in &parts {
+            assert!(
+                part.len() <= TELEGRAM_MAX_MESSAGE_LENGTH,
+                "hard-split part must be <= {TELEGRAM_MAX_MESSAGE_LENGTH}, got {}",
+                part.len()
+            );
+        }
+        let reassembled: String = parts.join("");
+        assert_eq!(reassembled, long_word);
+    }
+
+    #[test]
+    fn telegram_split_exactly_at_limit_no_split() {
+        let msg = "a".repeat(TELEGRAM_MAX_MESSAGE_LENGTH);
+        let parts = split_message_for_telegram(&msg);
+        assert_eq!(parts.len(), 1, "message exactly at limit should not split");
+    }
+
+    #[test]
+    fn telegram_split_one_over_limit() {
+        let msg = "a".repeat(TELEGRAM_MAX_MESSAGE_LENGTH + 1);
+        let parts = split_message_for_telegram(&msg);
+        assert!(parts.len() >= 2, "message 1 char over limit must split");
+    }
+
+    #[test]
+    fn telegram_split_many_short_lines() {
+        let msg: String = (0..1000).map(|i| format!("line {i}\n")).collect();
+        let parts = split_message_for_telegram(&msg);
+        for part in &parts {
+            assert!(
+                part.len() <= TELEGRAM_MAX_MESSAGE_LENGTH,
+                "short-line batch must be <= limit"
+            );
+        }
+    }
+
+    #[test]
+    fn telegram_split_only_whitespace() {
+        let msg = "   \n\n\t  ";
+        let parts = split_message_for_telegram(msg);
+        assert!(parts.len() <= 1);
+    }
+
+    #[test]
+    fn telegram_split_emoji_at_boundary() {
+        let mut msg = "a".repeat(4094);
+        msg.push_str("🎉🎊"); // 4096 chars total
+        let parts = split_message_for_telegram(&msg);
+        for part in &parts {
+            // The function splits on character count, not byte count
+            assert!(
+                part.chars().count() <= TELEGRAM_MAX_MESSAGE_LENGTH,
+                "emoji boundary split must respect limit"
+            );
+        }
+    }
+
+    #[test]
+    fn telegram_split_consecutive_newlines() {
+        let mut msg = "a".repeat(4090);
+        msg.push_str("\n\n\n\n\n\n");
+        msg.push_str(&"b".repeat(100));
+        let parts = split_message_for_telegram(&msg);
+        for part in &parts {
+            assert!(part.len() <= TELEGRAM_MAX_MESSAGE_LENGTH);
+        }
+    }
 }