From e982cbeb041baea9cd500e2a29862a7eca9e6e17 Mon Sep 17 00:00:00 2001 From: asepharyana Date: Fri, 28 Aug 2026 11:43:33 +0700 Subject: [PATCH] fix(agent): subagent patuhi tool-calling contract + truncation char-safe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hasil audit alur AI agent round 3 (fokus correctness & latent crash). - fix(subagent): engine.rs sebelumnya mengeksekusi tool lalu push ChatMessage::tool hasil TANPA mendahuluinya dengan pesan assistant yang mendeklarasikan tool_calls → history malformed ([..., tool, tool, assistant(text)]). Kontrak OpenAI/Anthropic mensyaratkan pesan assistant (berisi tool_calls) sebelum hasil tool. Kini push response_msg (assistant + tool_calls + content) sebelum eksekusi, dan hapus push assistant content-only di akhir (agar tidak duplikat). Loop utama sudah benar; subagent kini selaras. - fix(utils): &content[..1500] / &content[..1000] di build_rich_context dan &diff[..5000] di auto/engine.rs bisa panic saat indeks byte jatuh di tengah karakter multi-byte UTF-8 (emoji/CJK/panah). Tambah helper truncate_chars() yang memotong per karakter (char-safe) dan pakai di 3 titik tersebut. - test: +4 unit test truncate_chars (ASCII, potong, multibyte no-panic, emoji). Catatan audit: subagent/auto (auto-review) & build_rich_context adalah dead code (spawn_background_review & build_rich_context tidak pernah dipanggil). Auto-review jangan diaktifkan asal (parser format teks rapuh + tanpa verifikasi pasca-fix) — dilaporkan, bukan dicolokkan. --- Cargo.lock | 22 ++++---- .../src/subagent/auto/engine.rs | 2 +- apps/infrastructure/src/subagent/engine.rs | 13 ++--- apps/infrastructure/src/utils.rs | 54 ++++++++++++++++++- 4 files changed, 71 insertions(+), 20 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a351b20..2ddd072 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4862,7 +4862,7 @@ dependencies = [ [[package]] name = "zesdex-api" -version = "1.19.5" +version = "1.19.6" dependencies = [ "anyhow", "argon2", @@ -4885,7 +4885,7 @@ dependencies = [ [[package]] name = "zesdex-application" -version = "1.19.5" +version = "1.19.6" dependencies = [ "anyhow", "base64", @@ -4903,7 +4903,7 @@ dependencies = [ [[package]] name = "zesdex-bootstrap" -version = "1.19.5" +version = "1.19.6" dependencies = [ "anyhow", "chrono", @@ -4920,7 +4920,7 @@ dependencies = [ [[package]] name = "zesdex-daemon" -version = "1.19.5" +version = "1.19.6" dependencies = [ "anyhow", "base64", @@ -4944,7 +4944,7 @@ dependencies = [ [[package]] name = "zesdex-domain" -version = "1.19.5" +version = "1.19.6" dependencies = [ "anyhow", "base64", @@ -4960,7 +4960,7 @@ dependencies = [ [[package]] name = "zesdex-gateway" -version = "1.19.5" +version = "1.19.6" dependencies = [ "anyhow", "axum", @@ -4987,7 +4987,7 @@ dependencies = [ [[package]] name = "zesdex-grpc" -version = "1.19.5" +version = "1.19.6" dependencies = [ "anyhow", "axum", @@ -5004,7 +5004,7 @@ dependencies = [ [[package]] name = "zesdex-infrastructure" -version = "1.19.5" +version = "1.19.6" dependencies = [ "anyhow", "argon2", @@ -5052,7 +5052,7 @@ dependencies = [ [[package]] name = "zesdex-tui" -version = "1.19.5" +version = "1.19.6" dependencies = [ "anyhow", "base64", @@ -5078,7 +5078,7 @@ dependencies = [ [[package]] name = "zesdex-web" -version = "1.19.5" +version = "1.19.6" dependencies = [ "anyhow", "axum", @@ -5098,7 +5098,7 @@ dependencies = [ [[package]] name = "zesdex-ws" -version = "1.19.5" +version = "1.19.6" dependencies = [ "anyhow", "axum", diff --git a/apps/infrastructure/src/subagent/auto/engine.rs b/apps/infrastructure/src/subagent/auto/engine.rs index 4cc5f37..755b06e 100644 --- a/apps/infrastructure/src/subagent/auto/engine.rs +++ b/apps/infrastructure/src/subagent/auto/engine.rs @@ -130,7 +130,7 @@ pub fn spawn_background_review( ); format!( "{}...\n[diff truncated at {} characters]", - &diff[..MAX_DIFF_CHARS], + crate::utils::truncate_chars(&diff, MAX_DIFF_CHARS), MAX_DIFF_CHARS ) } else { diff --git a/apps/infrastructure/src/subagent/engine.rs b/apps/infrastructure/src/subagent/engine.rs index b1af8a9..cdb48b3 100644 --- a/apps/infrastructure/src/subagent/engine.rs +++ b/apps/infrastructure/src/subagent/engine.rs @@ -220,7 +220,7 @@ pub async fn run_agent( .await?; let content = response_msg.content.clone().unwrap_or_default(); - let tool_calls = response_msg.tool_calls.unwrap_or_default(); + let tool_calls = response_msg.tool_calls.clone().unwrap_or_default(); // If no tool calls, we're done — return content if tool_calls.is_empty() { @@ -229,6 +229,12 @@ pub async fn run_agent( return Ok(content); } + // Push the assistant message (with its tool_calls) BEFORE executing + // so the tool-calling contract is honoured: tool results reference + // the calls declared in the preceding assistant message. Without + // this, the history is malformed (`[...tool, tool, assistant]`). + messages.push(response_msg); + // Execute tool calls — read-only batches run concurrently (bounded, // order preserved); any mutating tool forces the safe sequential path. let results = execute_tool_batch(&tools, &tool_ctx, &tool_calls); @@ -266,11 +272,6 @@ pub async fn run_agent( messages.push(ChatMessage::tool(id, truncate_tool_output(result))); } - - // Add assistant response if there was text content - if !content.is_empty() { - messages.push(ChatMessage::assistant(Some(content))); - } } info!("Subagent reached iteration limit ({MAX_ITERATIONS})"); diff --git a/apps/infrastructure/src/utils.rs b/apps/infrastructure/src/utils.rs index e87c96f..fff7d3f 100644 --- a/apps/infrastructure/src/utils.rs +++ b/apps/infrastructure/src/utils.rs @@ -275,7 +275,7 @@ pub fn build_rich_context(root: &Path) -> String { let p = root.join(file); if let Ok(content) = std::fs::read_to_string(&p) { let snippet = if content.len() > 1500 { - format!("{}\n... (truncated)", &content[..1500]) + format!("{}\n... (truncated)", truncate_chars(&content, 1500)) } else { content }; @@ -301,7 +301,7 @@ pub fn build_rich_context(root: &Path) -> String { let readme_path = root.join("README.md"); if let Ok(content) = std::fs::read_to_string(&readme_path) { let snippet = if content.len() > 1000 { - format!("{}\n... (truncated)", &content[..1000]) + format!("{}\n... (truncated)", truncate_chars(&content, 1000)) } else { content }; @@ -348,3 +348,53 @@ pub fn build_rich_context(root: &Path) -> String { ctx.trim_end().to_string() } + +/// Truncate a string to at most `max_chars` **characters**, never cutting a +/// multi-byte UTF-8 code point in half. +/// +/// `&s[..n]` with `n` a raw byte index panics when `n` lands inside a +/// multi-byte character (e.g. an emoji, `→`, or CJK in a README/diff). This +/// helper slices on character boundaries so content is safely capped at a +/// byte budget while remaining valid UTF-8. +pub fn truncate_chars(s: &str, max_chars: usize) -> String { + if s.chars().count() <= max_chars { + return s.to_string(); + } + s.chars().take(max_chars).collect() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn truncate_chars_leaves_short_strings_unchanged() { + assert_eq!(truncate_chars("short", 100), "short"); + assert_eq!(truncate_chars("", 5), ""); + } + + #[test] + fn truncate_chars_cuts_to_max_chars() { + assert_eq!(truncate_chars("hello world", 5), "hello"); + } + + #[test] + fn truncate_chars_never_splits_multibyte_utf8() { + // 4 chars each: '→' is 3 bytes. Byte-slicing at 5 would panic; char + // slicing must not. + let s = "a→b→c→d"; + let t = truncate_chars(s, 5); + assert_eq!(t, "a→b→c"); + assert!(t.chars().count() <= 5); + // No replacement char must appear (valid UTF-8 preserved). + assert!(!t.contains('\u{FFFD}')); + } + + #[test] + fn truncate_chars_handles_emoji() { + let s = "🚀🚀🚀🚀"; + let t = truncate_chars(s, 2); + assert_eq!(t, "🚀🚀"); + assert!(t.chars().count() == 2); + } +}